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.

Sign up for GitHub

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

Jump to bottom

fix generated columns in mutable columns for PostgreSQL #210

Merged
merged 5 commits into from
Feb 1, 2023
Merged

fix generated columns in mutable columns for PostgreSQL #210

merged 5 commits into from
Feb 1, 2023

Conversation

rangzen
Copy link
Contributor

@rangzen rangzen commented Jan 30, 2023

Fix #209

@rangzen rangzen mentioned this pull request Jan 30, 2023
Merged
@rangzen
Copy link
Contributor Author

rangzen commented Jan 30, 2023

Need data from go-jet/jet-test-data#3

@go-jet
Copy link
Owner

go-jet commented Jan 31, 2023

LGTM. I've merged test data PR. To fix this PR build you'll need to push one more commit, with updated sub-module master head. You can just call make checkout-testdata and then push changes.

@rangzen
Copy link
Contributor Author

rangzen commented Jan 31, 2023 •
edited
Loading

CircleCI didn't take the new link to jet-test-data. Did I miss something?

@rangzen
Copy link
Contributor Author

rangzen commented Feb 1, 2023

I cannot restart the CI myself.

@go-jet
Copy link
Owner

go-jet commented Feb 1, 2023

Ahh, sorry I missed this. postgres tests are run on two databases postgres and cockrouchdb. pg tests are successfully, but cockrochdb are failing. To fix the tests we will have to add a the same table to cockroach test init data.

@rangzen
Copy link
Contributor Author

rangzen commented Feb 1, 2023

go-jet/jet-test-data#4

go-jet
go-jet reviewed Feb 1, 2023
View reviewed changes
generator/postgres/query_set.go Outdated
@@ -44,6 +44,7 @@ WITH primaryKeys AS (
)
SELECT column_name as "column.Name",
is_nullable = 'YES' as "column.isNullable",
is_generated = 'ALWAYS' as "column.isGenerated",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will need to update this condition since cockroachdb doesn't use ALWAYS but YES as a constant.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have an example in the code where we send one way for PostgreSQL and another way for CockroachDB?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This query is used for both databases. Cockroach shares interfaces with postgres, but there are occasionally some differences. So we just have to add: is_generated = 'ALWAYS' or is_generated = 'YES'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems ok in local tests.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, looks good.

@go-jet go-jet merged commit ec3adb3 into go-jet:master Feb 1, 2023
@rangzen
Copy link
Contributor Author

rangzen commented Feb 1, 2023

@go-jet Thank you 🥳

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@go-jet go-jet go-jet left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

How to customise mutableColumns to read-only generated columns in PostgreSQL?
2 participants
@rangzen @go-jet