From 3eab41727baefbd5f7ffd6b69215d7c37fcdf951 Mon Sep 17 00:00:00 2001 From: Sachin Agarwal Date: Tue, 18 Aug 2026 10:12:38 +0000 Subject: [PATCH] Add Foreign Key ON DELETE SET NULL support for Cloud Spanner Avro template --- .../cloud/teleport/spanner/ddl/ForeignKey.java | 5 ++++- .../spanner/AvroSchemaToDdlConverterTest.java | 15 +++++++++++++-- .../google/cloud/teleport/spanner/CopyDbIT.java | 2 ++ .../spanner/DdlToAvroSchemaConverterTest.java | 9 ++++++++- .../cloud/teleport/spanner/ddl/DdlTest.java | 17 ++++++++++++++--- .../teleport/spanner/ddl/ForeignKeyTest.java | 5 ++++- .../spanner/ddl/RandomDdlGenerator.java | 16 ++++++++++++---- 7 files changed, 57 insertions(+), 12 deletions(-) diff --git a/v1/src/main/java/com/google/cloud/teleport/spanner/ddl/ForeignKey.java b/v1/src/main/java/com/google/cloud/teleport/spanner/ddl/ForeignKey.java index 4db234c2d8..fcb1406427 100644 --- a/v1/src/main/java/com/google/cloud/teleport/spanner/ddl/ForeignKey.java +++ b/v1/src/main/java/com/google/cloud/teleport/spanner/ddl/ForeignKey.java @@ -38,9 +38,9 @@ public enum ReferentialAction { // Supported actions ON_DELETE_NO_ACTION("ON DELETE NO ACTION"), ON_DELETE_CASCADE("ON DELETE CASCADE"), + ON_DELETE_SET_NULL("ON DELETE SET NULL"), // Currently unsupported actions, listed here for completeness ON_DELETE_RESTRICT("ON DELETE RESTRICT"), - ON_DELETE_SET_NULL("ON DELETE SET NULL"), ON_DELETE_SET_DEFAULT("ON DELETE SET DEFAULT"), ON_UPDATE_NO_ACTION("ON UPDATE NO ACTION"), ON_UPDATE_CASCADE("ON UPDATE CASCADE"), @@ -69,6 +69,8 @@ public static ReferentialAction getReferentialAction(String changeType, String a return ReferentialAction.ON_DELETE_CASCADE; case "NO ACTION": return ReferentialAction.ON_DELETE_NO_ACTION; + case "SET NULL": + return ReferentialAction.ON_DELETE_SET_NULL; default: throw new IllegalArgumentException( "ON DELETE referential action not supported: " + action); @@ -131,6 +133,7 @@ private void prettyPrint(Appendable appendable) throws IOException { switch (action.get()) { case ON_DELETE_CASCADE: case ON_DELETE_NO_ACTION: + case ON_DELETE_SET_NULL: appendable.append(" " + action.get().getSqlString()); break; default: diff --git a/v1/src/test/java/com/google/cloud/teleport/spanner/AvroSchemaToDdlConverterTest.java b/v1/src/test/java/com/google/cloud/teleport/spanner/AvroSchemaToDdlConverterTest.java index 582019e6fb..58fd48268c 100644 --- a/v1/src/test/java/com/google/cloud/teleport/spanner/AvroSchemaToDdlConverterTest.java +++ b/v1/src/test/java/com/google/cloud/teleport/spanner/AvroSchemaToDdlConverterTest.java @@ -248,9 +248,12 @@ public void simple() { + " \"ALTER TABLE `Users` ADD CONSTRAINT `fk_odc` FOREIGN KEY (`last_name`) " + " REFERENCES `AllowedNames` (`last_name`) ON DELETE CASCADE\"," + " \"spannerForeignKey_2\" : " + + " \"ALTER TABLE `Users` ADD CONSTRAINT `fk_odsn` FOREIGN KEY (`last_name`) " + + " REFERENCES `AllowedNames` (`last_name`) ON DELETE SET NULL\"," + + " \"spannerForeignKey_3\" : " + " \"ALTER TABLE `Users` ADD CONSTRAINT `fk_not_enforced_no_action` FOREIGN KEY (`last_name`) " + " REFERENCES `AllowedNames` (`last_name`) ON DELETE NO ACTION NOT ENFORCED\"," - + " \"spannerForeignKey_3\" : " + + " \"spannerForeignKey_4\" : " + " \"ALTER TABLE `Users` ADD CONSTRAINT `fk_enforced` FOREIGN KEY (`last_name`) " + " REFERENCES `AllowedNames` (`last_name`) ENFORCED\"," + " \"spannerCheckConstraint_0\" : " @@ -318,6 +321,9 @@ public void simple() { + " ALTER TABLE `Users` ADD CONSTRAINT `fk_odc`" + " FOREIGN KEY (`last_name`) REFERENCES " + "`AllowedNames` (`last_name`) ON DELETE CASCADE" + + " ALTER TABLE `Users` ADD CONSTRAINT `fk_odsn`" + + " FOREIGN KEY (`last_name`) REFERENCES " + + "`AllowedNames` (`last_name`) ON DELETE SET NULL" + " ALTER TABLE `Users` ADD CONSTRAINT `fk_not_enforced_no_action`" + " FOREIGN KEY (`last_name`) REFERENCES " + "`AllowedNames` (`last_name`) ON DELETE NO ACTION NOT ENFORCED" @@ -544,6 +550,9 @@ public void pgSimple() { + " \"spannerForeignKey_1\" : \"ALTER TABLE \\\"Users\\\" ADD CONSTRAINT " + "\\\"fk_odc\\\" FOREIGN KEY (\\\"last_name\\\") REFERENCES \\\"AllowedNames\\\"" + " (\\\"last_name\\\") ON DELETE CASCADE\", " + + " \"spannerForeignKey_2\" : \"ALTER TABLE \\\"Users\\\" ADD CONSTRAINT " + + "\\\"fk_odsn\\\" FOREIGN KEY (\\\"last_name\\\") REFERENCES \\\"AllowedNames\\\"" + + " (\\\"last_name\\\") ON DELETE SET NULL\", " + " \"spannerCheckConstraint_0\" : \"CONSTRAINT \\\"ck\\\"" + " CHECK(\\\"first_name\\\" != \\\"last_name\\\")\"}"; @@ -606,7 +615,9 @@ public void pgSimple() { + " ALTER TABLE \"Users\" ADD CONSTRAINT \"fk\" FOREIGN KEY (\"first_name\")" + " REFERENCES \"AllowedNames\" (\"first_name\")" + " ALTER TABLE \"Users\" ADD CONSTRAINT \"fk_odc\" FOREIGN KEY (\"last_name\")" - + " REFERENCES \"AllowedNames\" (\"last_name\") ON DELETE CASCADE")); + + " REFERENCES \"AllowedNames\" (\"last_name\") ON DELETE CASCADE" + + " ALTER TABLE \"Users\" ADD CONSTRAINT \"fk_odsn\" FOREIGN KEY (\"last_name\")" + + " REFERENCES \"AllowedNames\" (\"last_name\") ON DELETE SET NULL")); } @Test diff --git a/v1/src/test/java/com/google/cloud/teleport/spanner/CopyDbIT.java b/v1/src/test/java/com/google/cloud/teleport/spanner/CopyDbIT.java index dc45f0e473..c1af9b5540 100644 --- a/v1/src/test/java/com/google/cloud/teleport/spanner/CopyDbIT.java +++ b/v1/src/test/java/com/google/cloud/teleport/spanner/CopyDbIT.java @@ -299,6 +299,8 @@ private void createAndPopulate(String sqlFile, Dialect dialect, int numBatches) } } + // TODO(b/550212069): Add integration test for Foreign Key ON DELETE SET NULL action after + // the feature release. @Test public void testAllSchemaAndDataGsql() throws Exception { createAndPopulate( diff --git a/v1/src/test/java/com/google/cloud/teleport/spanner/DdlToAvroSchemaConverterTest.java b/v1/src/test/java/com/google/cloud/teleport/spanner/DdlToAvroSchemaConverterTest.java index 3a946dfea7..278642e192 100644 --- a/v1/src/test/java/com/google/cloud/teleport/spanner/DdlToAvroSchemaConverterTest.java +++ b/v1/src/test/java/com/google/cloud/teleport/spanner/DdlToAvroSchemaConverterTest.java @@ -209,6 +209,8 @@ public void simple() { + " REFERENCES `AllowedNames` (`first_name`)", "ALTER TABLE `Users` ADD CONSTRAINT `fk_odc` FOREIGN KEY (`last_name`)" + " REFERENCES `AllowedNames` (`last_name`) ON DELETE CASCADE", + "ALTER TABLE `Users` ADD CONSTRAINT `fk_odsn` FOREIGN KEY (`last_name`)" + + " REFERENCES `AllowedNames` (`last_name`) ON DELETE SET NULL", "ALTER TABLE `Users` ADD CONSTRAINT `fk_not_enforced_no_action`" + " FOREIGN KEY (`last_name`) REFERENCES " + "`AllowedNames` (`last_name`) ON DELETE NO ACTION NOT ENFORCED", @@ -395,11 +397,16 @@ public void simple() { + " REFERENCES `AllowedNames` (`last_name`) ON DELETE CASCADE")); assertThat( avroSchema.getProp(SPANNER_FOREIGN_KEY + "2"), + equalTo( + "ALTER TABLE `Users` ADD CONSTRAINT `fk_odsn` FOREIGN KEY (`last_name`)" + + " REFERENCES `AllowedNames` (`last_name`) ON DELETE SET NULL")); + assertThat( + avroSchema.getProp(SPANNER_FOREIGN_KEY + "3"), equalTo( "ALTER TABLE `Users` ADD CONSTRAINT `fk_not_enforced_no_action` FOREIGN KEY (`last_name`)" + " REFERENCES `AllowedNames` (`last_name`) ON DELETE NO ACTION NOT ENFORCED")); assertThat( - avroSchema.getProp(SPANNER_FOREIGN_KEY + "3"), + avroSchema.getProp(SPANNER_FOREIGN_KEY + "4"), equalTo( "ALTER TABLE `Users` ADD CONSTRAINT `fk_enforced` FOREIGN KEY (`last_name`)" + " REFERENCES `AllowedNames` (`last_name`) ENFORCED")); diff --git a/v1/src/test/java/com/google/cloud/teleport/spanner/ddl/DdlTest.java b/v1/src/test/java/com/google/cloud/teleport/spanner/ddl/DdlTest.java index 480bf82c34..d76b4cc528 100644 --- a/v1/src/test/java/com/google/cloud/teleport/spanner/ddl/DdlTest.java +++ b/v1/src/test/java/com/google/cloud/teleport/spanner/ddl/DdlTest.java @@ -1764,21 +1764,32 @@ public void testForeignKeyBuilderActions() { ForeignKey fkWithDeleteNoAction = fkWithDeleteNoActionBuilder.build(); assertTrue(fkWithDeleteNoAction.equals(fkWithDeleteNoAction)); assertFalse(fkWithDeleteCascade1.equals(fkWithDeleteNoAction)); + + ForeignKey.Builder fkWithDeleteSetNullBuilder = + ForeignKey.builder().name("fk_odsn").table("Users").referencedTable("AllowedNames"); + fkWithDeleteSetNullBuilder.columnsBuilder().add("first_name", "last_name"); + fkWithDeleteSetNullBuilder.referencedColumnsBuilder().add("first_name", "last_name"); + fkWithDeleteSetNullBuilder.referentialAction( + Optional.of(ReferentialAction.ON_DELETE_SET_NULL)); + ForeignKey fkWithDeleteSetNull = fkWithDeleteSetNullBuilder.build(); + assertTrue(fkWithDeleteSetNull.equals(fkWithDeleteSetNull)); + assertFalse(fkWithDeleteCascade1.equals(fkWithDeleteSetNull)); + assertFalse(fkWithDeleteNoAction.equals(fkWithDeleteSetNull)); } @Test public void testUnsupportedForeignKeyBuilderActionThrowsError() { ForeignKey.Builder fkWithUnsupportedActionBuilder = - ForeignKey.builder().name("fk_odsn").table("Users").referencedTable("AllowedNames"); + ForeignKey.builder().name("fk_odr").table("Users").referencedTable("AllowedNames"); fkWithUnsupportedActionBuilder.columnsBuilder().add("first_name", "last_name"); fkWithUnsupportedActionBuilder.referencedColumnsBuilder().add("first_name", "last_name"); fkWithUnsupportedActionBuilder.referentialAction( - Optional.of(ReferentialAction.ON_DELETE_SET_NULL)); + Optional.of(ReferentialAction.ON_DELETE_RESTRICT)); ForeignKey fkWithUnsupportedAction = fkWithUnsupportedActionBuilder.build(); Throwable exception = assertThrows(IllegalArgumentException.class, () -> fkWithUnsupportedAction.prettyPrint()); assertThat(exception.getMessage()) - .matches("Foreign Key action not supported: ON DELETE SET NULL"); + .matches("Foreign Key action not supported: ON DELETE RESTRICT"); } @Test diff --git a/v1/src/test/java/com/google/cloud/teleport/spanner/ddl/ForeignKeyTest.java b/v1/src/test/java/com/google/cloud/teleport/spanner/ddl/ForeignKeyTest.java index 214c3ba0c0..8ccc0d934a 100644 --- a/v1/src/test/java/com/google/cloud/teleport/spanner/ddl/ForeignKeyTest.java +++ b/v1/src/test/java/com/google/cloud/teleport/spanner/ddl/ForeignKeyTest.java @@ -28,13 +28,16 @@ public final class ForeignKeyTest { @Test - public void testParsingOnDeleteCascadeActions() { + public void testParsingOnDeleteActions() { var onDeleteCascadeAction = ReferentialAction.getReferentialAction("DELETE", "CASCADE"); assertThat(onDeleteCascadeAction) .isEquivalentAccordingToCompareTo(ReferentialAction.ON_DELETE_CASCADE); var onDeleteNoAction = ReferentialAction.getReferentialAction("DELETE", "no action"); assertThat(onDeleteNoAction) .isEquivalentAccordingToCompareTo(ReferentialAction.ON_DELETE_NO_ACTION); + var onDeleteSetNullAction = ReferentialAction.getReferentialAction("DELETE", "set null"); + assertThat(onDeleteSetNullAction) + .isEquivalentAccordingToCompareTo(ReferentialAction.ON_DELETE_SET_NULL); } @Test diff --git a/v1/src/test/java/com/google/cloud/teleport/spanner/ddl/RandomDdlGenerator.java b/v1/src/test/java/com/google/cloud/teleport/spanner/ddl/RandomDdlGenerator.java index a63e5ef59e..5d5cd469af 100644 --- a/v1/src/test/java/com/google/cloud/teleport/spanner/ddl/RandomDdlGenerator.java +++ b/v1/src/test/java/com/google/cloud/teleport/spanner/ddl/RandomDdlGenerator.java @@ -571,7 +571,9 @@ private void generateTable(Ddl.Builder builder, Table parent, int level) { } if (rnd.nextBoolean()) { ReferentialAction action = generateRandomReferentialAction(rnd); - if (!isEnforced && action == ReferentialAction.ON_DELETE_CASCADE) { + if (!isEnforced + && (action == ReferentialAction.ON_DELETE_CASCADE + || action == ReferentialAction.ON_DELETE_SET_NULL)) { action = ReferentialAction.ON_DELETE_NO_ACTION; } foreignKeyBuilder.referentialAction(Optional.of(action)); @@ -625,9 +627,15 @@ private void generateTable(Ddl.Builder builder, Table parent, int level) { } private ReferentialAction generateRandomReferentialAction(Random rnd) { - return rnd.nextBoolean() - ? ReferentialAction.ON_DELETE_CASCADE - : ReferentialAction.ON_DELETE_NO_ACTION; + int actionChoice = rnd.nextInt(3); + switch (actionChoice) { + case 0: + return ReferentialAction.ON_DELETE_CASCADE; + case 1: + return ReferentialAction.ON_DELETE_SET_NULL; + default: + return ReferentialAction.ON_DELETE_NO_ACTION; + } } private String addDefaultValueToColumn(Type type) {