-
Notifications
You must be signed in to change notification settings - Fork 19
feat: Add more schema features #217
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
base: main
Are you sure you want to change the base?
feat: Add more schema features #217
Conversation
+ left.getTableName() | ||
+ " SET " | ||
+ right.getInterleaveClause().get().getOnDelete()); | ||
// Interleaving changed (added, parent changed, or ON DELETE changed) |
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.
From the documentation for ALTER TABLE T SET INTERLEAVE IN
https://cloud.google.com/spanner/docs/reference/standard-sql/data-definition-language#description_11
Note that directly migrating from an INTERLEAVE IN table to IN PARENT ON DELETE CASCADE is not supported. This must be done in two steps. The first step is to migrate INTERLEAVE IN to INTERLEAVE IN PARENT T [ON DELETE NO ACTION] and the second step is to migrate to INTERLEAVE IN PARENT T ON DELETE CASCADE
Also:
The ON DELETE clause is only supported when migrating to INTERLEAVE IN PARENT
While the tool requires the DDL to be valid (and hence "INTERLEAVE IN T ON DELETE CASCADE" is not valid DDL), adding a sanity check for this in the ASTtable_interleave_clause class would be useful.
Can you add the code checks for both of this please.
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.
Please also add tests in the originalDdl.txt, newDdl.txt, expectedDdlDiff.txt files in the src/test/resources directory...
Specifically ensuring expectedDdlDiff.txt statement order is correct for locality groups
(ie - new locality groups are created before tables that might reference them are created/modified, and removed locality groups are dropped after tables that might reference them are dropped/modified, in a similar way to indexes / foreign key constraints are done)
if (options.get(ALLOW_DROP_STATEMENTS_OPT)) { | ||
for (ASTcreate_locality_group_statement lg : | ||
localityGroupDifferences.entriesOnlyOnLeft().values()) { | ||
LOG.info("Dropping deleted locality group: {}", lg.getNameOrDefault()); |
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.
Is it possible to drop the DEFAULT locality group? If not might be worth adding a check here.
This adds at least some coverage for statements like the following:
CREATE LOCALITY GROUP lg OPTIONS (x=TRUE,y=FALSE);
CREATE TABLE c (k INT64) PRIMARY KEY (k), INTERLEAVE IN p ON DELETE CASCADE;
(rather thanINTERLEAVE IN PARENT
)CREATE TABLE t ( k INT64 ) PRIMARY KEY (k ASC), OPTIONS (opt1=TRUE,opt2='abc',opt3=123)
(allowingOPTIONS
onCREATE TABLE
)