Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
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_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;
Expand Down Expand Up @@ -64,6 +65,8 @@ public static DatabaseDefinition create(List<ASTddl_statement> statements) {
LinkedHashMap<String, ASTcreate_change_stream_statement> changeStreams = new LinkedHashMap<>();
LinkedHashMap<String, String> alterDatabaseOptions = new LinkedHashMap<>();
LinkedHashMap<String, ASTcreate_schema_statement> schemas = new LinkedHashMap<>();
LinkedHashMap<String, ASTcreate_locality_group_statement> localityGroups =
new LinkedHashMap<>();

for (ASTddl_statement ddlStatement : statements) {
final SimpleNode statement = (SimpleNode) ddlStatement.jjtGetChild(0);
Expand Down Expand Up @@ -92,6 +95,10 @@ public static DatabaseDefinition create(List<ASTddl_statement> 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(),
Expand Down Expand Up @@ -157,7 +164,8 @@ public static DatabaseDefinition create(List<ASTddl_statement> statements) {
ImmutableMap.copyOf(ttls),
ImmutableMap.copyOf(changeStreams),
ImmutableMap.copyOf(alterDatabaseOptions),
ImmutableMap.copyOf(schemas));
ImmutableMap.copyOf(schemas),
ImmutableMap.copyOf(localityGroups));
}

public abstract ImmutableMap<String, ASTcreate_table_statement> tablesInCreationOrder();
Expand All @@ -175,4 +183,6 @@ public static DatabaseDefinition create(List<ASTddl_statement> statements) {
abstract ImmutableMap<String, String> alterDatabaseOptions();

abstract ImmutableMap<String, ASTcreate_schema_statement> schemas();

abstract ImmutableMap<String, ASTcreate_locality_group_statement> localityGroups();
}
137 changes: 112 additions & 25 deletions src/main/java/com/google/cloud/solutions/spannerddl/diff/DdlDiff.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,17 @@
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_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;
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;
import com.google.cloud.solutions.spannerddl.parser.DdlParser;
import com.google.cloud.solutions.spannerddl.parser.DdlParserTreeConstants;
import com.google.cloud.solutions.spannerddl.parser.ParseException;
Expand All @@ -54,6 +57,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;
Expand Down Expand Up @@ -103,6 +107,7 @@ public class DdlDiff {
private final MapDifference<String, ASTcreate_search_index_statement> searchIndexDifferences;
private final String databaseName; // for alter Database
private final MapDifference<String, ASTcreate_schema_statement> schemaDifferences;
private final MapDifference<String, ASTcreate_locality_group_statement> localityGroupDifferences;

private DdlDiff(DatabaseDefinition originalDb, DatabaseDefinition newDb, String databaseName)
throws DdlDiffException {
Expand All @@ -122,6 +127,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...
Expand Down Expand Up @@ -197,6 +204,15 @@ public List<String> generateDifferenceStatements(Map<String, Boolean> 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());
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.

output.add("DROP LOCALITY GROUP " + lg.getNameOrDefault());
}
}

// drop deleted search indexes.
if (options.get(ALLOW_DROP_STATEMENTS_OPT)) {
for (String searchIndexName : searchIndexDifferences.entriesOnlyOnLeft().keySet()) {
Expand Down Expand Up @@ -273,6 +289,37 @@ public List<String> generateDifferenceStatements(Map<String, Boolean> options)
generateAlterTableStatements(difference.leftValue(), difference.rightValue(), options));
}

// update existing locality groups (options only)
for (ValueDifference<ASTcreate_locality_group_statement> 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<String, String> leftOptions =
leftOptionsClause == null ? Collections.emptyMap() : leftOptionsClause.getKeyValueMap();
Map<String, String> rightOptions =
rightOptionsClause == null ? Collections.emptyMap() : rightOptionsClause.getKeyValueMap();
MapDifference<String, String> optionsDiff = Maps.difference(leftOptions, rightOptions);

String updateText = generateOptionsUpdates(optionsDiff);
if (!Strings.isNullOrEmpty(updateText)) {
output.add(
String.format(
"ALTER LOCALITY GROUP %s SET OPTIONS (%s)", right.getNameOrDefault(), 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());
Expand Down Expand Up @@ -456,36 +503,62 @@ static List<String> 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)
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.

final Optional<ASTtable_interleave_clause> leftInterleave = left.getInterleaveClause();
final Optional<ASTtable_interleave_clause> rightInterleave = right.getInterleaveClause();

if (leftInterleave.isPresent() != rightInterleave.isPresent()) {
if (rightInterleave.isPresent()) {
// Added interleave
ASTtable_interleave_clause ri = rightInterleave.get();
alterStatements.add(
buildSetInterleaveStatement(
left.getTableName(),
ri.getParentTableName(),
ri.hasParentKeyword(),
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())
|| !Objects.equals(li.getOnDelete(), ri.getOnDelete())
|| li.hasParentKeyword() != ri.hasParentKeyword()) {
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()));
}
}
}

// compare columns.
Expand All @@ -511,6 +584,19 @@ static List<String> 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<String> alterStatements, ValueDifference<ASTcolumn_def> columnDiff)
throws DdlDiffException {
Expand Down Expand Up @@ -815,6 +901,7 @@ public static List<ASTddl_statement> 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:
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -141,6 +142,7 @@ private void validateChildren() {
ASTprimary_key.class,
ASTtable_interleave_clause.class,
ASTrow_deletion_policy_clause.class,
ASToptions_clause.class,
ASTannotation.class));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading