-
Notifications
You must be signed in to change notification settings - Fork 82
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
Unify foreign key options in create_table
, create_constraint
and column level constraints
#628
base: main
Are you sure you want to change the base?
Conversation
break | ||
} | ||
return true | ||
return !constraint.SkipValidation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The constraint writer already supports skipping validation. But I am adding it to sql2pgroll
later.
There was a problem hiding this 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.
}, | ||
"on_update": { | ||
"description": "On update behavior of the foreign key constraint", | ||
"$ref": "#/$defs/ForeignKeyOnDelete", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like the wrong $ref
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it is on purpose. ON UPDATE
and ON DELETE
have the same actions, so we can reuse it in both places. But I can rename it to make clearer that it is intentional.
Requires #639. |
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
This PR is the beginning of unifying constraint options all over
pgroll
. This addresses the missing options in column level foreign key constraints andcreate_constraint
operation.Now more foreign key options are supported:
match_type
in column definitions and increate_constraint
on_update
in column definitions and increate_constraint
on_delete_set_columns
increate_constraint
It also comes with several refactorings and simplifications:
ForeignKeyReferenceOnDeleteNOACTION
and others are renamed toForeignKeyOnDeleteNOACTION
, because my eyes were bleeding from the long namesTableForeignKeyReference
is used in bothconstraints
ofcreate_table
andcreate_constraint
operationConstraintSQLWriter
is used in adding new columns, creating constraints and creating tablesConstraintSQLWriter
can produce not validated constraintsconvertFkConstraint
insql2pgroll
is the unified way to process FK constraints of columns and tablesWhat comes next in future PRs?
ConstraintSQLWriter
into a separate file and adding unit tests, as it is used from several parts of the codebase nowConstraintSQLWriter
to support other constraint types for columns and creating constraintsunique
,primary_key
andcheck
constraints optionssql2pgroll