Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Error with JSONB columns containing arrays. #279

Closed
wants to merge 2 commits into from

Conversation

sebastianswms
Copy link
Collaborator

Postgres JSONB columns would always be reported as jsonschema objects, even though they can sometimes hold jsonschema arrays. This can cause downstream issues, such as those documented in the below target-postgres issue.

Closes MeltanoLabs/target-postgres#189

@sebastianswms sebastianswms requested a review from visch October 23, 2023 17:13
@edgarrmondragon
Copy link
Member

@sebastianswms @visch I'm curious what you think of the more general solution in #239?

@visch
Copy link
Member

visch commented Oct 23, 2023

@edgarrmondragon #239 seems better

I dislike how counter-intuitive it is from a user's perspective as If I"m pulling a JSONB column I'd assume it's going to serialize as an object in jsonschema. So we should probably add a decent readme with the examples. @edgarrmondragon has examples in that issue would help folks understand the why here (JSONB can have a bunch of values)

What do you think @sebastianswms curious if you have a different opinion. If not we should just get edgars PR in. It looks like I tested it and just needed to upgrade my dependencies (I was testing the different scenario's myself)

@sebastianswms
Copy link
Collaborator Author

This goes deeper than I thought. After reading through #239 it seems like a fully variant schema is the better solution.

@visch
Copy link
Member

visch commented Nov 1, 2023

Can you get that Readme info added to tap-postgres for JSONB? I'll get #239 merged. Should probably be a sepearte pr

We should close this one based on #279 (comment) right @sebastianswms ?

@sebastianswms
Copy link
Collaborator Author

Closed because #239 is better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ValidationError with Postgres JSON arrays
3 participants