Skip to content

Conversation

cliff-openai
Copy link

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 than INTERLEAVE IN PARENT)
  • CREATE TABLE t ( k INT64 ) PRIMARY KEY (k ASC), OPTIONS (opt1=TRUE,opt2='abc',opt3=123) (allowing OPTIONS on CREATE TABLE)

+ left.getTableName()
+ " SET "
+ right.getInterleaveClause().get().getOnDelete());
// Interleaving changed (added, parent changed, or ON DELETE changed)
Copy link
Collaborator

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.

Copy link
Collaborator

@nielm nielm left a 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());
Copy link
Collaborator

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.

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