Skip to content

Unify table level foreign key conversion in sql2pgroll #628

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

Merged
merged 23 commits into from
Feb 7, 2025

Conversation

kvch
Copy link
Contributor

@kvch kvch commented Jan 27, 2025

This PR unifies FK constraints in sql2pgroll package. The new function convertFkConstraint in is the unified way to process table level FK constraints of columns and tables.

@kvch kvch marked this pull request as ready for review January 28, 2025 16:32
Copy link
Collaborator

@andrew-farries andrew-farries left a comment

Choose a reason for hiding this comment

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

I think this looks good 👍

It would be good to separate out the simpler changes (eg constant renames) from other changes so that individual PRs don't get too big.

@kvch
Copy link
Contributor Author

kvch commented Jan 29, 2025

Requires #639.
Until that PR is merged, it can stay open as a draft.

@kvch kvch marked this pull request as draft January 29, 2025 12:10
kvch added a commit that referenced this pull request Jan 29, 2025
This PR renames `ForeignKeyReferenceOnDelete` to `ForeignKeyAction`.
The name was too long. Also, `ON DELETE` and `ON UPDATE` referential
actions are the
same. This name makes it clearer that we are reusing this enum for both
options.

Extracted from #628
kvch added a commit that referenced this pull request Feb 3, 2025
…`create_table` (#640)

This PR reuses the same object from constraint definition of
`create_table`.
Also, match type gets its dedicated definition, as it can be reused in
multiple
places as well.

Extracted from #628
kvch added a commit that referenced this pull request Feb 3, 2025
This PR adds support for creating not validated constraints in `ALTER
TABLE` statements.
I am adding unit tests in a follow-up PR, when `ConstraintSQLWriter` is
moved into a separate file.
Either way, it is not used anywhere yet to avoid conflicts with other FK
related
changes in future PRs.

Extracted from #628
kvch added a commit that referenced this pull request Feb 4, 2025
This PR adds the missing `match_type` configuration to
`ConstraintSQLWriter`.
Tests are coming in a follow-up PR when the `ConstraintSQLWriter` gets
its own file.

Extracted from #628
kvch added a commit that referenced this pull request Feb 6, 2025
…`create_constraint` (#653)

Now more foreign key options are supported:
* `match_type` in column definitions and in `create_constraint`
* `on_update` in column definitions and in `create_constraint`
* `on_delete_set_columns` in `create_constraint`
* `on_update` in inline FK definitions in a column's `references`
* `match_type` in inline FK definction in a column's `references`

Extracted from #628. The only
remaining part of that PR is adding support in `sql2pgroll`. 🎉
@kvch kvch changed the title Unify foreign key options in create_table, create_constraint and column level constraints Unify table level foreign key conversion in sql2pgroll Feb 6, 2025
@kvch kvch requested a review from andrew-farries February 6, 2025 16:55
@kvch kvch marked this pull request as ready for review February 6, 2025 16:55
@kvch kvch merged commit 5cff353 into xataio:main Feb 7, 2025
28 checks passed
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.

2 participants