From 0cc0280ed95ad34277e1cd7939ebda553d0d03d1 Mon Sep 17 00:00:00 2001 From: Cliff Frey Date: Tue, 26 Aug 2025 22:17:38 -0700 Subject: [PATCH 1/5] add LOCALITY GROUP support --- .../spannerddl/diff/DatabaseDefinition.java | 11 ++++- .../solutions/spannerddl/diff/DdlDiff.java | 45 +++++++++++++++++++ src/main/jjtree-sources/ddl_keywords.jjt | 2 + src/main/jjtree-sources/ddl_parser.jjt | 22 +++++++++ .../spannerddl/diff/DdlDiffTest.java | 26 +++++++++++ .../spannerddl/parser/DDLParserTest.java | 27 +++++++++++ 6 files changed, 132 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/cloud/solutions/spannerddl/diff/DatabaseDefinition.java b/src/main/java/com/google/cloud/solutions/spannerddl/diff/DatabaseDefinition.java index c80fdcd..bf990b1 100644 --- a/src/main/java/com/google/cloud/solutions/spannerddl/diff/DatabaseDefinition.java +++ b/src/main/java/com/google/cloud/solutions/spannerddl/diff/DatabaseDefinition.java @@ -24,6 +24,7 @@ import com.google.cloud.solutions.spannerddl.parser.ASTcreate_change_stream_statement; import com.google.cloud.solutions.spannerddl.parser.ASTcreate_index_statement; import com.google.cloud.solutions.spannerddl.parser.ASTcreate_or_replace_statement; +import com.google.cloud.solutions.spannerddl.parser.ASTcreate_locality_group_statement; import com.google.cloud.solutions.spannerddl.parser.ASTcreate_schema_statement; import com.google.cloud.solutions.spannerddl.parser.ASTcreate_search_index_statement; import com.google.cloud.solutions.spannerddl.parser.ASTcreate_table_statement; @@ -64,6 +65,7 @@ public static DatabaseDefinition create(List statements) { LinkedHashMap changeStreams = new LinkedHashMap<>(); LinkedHashMap alterDatabaseOptions = new LinkedHashMap<>(); LinkedHashMap schemas = new LinkedHashMap<>(); + LinkedHashMap localityGroups = new LinkedHashMap<>(); for (ASTddl_statement ddlStatement : statements) { final SimpleNode statement = (SimpleNode) ddlStatement.jjtGetChild(0); @@ -92,6 +94,10 @@ public static DatabaseDefinition create(List statements) { ((ASTcreate_search_index_statement) statement).getName(), (ASTcreate_search_index_statement) statement); break; + case DdlParserTreeConstants.JJTCREATE_LOCALITY_GROUP_STATEMENT: + ASTcreate_locality_group_statement lg = (ASTcreate_locality_group_statement) statement; + localityGroups.put(lg.getNameOrDefault(), lg); + break; case DdlParserTreeConstants.JJTCREATE_INDEX_STATEMENT: indexes.put( ((ASTcreate_index_statement) statement).getIndexName(), @@ -157,7 +163,8 @@ public static DatabaseDefinition create(List statements) { ImmutableMap.copyOf(ttls), ImmutableMap.copyOf(changeStreams), ImmutableMap.copyOf(alterDatabaseOptions), - ImmutableMap.copyOf(schemas)); + ImmutableMap.copyOf(schemas), + ImmutableMap.copyOf(localityGroups)); } public abstract ImmutableMap tablesInCreationOrder(); @@ -175,4 +182,6 @@ public static DatabaseDefinition create(List statements) { abstract ImmutableMap alterDatabaseOptions(); abstract ImmutableMap schemas(); + + abstract ImmutableMap localityGroups(); } diff --git a/src/main/java/com/google/cloud/solutions/spannerddl/diff/DdlDiff.java b/src/main/java/com/google/cloud/solutions/spannerddl/diff/DdlDiff.java index 978a276..916a5f6 100644 --- a/src/main/java/com/google/cloud/solutions/spannerddl/diff/DdlDiff.java +++ b/src/main/java/com/google/cloud/solutions/spannerddl/diff/DdlDiff.java @@ -28,6 +28,7 @@ import com.google.cloud.solutions.spannerddl.parser.ASTcreate_change_stream_statement; import com.google.cloud.solutions.spannerddl.parser.ASTcreate_index_statement; import com.google.cloud.solutions.spannerddl.parser.ASTcreate_or_replace_statement; +import com.google.cloud.solutions.spannerddl.parser.ASTcreate_locality_group_statement; import com.google.cloud.solutions.spannerddl.parser.ASTcreate_schema_statement; import com.google.cloud.solutions.spannerddl.parser.ASTcreate_search_index_statement; import com.google.cloud.solutions.spannerddl.parser.ASTcreate_table_statement; @@ -103,6 +104,8 @@ public class DdlDiff { private final MapDifference searchIndexDifferences; private final String databaseName; // for alter Database private final MapDifference schemaDifferences; + private final MapDifference + localityGroupDifferences; private DdlDiff(DatabaseDefinition originalDb, DatabaseDefinition newDb, String databaseName) throws DdlDiffException { @@ -122,6 +125,8 @@ private DdlDiff(DatabaseDefinition originalDb, DatabaseDefinition newDb, String this.searchIndexDifferences = Maps.difference(originalDb.searchIndexes(), newDb.searchIndexes()); this.schemaDifferences = Maps.difference(originalDb.schemas(), newDb.schemas()); + this.localityGroupDifferences = + Maps.difference(originalDb.localityGroups(), newDb.localityGroups()); if (!alterDatabaseOptionsDifferences.areEqual() && Strings.isNullOrEmpty(databaseName)) { // should never happen, but... @@ -197,6 +202,15 @@ public List generateDifferenceStatements(Map options) } } + // Drop deleted locality groups. + if (options.get(ALLOW_DROP_STATEMENTS_OPT)) { + for (ASTcreate_locality_group_statement lg : + localityGroupDifferences.entriesOnlyOnLeft().values()) { + LOG.info("Dropping deleted locality group: {}", lg.getNameOrDefault()); + output.add("DROP LOCALITY GROUP " + lg.getNameOrDefault()); + } + } + // drop deleted search indexes. if (options.get(ALLOW_DROP_STATEMENTS_OPT)) { for (String searchIndexName : searchIndexDifferences.entriesOnlyOnLeft().keySet()) { @@ -273,6 +287,36 @@ public List generateDifferenceStatements(Map options) generateAlterTableStatements(difference.leftValue(), difference.rightValue(), options)); } + // update existing locality groups (options only) + for (ValueDifference lgDiff : + localityGroupDifferences.entriesDiffering().values()) { + ASTcreate_locality_group_statement left = lgDiff.leftValue(); + ASTcreate_locality_group_statement right = lgDiff.rightValue(); + + // Only OPTIONS diffs are supported + ASToptions_clause leftOptionsClause = left.getOptionsClause(); + ASToptions_clause rightOptionsClause = right.getOptionsClause(); + + Map leftOptions = + leftOptionsClause == null ? Collections.emptyMap() : leftOptionsClause.getKeyValueMap(); + Map rightOptions = + rightOptionsClause == null ? Collections.emptyMap() : rightOptionsClause.getKeyValueMap(); + MapDifference optionsDiff = Maps.difference(leftOptions, rightOptions); + + String updateText = generateOptionsUpdates(optionsDiff); + if (!Strings.isNullOrEmpty(updateText)) { + output.add( + "ALTER LOCALITY GROUP " + right.getNameOrDefault() + " SET OPTIONS (" + updateText + ")"); + } + } + + // Create new locality groups + for (ASTcreate_locality_group_statement lg : + localityGroupDifferences.entriesOnlyOnRight().values()) { + LOG.info("Creating new locality group: {}", lg.getNameOrDefault()); + output.add(lg.toStringOptionalExistClause(false)); + } + // create schemas for (ASTcreate_schema_statement schema : schemaDifferences.entriesOnlyOnRight().values()) { LOG.info("creating schema: {}", schema.getName()); @@ -815,6 +859,7 @@ public static List parseDdl(String original, boolean parseAnno case DdlParserTreeConstants.JJTALTER_DATABASE_STATEMENT: case DdlParserTreeConstants.JJTCREATE_CHANGE_STREAM_STATEMENT: case DdlParserTreeConstants.JJTCREATE_SEARCH_INDEX_STATEMENT: + case DdlParserTreeConstants.JJTCREATE_LOCALITY_GROUP_STATEMENT: // no-op - allowed break; case DdlParserTreeConstants.JJTCREATE_OR_REPLACE_STATEMENT: diff --git a/src/main/jjtree-sources/ddl_keywords.jjt b/src/main/jjtree-sources/ddl_keywords.jjt index 5056c09..a3ae42d 100755 --- a/src/main/jjtree-sources/ddl_keywords.jjt +++ b/src/main/jjtree-sources/ddl_keywords.jjt @@ -149,6 +149,7 @@ TOKEN: | | | + | | | | @@ -225,6 +226,7 @@ void pseudoReservedWord() #void : | | | + | | | | diff --git a/src/main/jjtree-sources/ddl_parser.jjt b/src/main/jjtree-sources/ddl_parser.jjt index 9c13873..c2a482b 100644 --- a/src/main/jjtree-sources/ddl_parser.jjt +++ b/src/main/jjtree-sources/ddl_parser.jjt @@ -111,6 +111,7 @@ void create_statement() #void : | create_or_replace_statement() | create_change_stream_statement() | create_sequence_statement() + | create_locality_group_statement() | create_role_statement() ) } @@ -520,6 +521,7 @@ void target_type(): | #change_stream | #view | #sequence + | #locality_group } void privilege_target(): @@ -561,6 +563,15 @@ void create_sequence_statement() : [ options_clause() ] } +void create_locality_group_statement() : +{} +{ + + [ if_not_exists() ] + ( #defaultt | qualified_identifier() #name ) + [ options_clause() ] +} + void key() #void : {} { @@ -701,6 +712,7 @@ void drop_statement() : | #schema [ if_exists() ] ) qualified_identifier() #name | #proto_bundle #name + | #locality_group [ if_exists() ] ( #defaultt | qualified_identifier() #name ) ) } @@ -718,6 +730,7 @@ void alter_statement() #void : | alter_sequence_statement() | alter_model_statement() | alter_schema_statement() + | alter_locality_group_statement() ) } @@ -796,6 +809,15 @@ void alter_model_statement() : [ options_clause()] } +void alter_locality_group_statement() : +{} +{ + + [ if_exists() ] + ( #defaultt | qualified_identifier() #name ) + [ options_clause() ] +} + void analyze_statement() : {} { diff --git a/src/test/java/com/google/cloud/solutions/spannerddl/diff/DdlDiffTest.java b/src/test/java/com/google/cloud/solutions/spannerddl/diff/DdlDiffTest.java index 9f255fc..5450be3 100644 --- a/src/test/java/com/google/cloud/solutions/spannerddl/diff/DdlDiffTest.java +++ b/src/test/java/com/google/cloud/solutions/spannerddl/diff/DdlDiffTest.java @@ -538,6 +538,32 @@ public void generateDifferences_dropIndexes() throws DdlDiffException { .isEmpty(); } + @Test + public void generateDifferences_createLocalityGroup() throws DdlDiffException { + DdlDiff diff = DdlDiff.build("", "CREATE LOCALITY GROUP lg OPTIONS (x=TRUE)"); + assertThat(diff.generateDifferenceStatements(ImmutableMap.of(ALLOW_DROP_STATEMENTS_OPT, true))) + .containsExactly("CREATE LOCALITY GROUP lg OPTIONS (x=TRUE)"); + } + + @Test + public void generateDifferences_dropLocalityGroup() throws DdlDiffException { + DdlDiff diff = DdlDiff.build("CREATE LOCALITY GROUP lg", ""); + assertThat(diff.generateDifferenceStatements(ImmutableMap.of(ALLOW_DROP_STATEMENTS_OPT, true))) + .containsExactly("DROP LOCALITY GROUP lg"); + assertThat(diff.generateDifferenceStatements(ImmutableMap.of(ALLOW_DROP_STATEMENTS_OPT, false))) + .isEmpty(); + } + + @Test + public void generateDifferences_alterLocalityGroupOptions() throws DdlDiffException { + DdlDiff diff = + DdlDiff.build( + "CREATE LOCALITY GROUP lg OPTIONS (x=TRUE,y=FALSE)", + "CREATE LOCALITY GROUP lg OPTIONS (x=NULL,y=TRUE,z=123)"); + assertThat(diff.generateDifferenceStatements(ImmutableMap.of(ALLOW_DROP_STATEMENTS_OPT, true))) + .containsExactly("ALTER LOCALITY GROUP lg SET OPTIONS (x=NULL,y=TRUE,z=123)"); + } + @Test public void differentIndexesWithNoRecreate() { Map options = diff --git a/src/test/java/com/google/cloud/solutions/spannerddl/parser/DDLParserTest.java b/src/test/java/com/google/cloud/solutions/spannerddl/parser/DDLParserTest.java index 900358e..1948ac6 100644 --- a/src/test/java/com/google/cloud/solutions/spannerddl/parser/DDLParserTest.java +++ b/src/test/java/com/google/cloud/solutions/spannerddl/parser/DDLParserTest.java @@ -186,6 +186,33 @@ public void ignoreCreateOrReplace() throws ParseException { assertThat(statement.toString()).isEqualTo("CREATE SCHEMA schema_name"); } + @Test + public void parseCreateLocalityGroupNamed() throws ParseException { + ASTcreate_locality_group_statement statement = + (ASTcreate_locality_group_statement) + parse("CREATE LOCALITY GROUP lg OPTIONS (opt1=TRUE,opt2=123)").jjtGetChild(0); + assertThat(statement.toString()) + .isEqualTo("CREATE LOCALITY GROUP lg OPTIONS (opt1=TRUE,opt2=123)"); + + ASTcreate_locality_group_statement statement2 = + (ASTcreate_locality_group_statement) + parseAndVerifyToString(statement.toString()).jjtGetChild(0); + assertThat(statement).isEqualTo(statement2); + } + + @Test + public void parseCreateLocalityGroupDefault() throws ParseException { + ASTcreate_locality_group_statement statement = + (ASTcreate_locality_group_statement) + parse("CREATE LOCALITY GROUP DEFAULT").jjtGetChild(0); + assertThat(statement.toString()).isEqualTo("CREATE LOCALITY GROUP DEFAULT"); + + ASTcreate_locality_group_statement statement2 = + (ASTcreate_locality_group_statement) + parseAndVerifyToString(statement.toString()).jjtGetChild(0); + assertThat(statement).isEqualTo(statement2); + } + private static void parseCheckingParseException(String ddlStatement, String exceptionContains) { ParseException e = assertThrows(ParseException.class, () -> parseAndVerifyToString(ddlStatement)); From d5ed186342a514849ef8a4746ed0d25086ae62a5 Mon Sep 17 00:00:00 2001 From: Cliff Frey Date: Tue, 26 Aug 2025 22:45:25 -0700 Subject: [PATCH 2/5] add INTERLEAVE IN (without PARENT) support --- .../solutions/spannerddl/diff/DdlDiff.java | 67 ++++++++++++------- .../parser/ASTtable_interleave_clause.java | 34 ++++++++-- src/main/jjtree-sources/ddl_parser.jjt | 2 +- .../spannerddl/diff/DdlDiffTest.java | 52 +++++++++++--- .../spannerddl/parser/DDLParserTest.java | 15 +++++ src/test/resources/expectedDdlDiff.txt | 2 +- 6 files changed, 127 insertions(+), 45 deletions(-) diff --git a/src/main/java/com/google/cloud/solutions/spannerddl/diff/DdlDiff.java b/src/main/java/com/google/cloud/solutions/spannerddl/diff/DdlDiff.java index 916a5f6..fc16836 100644 --- a/src/main/java/com/google/cloud/solutions/spannerddl/diff/DdlDiff.java +++ b/src/main/java/com/google/cloud/solutions/spannerddl/diff/DdlDiff.java @@ -36,6 +36,7 @@ import com.google.cloud.solutions.spannerddl.parser.ASTforeign_key; import com.google.cloud.solutions.spannerddl.parser.ASToptions_clause; import com.google.cloud.solutions.spannerddl.parser.ASTrow_deletion_policy_clause; +import com.google.cloud.solutions.spannerddl.parser.ASTtable_interleave_clause; import com.google.cloud.solutions.spannerddl.parser.DdlParser; import com.google.cloud.solutions.spannerddl.parser.DdlParserTreeConstants; import com.google.cloud.solutions.spannerddl.parser.ParseException; @@ -55,6 +56,7 @@ import java.util.Map; import java.util.Map.Entry; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.TreeMap; import java.util.function.Function; @@ -500,36 +502,51 @@ static List generateAlterTableStatements( // - change not null on column. // note that constraints need to be dropped before columns, and created after columns. - // Check interleaving has not changed. - if (left.getInterleaveClause().isPresent() != right.getInterleaveClause().isPresent()) { - throw new DdlDiffException("Cannot change interleaving on table " + left.getTableName()); - } - - if (left.getInterleaveClause().isPresent() - && !left.getInterleaveClause() - .get() - .getParentTableName() - .equals(right.getInterleaveClause().get().getParentTableName())) { - throw new DdlDiffException( - "Cannot change interleaved parent of table " + left.getTableName()); - } - // Check Key is same if (!left.getPrimaryKey().toString().equals(right.getPrimaryKey().toString())) { throw new DdlDiffException("Cannot change primary key of table " + left.getTableName()); } - // On delete changed - if (left.getInterleaveClause().isPresent() - && !left.getInterleaveClause() - .get() - .getOnDelete() - .equals(right.getInterleaveClause().get().getOnDelete())) { - alterStatements.add( - "ALTER TABLE " - + left.getTableName() - + " SET " - + right.getInterleaveClause().get().getOnDelete()); + // Interleaving changed (added, parent changed, or ON DELETE changed) + final Optional leftInterleave = left.getInterleaveClause(); + final Optional rightInterleave = right.getInterleaveClause(); + + if (leftInterleave.isPresent() != rightInterleave.isPresent()) { + if (rightInterleave.isPresent()) { + // Added interleave + ASTtable_interleave_clause ri = rightInterleave.get(); + alterStatements.add( + Joiner.on(" ") + .skipNulls() + .join( + "ALTER TABLE", + left.getTableName(), + "SET INTERLEAVE IN", + (ri.hasParentKeyword() ? "PARENT" : null), + ri.getParentTableName(), + ri.getOnDelete())); + } else { + // Removal not supported + throw new DdlDiffException( + "Cannot change interleaving on table " + left.getTableName()); + } + } else if (leftInterleave.isPresent()) { + ASTtable_interleave_clause li = leftInterleave.get(); + ASTtable_interleave_clause ri = rightInterleave.get(); + if (!li.getParentTableName().equals(ri.getParentTableName()) + || !li.getOnDelete().equals(ri.getOnDelete()) + || li.hasParentKeyword() != ri.hasParentKeyword()) { + alterStatements.add( + Joiner.on(" ") + .skipNulls() + .join( + "ALTER TABLE", + left.getTableName(), + "SET INTERLEAVE IN", + (ri.hasParentKeyword() ? "PARENT" : null), + ri.getParentTableName(), + ri.getOnDelete())); + } } // compare columns. diff --git a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTtable_interleave_clause.java b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTtable_interleave_clause.java index 4965756..38e3916 100644 --- a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTtable_interleave_clause.java +++ b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTtable_interleave_clause.java @@ -30,20 +30,40 @@ public ASTtable_interleave_clause(DdlParser p, int id) { } public String getParentTableName() { - return AstTreeUtils.tokensToString((ASTinterleave_in) children[0]); + for (Node child : children) { + if (child instanceof ASTinterleave_in) { + return AstTreeUtils.tokensToString((ASTinterleave_in) child); + } + } + return null; + } + + public boolean hasParentKeyword() { + for (Node child : children) { + if (child instanceof ASTparent) { + return true; + } + } + return false; } public String getOnDelete() { - if (children.length == 2) { - // verify child type - return children[1].toString(); - } else { - return ASTon_delete_clause.ON_DELETE_NO_ACTION; + for (Node child : children) { + if (child instanceof ASTon_delete_clause) { + return child.toString(); + } } + return ASTon_delete_clause.ON_DELETE_NO_ACTION; } @Override public String toString() { - return Joiner.on(" ").join("INTERLEAVE IN PARENT", getParentTableName(), getOnDelete()); + return Joiner.on(" ") + .skipNulls() + .join( + "INTERLEAVE IN", + (hasParentKeyword() ? "PARENT" : null), + getParentTableName(), + getOnDelete()); } } diff --git a/src/main/jjtree-sources/ddl_parser.jjt b/src/main/jjtree-sources/ddl_parser.jjt index c2a482b..3ac1bbe 100644 --- a/src/main/jjtree-sources/ddl_parser.jjt +++ b/src/main/jjtree-sources/ddl_parser.jjt @@ -350,7 +350,7 @@ void synonym_clause() : void table_interleave_clause() : {} { - qualified_identifier() #interleave_in + [ ( #parent ) ] qualified_identifier() #interleave_in [ on_delete_clause() ] } diff --git a/src/test/java/com/google/cloud/solutions/spannerddl/diff/DdlDiffTest.java b/src/test/java/com/google/cloud/solutions/spannerddl/diff/DdlDiffTest.java index 5450be3..9554944 100644 --- a/src/test/java/com/google/cloud/solutions/spannerddl/diff/DdlDiffTest.java +++ b/src/test/java/com/google/cloud/solutions/spannerddl/diff/DdlDiffTest.java @@ -300,14 +300,15 @@ public void generateAlterTable_changeInterleaving() throws DdlDiffException { true, "Cannot change interleaving on table test1"); - // remove different parent - getDiffCheckDdlDiffException( - "create table test1 (col1 int64, col2 int64) " - + "primary key (col1), interleave in parent testparent;", - "create table test1 (col1 int64, col2 int64) " - + "primary key (col1), interleave in parent otherparent", - true, - "Cannot change interleaved parent of table test1"); + // change parent should generate ALTER + assertThat( + getDiff( + "create table test1 (col1 int64, col2 int64) " + + "primary key (col1), interleave in parent testparent;", + "create table test1 (col1 int64, col2 int64) " + + "primary key (col1), interleave in parent otherparent", + true)) + .containsExactly("ALTER TABLE test1 SET INTERLEAVE IN PARENT otherparent ON DELETE NO ACTION"); // change on delete assertThat( @@ -317,7 +318,7 @@ public void generateAlterTable_changeInterleaving() throws DdlDiffException { "create table test1 (col1 int64, col2 int64) " + "primary key (col1), interleave in parent testparent", true)) - .containsExactly("ALTER TABLE test1 SET ON DELETE NO ACTION"); + .containsExactly("ALTER TABLE test1 SET INTERLEAVE IN PARENT testparent ON DELETE NO ACTION"); // change on delete assertThat( getDiff( @@ -326,7 +327,7 @@ public void generateAlterTable_changeInterleaving() throws DdlDiffException { "create table test1 (col1 int64, col2 int64) " + "primary key (col1), interleave in parent testparent on delete cascade", true)) - .containsExactly("ALTER TABLE test1 SET ON DELETE CASCADE"); + .containsExactly("ALTER TABLE test1 SET INTERLEAVE IN PARENT testparent ON DELETE CASCADE"); } @Test @@ -384,7 +385,7 @@ public void generateAlterTable_alterStatementOrdering() throws DdlDiffException true)) // allow drop .containsExactly( // change table options. - "ALTER TABLE test1 SET ON DELETE CASCADE", + "ALTER TABLE test1 SET INTERLEAVE IN PARENT testparent ON DELETE CASCADE", // then drop cols "ALTER TABLE test1 DROP COLUMN col2", // then add cols @@ -564,6 +565,35 @@ public void generateDifferences_alterLocalityGroupOptions() throws DdlDiffExcept .containsExactly("ALTER LOCALITY GROUP lg SET OPTIONS (x=NULL,y=TRUE,z=123)"); } + @Test + public void alterTable_interleaveOnDeleteChange_generatesAlter() throws DdlDiffException { + String original = + "CREATE TABLE c (k INT64) PRIMARY KEY (k), INTERLEAVE IN PARENT p ON DELETE NO ACTION;"; + String updated = + "CREATE TABLE c (k INT64) PRIMARY KEY (k), INTERLEAVE IN p ON DELETE CASCADE;"; + assertThat(getDiff(original, updated, true)) + .containsExactly("ALTER TABLE c SET INTERLEAVE IN p ON DELETE CASCADE"); + } + + @Test + public void alterTable_interleaveParentChange_generatesAlter() throws DdlDiffException { + String original = + "CREATE TABLE c (k INT64) PRIMARY KEY (k), INTERLEAVE IN PARENT p1 ON DELETE CASCADE;"; + String updated = + "CREATE TABLE c (k INT64) PRIMARY KEY (k), INTERLEAVE IN p2 ON DELETE CASCADE;"; + assertThat(getDiff(original, updated, true)) + .containsExactly("ALTER TABLE c SET INTERLEAVE IN p2 ON DELETE CASCADE"); + } + + @Test + public void alterTable_interleaveAdded_generatesAlter() throws DdlDiffException { + String original = "CREATE TABLE c (k INT64) PRIMARY KEY (k);"; + String updated = + "CREATE TABLE c (k INT64) PRIMARY KEY (k), INTERLEAVE IN p ON DELETE NO ACTION;"; + assertThat(getDiff(original, updated, true)) + .containsExactly("ALTER TABLE c SET INTERLEAVE IN p ON DELETE NO ACTION"); + } + @Test public void differentIndexesWithNoRecreate() { Map options = diff --git a/src/test/java/com/google/cloud/solutions/spannerddl/parser/DDLParserTest.java b/src/test/java/com/google/cloud/solutions/spannerddl/parser/DDLParserTest.java index 1948ac6..61f718c 100644 --- a/src/test/java/com/google/cloud/solutions/spannerddl/parser/DDLParserTest.java +++ b/src/test/java/com/google/cloud/solutions/spannerddl/parser/DDLParserTest.java @@ -178,6 +178,21 @@ public void doNotNormalizeSQLExpressions() throws ParseException { assertThat(statement).isEqualTo(statement2); } + @Test + public void parseCreateTable_interleaveWithoutParent_parsesAndNormalizes() throws ParseException { + // Input omits PARENT, parser should accept and normalize to include PARENT in toString + ASTcreate_table_statement stmt = + (ASTcreate_table_statement) + parse( + "CREATE TABLE t (k INT64) PRIMARY KEY (k), INTERLEAVE IN parent_table") + .jjtGetChild(0); + + // Output preserves omission of PARENT when not provided + assertThat(stmt.toString()) + .isEqualTo( + "CREATE TABLE t ( k INT64 ) PRIMARY KEY (k ASC), INTERLEAVE IN parent_table ON DELETE NO ACTION"); + } + @Test public void ignoreCreateOrReplace() throws ParseException { ASTcreate_or_replace_statement statement = diff --git a/src/test/resources/expectedDdlDiff.txt b/src/test/resources/expectedDdlDiff.txt index f41ac9d..6488d64 100644 --- a/src/test/resources/expectedDdlDiff.txt +++ b/src/test/resources/expectedDdlDiff.txt @@ -37,7 +37,7 @@ ALTER TABLE test1 ALTER COLUMN col2 INT64 NOT NULL ALTER TABLE test1 ALTER COLUMN col3 STRING(MAX) ALTER TABLE test1 ALTER COLUMN col4 ARRAY ALTER TABLE test2 ADD COLUMN newcol2 STRING(MAX) -ALTER TABLE test3 SET ON DELETE NO ACTION +ALTER TABLE test3 SET INTERLEAVE IN PARENT test2 ON DELETE NO ACTION ALTER TABLE test3 ADD COLUMN col3 TIMESTAMP CREATE TABLE cccparent ( col1 INT64 ) PRIMARY KEY (col1 ASC) CREATE TABLE bbbchild1 ( col1 INT64, col2 INT64 ) PRIMARY KEY (col1 ASC, col2 ASC), INTERLEAVE IN PARENT cccparent ON DELETE NO ACTION From 2fffb7a95e1cf29cd2a6a28079c219e7854e7478 Mon Sep 17 00:00:00 2001 From: Cliff Frey Date: Tue, 2 Sep 2025 12:43:13 -0700 Subject: [PATCH 3/5] add support for OPTIONS on CREATE TABLE --- .../parser/ASTcreate_table_statement.java | 4 +- src/main/jjtree-sources/ddl_parser.jjt | 1 + .../spannerddl/parser/DDLParserTest.java | 49 +++++++++++++++++++ 3 files changed, 53 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcreate_table_statement.java b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcreate_table_statement.java index 821c856..f7b593a 100644 --- a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcreate_table_statement.java +++ b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcreate_table_statement.java @@ -126,7 +126,8 @@ public String toStringOptionalExistClause(boolean includeExists) { getOptionalChildByType(children, ASTtable_interleave_clause.class), (withConstraints ? getOptionalChildByType(children, ASTrow_deletion_policy_clause.class) - : null))); + : null), + getOptionalChildByType(children, ASToptions_clause.class))); } private void validateChildren() { @@ -141,6 +142,7 @@ private void validateChildren() { ASTprimary_key.class, ASTtable_interleave_clause.class, ASTrow_deletion_policy_clause.class, + ASToptions_clause.class, ASTannotation.class)); } diff --git a/src/main/jjtree-sources/ddl_parser.jjt b/src/main/jjtree-sources/ddl_parser.jjt index 3ac1bbe..968d497 100644 --- a/src/main/jjtree-sources/ddl_parser.jjt +++ b/src/main/jjtree-sources/ddl_parser.jjt @@ -205,6 +205,7 @@ void create_table_statement() : primary_key() [ LOOKAHEAD(2) "," table_interleave_clause() ] [ LOOKAHEAD(2) "," row_deletion_policy_clause() ] + [ LOOKAHEAD(2) "," options_clause() ] } void table_element() #void : diff --git a/src/test/java/com/google/cloud/solutions/spannerddl/parser/DDLParserTest.java b/src/test/java/com/google/cloud/solutions/spannerddl/parser/DDLParserTest.java index 61f718c..facd3ff 100644 --- a/src/test/java/com/google/cloud/solutions/spannerddl/parser/DDLParserTest.java +++ b/src/test/java/com/google/cloud/solutions/spannerddl/parser/DDLParserTest.java @@ -228,6 +228,55 @@ public void parseCreateLocalityGroupDefault() throws ParseException { assertThat(statement).isEqualTo(statement2); } + @Test + public void parseCreateTable_withTableOptions_onlyPrimaryKey() throws ParseException { + ASTcreate_table_statement stmt = + (ASTcreate_table_statement) + parse( + "CREATE TABLE t (\n" + + " k INT64,\n" + + ") PRIMARY KEY(k), OPTIONS (opt1=TRUE,opt2='abc',opt3=123)") + .jjtGetChild(0); + + assertThat(stmt.toString()) + .isEqualTo( + "CREATE TABLE t ( k INT64 ) PRIMARY KEY (k ASC), OPTIONS (opt1=TRUE,opt2='abc',opt3=123)"); + + ASTcreate_table_statement stmt2 = + (ASTcreate_table_statement) parseAndVerifyToString(stmt.toString()).jjtGetChild(0); + assertThat(stmt).isEqualTo(stmt2); + } + + @Test + public void parseCreateTable_withInterleaveRowDeletion_andTableOptions() throws ParseException { + ASTcreate_table_statement stmt = + (ASTcreate_table_statement) + parse( + "CREATE TABLE child_table (\n" + + " k1 STRING(MAX) NOT NULL,\n" + + " k2 STRING(MAX) NOT NULL,\n" + + " col1 STRING(MAX),\n" + + " col2 INT64,\n" + + " ts TIMESTAMP,\n" + + ") PRIMARY KEY(k1, k2),\n" + + " INTERLEAVE IN parent_table, ROW DELETION POLICY (OLDER_THAN(ts, INTERVAL 1 DAY)), OPTIONS (\n" + + " opt1 = TRUE\n" + + ")") + .jjtGetChild(0); + + assertThat(stmt.toString()) + .isEqualTo( + ("CREATE TABLE child_table ( k1 STRING(MAX) NOT NULL, k2 STRING(MAX) NOT NULL, col1" + + " STRING(MAX), col2 INT64, ts TIMESTAMP ) PRIMARY KEY (k1 ASC, k2 ASC)," + + " INTERLEAVE IN parent_table ON DELETE NO ACTION, ROW DELETION POLICY (OLDER_THAN (" + + " ts, INTERVAL 1 DAY )), OPTIONS (opt1=TRUE)") + .replaceAll("\\s+", " ")); + + ASTcreate_table_statement stmt2 = + (ASTcreate_table_statement) parseAndVerifyToString(stmt.toString()).jjtGetChild(0); + assertThat(stmt).isEqualTo(stmt2); + } + private static void parseCheckingParseException(String ddlStatement, String exceptionContains) { ParseException e = assertThrows(ParseException.class, () -> parseAndVerifyToString(ddlStatement)); From 092314835f960ebbaeddf93be248d252ac89563a Mon Sep 17 00:00:00 2001 From: Cliff Frey Date: Mon, 22 Sep 2025 09:29:41 -0700 Subject: [PATCH 4/5] clean up 'mvn -B verify --file pom.xml' --- .../spannerddl/diff/DatabaseDefinition.java | 5 +- .../solutions/spannerddl/diff/DdlDiff.java | 11 ++- .../ASTcreate_locality_group_statement.java | 81 +++++++++++++++++++ .../spannerddl/diff/DdlDiffTest.java | 9 ++- .../spannerddl/parser/DDLParserTest.java | 6 +- 5 files changed, 96 insertions(+), 16 deletions(-) create mode 100644 src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcreate_locality_group_statement.java diff --git a/src/main/java/com/google/cloud/solutions/spannerddl/diff/DatabaseDefinition.java b/src/main/java/com/google/cloud/solutions/spannerddl/diff/DatabaseDefinition.java index bf990b1..d6c1037 100644 --- a/src/main/java/com/google/cloud/solutions/spannerddl/diff/DatabaseDefinition.java +++ b/src/main/java/com/google/cloud/solutions/spannerddl/diff/DatabaseDefinition.java @@ -23,8 +23,8 @@ import com.google.cloud.solutions.spannerddl.parser.ASTcheck_constraint; import com.google.cloud.solutions.spannerddl.parser.ASTcreate_change_stream_statement; import com.google.cloud.solutions.spannerddl.parser.ASTcreate_index_statement; -import com.google.cloud.solutions.spannerddl.parser.ASTcreate_or_replace_statement; import com.google.cloud.solutions.spannerddl.parser.ASTcreate_locality_group_statement; +import com.google.cloud.solutions.spannerddl.parser.ASTcreate_or_replace_statement; import com.google.cloud.solutions.spannerddl.parser.ASTcreate_schema_statement; import com.google.cloud.solutions.spannerddl.parser.ASTcreate_search_index_statement; import com.google.cloud.solutions.spannerddl.parser.ASTcreate_table_statement; @@ -65,7 +65,8 @@ public static DatabaseDefinition create(List statements) { LinkedHashMap changeStreams = new LinkedHashMap<>(); LinkedHashMap alterDatabaseOptions = new LinkedHashMap<>(); LinkedHashMap schemas = new LinkedHashMap<>(); - LinkedHashMap localityGroups = new LinkedHashMap<>(); + LinkedHashMap localityGroups = + new LinkedHashMap<>(); for (ASTddl_statement ddlStatement : statements) { final SimpleNode statement = (SimpleNode) ddlStatement.jjtGetChild(0); diff --git a/src/main/java/com/google/cloud/solutions/spannerddl/diff/DdlDiff.java b/src/main/java/com/google/cloud/solutions/spannerddl/diff/DdlDiff.java index fc16836..5b869a4 100644 --- a/src/main/java/com/google/cloud/solutions/spannerddl/diff/DdlDiff.java +++ b/src/main/java/com/google/cloud/solutions/spannerddl/diff/DdlDiff.java @@ -27,8 +27,8 @@ import com.google.cloud.solutions.spannerddl.parser.ASTcolumn_type; import com.google.cloud.solutions.spannerddl.parser.ASTcreate_change_stream_statement; import com.google.cloud.solutions.spannerddl.parser.ASTcreate_index_statement; -import com.google.cloud.solutions.spannerddl.parser.ASTcreate_or_replace_statement; import com.google.cloud.solutions.spannerddl.parser.ASTcreate_locality_group_statement; +import com.google.cloud.solutions.spannerddl.parser.ASTcreate_or_replace_statement; import com.google.cloud.solutions.spannerddl.parser.ASTcreate_schema_statement; import com.google.cloud.solutions.spannerddl.parser.ASTcreate_search_index_statement; import com.google.cloud.solutions.spannerddl.parser.ASTcreate_table_statement; @@ -106,8 +106,7 @@ public class DdlDiff { private final MapDifference searchIndexDifferences; private final String databaseName; // for alter Database private final MapDifference schemaDifferences; - private final MapDifference - localityGroupDifferences; + private final MapDifference localityGroupDifferences; private DdlDiff(DatabaseDefinition originalDb, DatabaseDefinition newDb, String databaseName) throws DdlDiffException { @@ -308,7 +307,8 @@ public List generateDifferenceStatements(Map options) String updateText = generateOptionsUpdates(optionsDiff); if (!Strings.isNullOrEmpty(updateText)) { output.add( - "ALTER LOCALITY GROUP " + right.getNameOrDefault() + " SET OPTIONS (" + updateText + ")"); + String.format( + "ALTER LOCALITY GROUP %s SET OPTIONS (%s)", right.getNameOrDefault(), updateText)); } } @@ -527,8 +527,7 @@ static List generateAlterTableStatements( ri.getOnDelete())); } else { // Removal not supported - throw new DdlDiffException( - "Cannot change interleaving on table " + left.getTableName()); + throw new DdlDiffException("Cannot change interleaving on table " + left.getTableName()); } } else if (leftInterleave.isPresent()) { ASTtable_interleave_clause li = leftInterleave.get(); diff --git a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcreate_locality_group_statement.java b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcreate_locality_group_statement.java new file mode 100644 index 0000000..3a7f6c8 --- /dev/null +++ b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTcreate_locality_group_statement.java @@ -0,0 +1,81 @@ +/* + * Copyright 2024 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.cloud.solutions.spannerddl.parser; + +import static com.google.cloud.solutions.spannerddl.diff.AstTreeUtils.getChildByType; +import static com.google.cloud.solutions.spannerddl.diff.AstTreeUtils.getOptionalChildByType; + +import com.google.cloud.solutions.spannerddl.diff.AstTreeUtils; +import com.google.common.base.Joiner; +import com.google.common.collect.ImmutableSet; + +public class ASTcreate_locality_group_statement extends SimpleNode { + public ASTcreate_locality_group_statement(int id) { + super(id); + } + + public ASTcreate_locality_group_statement(DdlParser p, int id) { + super(p, id); + } + + private void validateChildren() { + AstTreeUtils.validateChildrenClasses( + children, + ImmutableSet.of( + ASTif_not_exists.class, ASTdefaultt.class, ASTname.class, ASToptions_clause.class)); + } + + public boolean isDefault() { + return getOptionalChildByType(children, ASTdefaultt.class) != null; + } + + public String getNameOrDefault() { + return isDefault() ? "DEFAULT" : getChildByType(children, ASTname.class).toString(); + } + + @Override + public String toString() { + return toStringOptionalExistClause(true); + } + + /** Create string version, optionally including the IF NOT EXISTS clause */ + public String toStringOptionalExistClause(boolean includeExists) { + validateChildren(); + return Joiner.on(" ") + .skipNulls() + .join( + "CREATE", + "LOCALITY", + "GROUP", + (includeExists ? getOptionalChildByType(children, ASTif_not_exists.class) : null), + getNameOrDefault(), + AstTreeUtils.getOptionalChildByType(children, ASToptions_clause.class)); + } + + @Override + public boolean equals(Object obj) { + return (obj instanceof ASTcreate_locality_group_statement) && toString().equals(obj.toString()); + } + + @Override + public int hashCode() { + return toString().hashCode(); + } + + public ASToptions_clause getOptionsClause() { + return getOptionalChildByType(children, ASToptions_clause.class); + } +} diff --git a/src/test/java/com/google/cloud/solutions/spannerddl/diff/DdlDiffTest.java b/src/test/java/com/google/cloud/solutions/spannerddl/diff/DdlDiffTest.java index 9554944..fe24c43 100644 --- a/src/test/java/com/google/cloud/solutions/spannerddl/diff/DdlDiffTest.java +++ b/src/test/java/com/google/cloud/solutions/spannerddl/diff/DdlDiffTest.java @@ -308,7 +308,8 @@ public void generateAlterTable_changeInterleaving() throws DdlDiffException { "create table test1 (col1 int64, col2 int64) " + "primary key (col1), interleave in parent otherparent", true)) - .containsExactly("ALTER TABLE test1 SET INTERLEAVE IN PARENT otherparent ON DELETE NO ACTION"); + .containsExactly( + "ALTER TABLE test1 SET INTERLEAVE IN PARENT otherparent ON DELETE NO ACTION"); // change on delete assertThat( @@ -318,7 +319,8 @@ public void generateAlterTable_changeInterleaving() throws DdlDiffException { "create table test1 (col1 int64, col2 int64) " + "primary key (col1), interleave in parent testparent", true)) - .containsExactly("ALTER TABLE test1 SET INTERLEAVE IN PARENT testparent ON DELETE NO ACTION"); + .containsExactly( + "ALTER TABLE test1 SET INTERLEAVE IN PARENT testparent ON DELETE NO ACTION"); // change on delete assertThat( getDiff( @@ -569,8 +571,7 @@ public void generateDifferences_alterLocalityGroupOptions() throws DdlDiffExcept public void alterTable_interleaveOnDeleteChange_generatesAlter() throws DdlDiffException { String original = "CREATE TABLE c (k INT64) PRIMARY KEY (k), INTERLEAVE IN PARENT p ON DELETE NO ACTION;"; - String updated = - "CREATE TABLE c (k INT64) PRIMARY KEY (k), INTERLEAVE IN p ON DELETE CASCADE;"; + String updated = "CREATE TABLE c (k INT64) PRIMARY KEY (k), INTERLEAVE IN p ON DELETE CASCADE;"; assertThat(getDiff(original, updated, true)) .containsExactly("ALTER TABLE c SET INTERLEAVE IN p ON DELETE CASCADE"); } diff --git a/src/test/java/com/google/cloud/solutions/spannerddl/parser/DDLParserTest.java b/src/test/java/com/google/cloud/solutions/spannerddl/parser/DDLParserTest.java index facd3ff..d2ca1a7 100644 --- a/src/test/java/com/google/cloud/solutions/spannerddl/parser/DDLParserTest.java +++ b/src/test/java/com/google/cloud/solutions/spannerddl/parser/DDLParserTest.java @@ -183,8 +183,7 @@ public void parseCreateTable_interleaveWithoutParent_parsesAndNormalizes() throw // Input omits PARENT, parser should accept and normalize to include PARENT in toString ASTcreate_table_statement stmt = (ASTcreate_table_statement) - parse( - "CREATE TABLE t (k INT64) PRIMARY KEY (k), INTERLEAVE IN parent_table") + parse("CREATE TABLE t (k INT64) PRIMARY KEY (k), INTERLEAVE IN parent_table") .jjtGetChild(0); // Output preserves omission of PARENT when not provided @@ -218,8 +217,7 @@ public void parseCreateLocalityGroupNamed() throws ParseException { @Test public void parseCreateLocalityGroupDefault() throws ParseException { ASTcreate_locality_group_statement statement = - (ASTcreate_locality_group_statement) - parse("CREATE LOCALITY GROUP DEFAULT").jjtGetChild(0); + (ASTcreate_locality_group_statement) parse("CREATE LOCALITY GROUP DEFAULT").jjtGetChild(0); assertThat(statement.toString()).isEqualTo("CREATE LOCALITY GROUP DEFAULT"); ASTcreate_locality_group_statement statement2 = From dc5a4fd0b2777ed44e36575ed44ac9ca34ddff3f Mon Sep 17 00:00:00 2001 From: Cliff Frey Date: Fri, 5 Dec 2025 16:37:20 -0800 Subject: [PATCH 5/5] interleave support? --- .../solutions/spannerddl/diff/DdlDiff.java | 66 +++++++++++++------ .../parser/ASTon_delete_clause.java | 4 +- .../parser/ASTtable_interleave_clause.java | 20 +++++- .../spannerddl/diff/DdlDiffTest.java | 24 +++++-- .../spannerddl/parser/DDLParserTest.java | 22 +++++-- 5 files changed, 101 insertions(+), 35 deletions(-) diff --git a/src/main/java/com/google/cloud/solutions/spannerddl/diff/DdlDiff.java b/src/main/java/com/google/cloud/solutions/spannerddl/diff/DdlDiff.java index 5b869a4..d043c61 100644 --- a/src/main/java/com/google/cloud/solutions/spannerddl/diff/DdlDiff.java +++ b/src/main/java/com/google/cloud/solutions/spannerddl/diff/DdlDiff.java @@ -34,6 +34,7 @@ import com.google.cloud.solutions.spannerddl.parser.ASTcreate_table_statement; import com.google.cloud.solutions.spannerddl.parser.ASTddl_statement; import com.google.cloud.solutions.spannerddl.parser.ASTforeign_key; +import com.google.cloud.solutions.spannerddl.parser.ASTon_delete_clause; import com.google.cloud.solutions.spannerddl.parser.ASToptions_clause; import com.google.cloud.solutions.spannerddl.parser.ASTrow_deletion_policy_clause; import com.google.cloud.solutions.spannerddl.parser.ASTtable_interleave_clause; @@ -516,15 +517,11 @@ static List generateAlterTableStatements( // Added interleave ASTtable_interleave_clause ri = rightInterleave.get(); alterStatements.add( - Joiner.on(" ") - .skipNulls() - .join( - "ALTER TABLE", - left.getTableName(), - "SET INTERLEAVE IN", - (ri.hasParentKeyword() ? "PARENT" : null), - ri.getParentTableName(), - ri.getOnDelete())); + buildSetInterleaveStatement( + left.getTableName(), + ri.getParentTableName(), + ri.hasParentKeyword(), + ri.getOnDelete())); } else { // Removal not supported throw new DdlDiffException("Cannot change interleaving on table " + left.getTableName()); @@ -533,18 +530,34 @@ static List generateAlterTableStatements( ASTtable_interleave_clause li = leftInterleave.get(); ASTtable_interleave_clause ri = rightInterleave.get(); if (!li.getParentTableName().equals(ri.getParentTableName()) - || !li.getOnDelete().equals(ri.getOnDelete()) + || !Objects.equals(li.getOnDelete(), ri.getOnDelete()) || li.hasParentKeyword() != ri.hasParentKeyword()) { - alterStatements.add( - Joiner.on(" ") - .skipNulls() - .join( - "ALTER TABLE", - left.getTableName(), - "SET INTERLEAVE IN", - (ri.hasParentKeyword() ? "PARENT" : null), - ri.getParentTableName(), - ri.getOnDelete())); + if (!li.hasParentKeyword() + && ri.hasParentKeyword() + && Objects.equals(ri.getParentTableName(), li.getParentTableName()) + && Objects.equals(ri.getOnDelete(), ASTon_delete_clause.ON_DELETE_CASCADE)) { + // Migration from INTERLEAVE IN -> INTERLEAVE IN PARENT ... ON DELETE CASCADE must + // be performed in two statements. + alterStatements.add( + buildSetInterleaveStatement( + left.getTableName(), + ri.getParentTableName(), + true, + ASTon_delete_clause.ON_DELETE_NO_ACTION)); + alterStatements.add( + buildSetInterleaveStatement( + left.getTableName(), + ri.getParentTableName(), + true, + ASTon_delete_clause.ON_DELETE_CASCADE)); + } else { + alterStatements.add( + buildSetInterleaveStatement( + left.getTableName(), + ri.getParentTableName(), + ri.hasParentKeyword(), + ri.getOnDelete())); + } } } @@ -571,6 +584,19 @@ static List generateAlterTableStatements( return alterStatements; } + private static String buildSetInterleaveStatement( + String tableName, String parentTableName, boolean includeParentKeyword, String onDeleteClause) { + return Joiner.on(" ") + .skipNulls() + .join( + "ALTER TABLE", + tableName, + "SET INTERLEAVE IN", + (includeParentKeyword ? "PARENT" : null), + parentTableName, + onDeleteClause); + } + private static void addColumnDiffs( String tableName, List alterStatements, ValueDifference columnDiff) throws DdlDiffException { diff --git a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTon_delete_clause.java b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTon_delete_clause.java index a050a2e..90375e1 100644 --- a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTon_delete_clause.java +++ b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTon_delete_clause.java @@ -19,8 +19,8 @@ /** Abstract Syntax Tree parser object for "on_delete_clause" token */ public class ASTon_delete_clause extends SimpleNode { - static final String ON_DELETE_CASCADE = "ON DELETE CASCADE"; - static final String ON_DELETE_NO_ACTION = "ON DELETE NO ACTION"; + public static final String ON_DELETE_CASCADE = "ON DELETE CASCADE"; + public static final String ON_DELETE_NO_ACTION = "ON DELETE NO ACTION"; public ASTon_delete_clause(int id) { super(id); diff --git a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTtable_interleave_clause.java b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTtable_interleave_clause.java index 38e3916..6e9dc47 100644 --- a/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTtable_interleave_clause.java +++ b/src/main/java/com/google/cloud/solutions/spannerddl/parser/ASTtable_interleave_clause.java @@ -47,13 +47,29 @@ public boolean hasParentKeyword() { return false; } + public boolean hasOnDeleteClause() { + for (Node child : children) { + if (child instanceof ASTon_delete_clause) { + return true; + } + } + return false; + } + public String getOnDelete() { for (Node child : children) { if (child instanceof ASTon_delete_clause) { + if (!hasParentKeyword()) { + throw new IllegalArgumentException( + "ON DELETE clause requires INTERLEAVE IN PARENT syntax"); + } return child.toString(); } } - return ASTon_delete_clause.ON_DELETE_NO_ACTION; + if (hasParentKeyword()) { + return ASTon_delete_clause.ON_DELETE_NO_ACTION; + } + return null; } @Override @@ -62,7 +78,7 @@ public String toString() { .skipNulls() .join( "INTERLEAVE IN", - (hasParentKeyword() ? "PARENT" : null), + ((hasParentKeyword() || hasOnDeleteClause()) ? "PARENT" : null), getParentTableName(), getOnDelete()); } diff --git a/src/test/java/com/google/cloud/solutions/spannerddl/diff/DdlDiffTest.java b/src/test/java/com/google/cloud/solutions/spannerddl/diff/DdlDiffTest.java index fe24c43..b573a30 100644 --- a/src/test/java/com/google/cloud/solutions/spannerddl/diff/DdlDiffTest.java +++ b/src/test/java/com/google/cloud/solutions/spannerddl/diff/DdlDiffTest.java @@ -571,9 +571,21 @@ public void generateDifferences_alterLocalityGroupOptions() throws DdlDiffExcept public void alterTable_interleaveOnDeleteChange_generatesAlter() throws DdlDiffException { String original = "CREATE TABLE c (k INT64) PRIMARY KEY (k), INTERLEAVE IN PARENT p ON DELETE NO ACTION;"; - String updated = "CREATE TABLE c (k INT64) PRIMARY KEY (k), INTERLEAVE IN p ON DELETE CASCADE;"; + String updated = + "CREATE TABLE c (k INT64) PRIMARY KEY (k), INTERLEAVE IN PARENT p ON DELETE CASCADE;"; assertThat(getDiff(original, updated, true)) - .containsExactly("ALTER TABLE c SET INTERLEAVE IN p ON DELETE CASCADE"); + .containsExactly("ALTER TABLE c SET INTERLEAVE IN PARENT p ON DELETE CASCADE"); + } + + @Test + public void alterTable_interleaveLegacyToCascade_generatesTwoStatements() throws DdlDiffException { + String original = "CREATE TABLE c (k INT64) PRIMARY KEY (k), INTERLEAVE IN p;"; + String updated = + "CREATE TABLE c (k INT64) PRIMARY KEY (k), INTERLEAVE IN PARENT p ON DELETE CASCADE;"; + assertThat(getDiff(original, updated, true)) + .containsExactly( + "ALTER TABLE c SET INTERLEAVE IN PARENT p ON DELETE NO ACTION", + "ALTER TABLE c SET INTERLEAVE IN PARENT p ON DELETE CASCADE"); } @Test @@ -581,18 +593,18 @@ public void alterTable_interleaveParentChange_generatesAlter() throws DdlDiffExc String original = "CREATE TABLE c (k INT64) PRIMARY KEY (k), INTERLEAVE IN PARENT p1 ON DELETE CASCADE;"; String updated = - "CREATE TABLE c (k INT64) PRIMARY KEY (k), INTERLEAVE IN p2 ON DELETE CASCADE;"; + "CREATE TABLE c (k INT64) PRIMARY KEY (k), INTERLEAVE IN PARENT p2 ON DELETE CASCADE;"; assertThat(getDiff(original, updated, true)) - .containsExactly("ALTER TABLE c SET INTERLEAVE IN p2 ON DELETE CASCADE"); + .containsExactly("ALTER TABLE c SET INTERLEAVE IN PARENT p2 ON DELETE CASCADE"); } @Test public void alterTable_interleaveAdded_generatesAlter() throws DdlDiffException { String original = "CREATE TABLE c (k INT64) PRIMARY KEY (k);"; String updated = - "CREATE TABLE c (k INT64) PRIMARY KEY (k), INTERLEAVE IN p ON DELETE NO ACTION;"; + "CREATE TABLE c (k INT64) PRIMARY KEY (k), INTERLEAVE IN p;"; assertThat(getDiff(original, updated, true)) - .containsExactly("ALTER TABLE c SET INTERLEAVE IN p ON DELETE NO ACTION"); + .containsExactly("ALTER TABLE c SET INTERLEAVE IN p"); } @Test diff --git a/src/test/java/com/google/cloud/solutions/spannerddl/parser/DDLParserTest.java b/src/test/java/com/google/cloud/solutions/spannerddl/parser/DDLParserTest.java index d2ca1a7..7da79b8 100644 --- a/src/test/java/com/google/cloud/solutions/spannerddl/parser/DDLParserTest.java +++ b/src/test/java/com/google/cloud/solutions/spannerddl/parser/DDLParserTest.java @@ -180,16 +180,15 @@ public void doNotNormalizeSQLExpressions() throws ParseException { @Test public void parseCreateTable_interleaveWithoutParent_parsesAndNormalizes() throws ParseException { - // Input omits PARENT, parser should accept and normalize to include PARENT in toString + // Input omits PARENT, parser should accept and preserve omission in toString ASTcreate_table_statement stmt = (ASTcreate_table_statement) parse("CREATE TABLE t (k INT64) PRIMARY KEY (k), INTERLEAVE IN parent_table") .jjtGetChild(0); - // Output preserves omission of PARENT when not provided + // Output preserves omission of PARENT and does not append an ON DELETE clause assertThat(stmt.toString()) - .isEqualTo( - "CREATE TABLE t ( k INT64 ) PRIMARY KEY (k ASC), INTERLEAVE IN parent_table ON DELETE NO ACTION"); + .isEqualTo("CREATE TABLE t ( k INT64 ) PRIMARY KEY (k ASC), INTERLEAVE IN parent_table"); } @Test @@ -266,7 +265,7 @@ public void parseCreateTable_withInterleaveRowDeletion_andTableOptions() throws .isEqualTo( ("CREATE TABLE child_table ( k1 STRING(MAX) NOT NULL, k2 STRING(MAX) NOT NULL, col1" + " STRING(MAX), col2 INT64, ts TIMESTAMP ) PRIMARY KEY (k1 ASC, k2 ASC)," - + " INTERLEAVE IN parent_table ON DELETE NO ACTION, ROW DELETION POLICY (OLDER_THAN (" + + " INTERLEAVE IN parent_table, ROW DELETION POLICY (OLDER_THAN (" + " ts, INTERVAL 1 DAY )), OPTIONS (opt1=TRUE)") .replaceAll("\\s+", " ")); @@ -275,6 +274,19 @@ public void parseCreateTable_withInterleaveRowDeletion_andTableOptions() throws assertThat(stmt).isEqualTo(stmt2); } + @Test + public void interleaveOnDeleteWithoutParent_throws() throws ParseException { + ASTcreate_table_statement stmt = + (ASTcreate_table_statement) + parse( + "CREATE TABLE child_table (k INT64) PRIMARY KEY (k), INTERLEAVE IN parent_table" + + " ON DELETE CASCADE") + .jjtGetChild(0); + + ASTtable_interleave_clause clause = stmt.getInterleaveClause().get(); + assertThrows(IllegalArgumentException.class, clause::toString); + } + private static void parseCheckingParseException(String ddlStatement, String exceptionContains) { ParseException e = assertThrows(ParseException.class, () -> parseAndVerifyToString(ddlStatement));