From 0cc0280ed95ad34277e1cd7939ebda553d0d03d1 Mon Sep 17 00:00:00 2001 From: Cliff Frey Date: Tue, 26 Aug 2025 22:17:38 -0700 Subject: [PATCH 1/3] 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/3] 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/3] 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));