From b2275f5708ab675170d0c35a63efc83b5f818191 Mon Sep 17 00:00:00 2001 From: Yingjian Wu Date: Tue, 2 Jul 2024 11:26:19 -0400 Subject: [PATCH] improve rename logic to use 2 phase commit idea --- .../ParentChildRelMetadataService.java | 12 +- .../netflix/metacat/MetacatSmokeSpec.groovy | 279 ++++++++++++++---- .../ParentChildRelMetadataServiceSpec.groovy | 213 ++++++++++++- .../main/services/impl/TableServiceImpl.java | 35 ++- .../services/impl/TableServiceImplSpec.groovy | 22 +- .../MySqlParentChildRelMetaDataService.java | 156 ++++++++-- 6 files changed, 605 insertions(+), 112 deletions(-) diff --git a/metacat-common-server/src/main/java/com/netflix/metacat/common/server/usermetadata/ParentChildRelMetadataService.java b/metacat-common-server/src/main/java/com/netflix/metacat/common/server/usermetadata/ParentChildRelMetadataService.java index b8e6220a2..40d9bf208 100644 --- a/metacat-common-server/src/main/java/com/netflix/metacat/common/server/usermetadata/ParentChildRelMetadataService.java +++ b/metacat-common-server/src/main/java/com/netflix/metacat/common/server/usermetadata/ParentChildRelMetadataService.java @@ -3,6 +3,8 @@ import com.netflix.metacat.common.dto.notifications.ChildInfoDto; import com.netflix.metacat.common.server.model.ChildInfo; import com.netflix.metacat.common.server.model.ParentInfo; +import org.apache.commons.lang3.tuple.Pair; +import java.util.Optional; import java.util.Set; @@ -60,8 +62,11 @@ void deleteParentChildRelation( * * @param oldName the current name to be renamed * @param newName the new name to rename to + * @return return a pair of set, + * where the first set represents the affected parent_uuid with name = oldName + * and the second set represents the affected child_uuid with name = oldName */ - void rename( + Pair, Set> rename( QualifiedName oldName, QualifiedName newName ); @@ -71,10 +76,13 @@ void rename( * This involves two steps: * 1. drop all records where the child column = `name` * 2. drop all records where the parent column = `name` + * * Note if uuids are specified, it is going to drop the table with that name with the corresponding uuids * @param name the name of the entity to drop + * @param uuids the uuids to drop, where the first pair is the parent uuid and second pair is the child uuid */ void drop( - QualifiedName name + QualifiedName name, + Optional, Set>> uuids ); /** diff --git a/metacat-functional-tests/src/functionalTest/groovy/com/netflix/metacat/MetacatSmokeSpec.groovy b/metacat-functional-tests/src/functionalTest/groovy/com/netflix/metacat/MetacatSmokeSpec.groovy index 9ccfd1e70..c8364add9 100644 --- a/metacat-functional-tests/src/functionalTest/groovy/com/netflix/metacat/MetacatSmokeSpec.groovy +++ b/metacat-functional-tests/src/functionalTest/groovy/com/netflix/metacat/MetacatSmokeSpec.groovy @@ -1992,12 +1992,15 @@ class MetacatSmokeSpec extends Specification { def parent1UUID = "p1_uuid" def renameParent1 = "rename_parent1" def parent1Uri = isLocalEnv ? String.format('file:/tmp/%s/%s', databaseName, parent1) : null + def renameParent1Uri = isLocalEnv ? String.format('file:/tmp/%s/%s', databaseName, parent1) : null def parent1FullName = catalogName + "/" + databaseName + "/" + parent1 + def renameParent1FullName = catalogName + "/" + databaseName + "/" + renameParent1 def child11 = "child11" def child11UUID = "c11_uuid" def renameChild11 = "rename_child11" def child11Uri = isLocalEnv ? String.format('file:/tmp/%s/%s', databaseName, child11) : null + def renameChild11Uri = isLocalEnv ? String.format('file:/tmp/%s/%s', databaseName, renameChild11) : null def child11FullName = catalogName + "/" + databaseName + "/" + child11 def child12 = "child12" @@ -2043,7 +2046,6 @@ class MetacatSmokeSpec extends Specification { def parent1Table = api.getTable(catalogName, databaseName, parent1, true, true, false) def child11Table = api.getTable(catalogName, databaseName, child11, true, true, false) - def child11ParentChildRelationInfo = child11Table.definitionMetadata.get("parentChildRelationInfo") then: // Test Parent 1 parentChildInfo assert parent1Table.definitionMetadata.get("parentChildRelationInfo").get("isParent").booleanValue() @@ -2052,7 +2054,7 @@ class MetacatSmokeSpec extends Specification { // Test Child11 parentChildInfo assert !child11Table.definitionMetadata.get("parentChildRelationInfo").has("isParent") assert child11Table.definitionMetadata.get("random_key").asText() == "random_value" - JSONAssert.assertEquals(child11ParentChildRelationInfo.toString(), + JSONAssert.assertEquals(child11Table.definitionMetadata.get("parentChildRelationInfo").toString(), '{"parentInfos":[{"name":"embedded-fast-hive-metastore/iceberg_db/parent1","relationType":"CLONE", "uuid":"p1_uuid"}]}', false) assert parentChildRelV1.getChildren(catalogName, databaseName, child11).isEmpty() @@ -2069,9 +2071,20 @@ class MetacatSmokeSpec extends Specification { assert e.message.contains("Cannot have a child table having more than one parent") /* - Step 3: create another table with the same name different uuid without specifying the parent child relation + Step 3: create another table with the same child1 name but different uuid under the a different parent should fail + */ + when: + child11TableDto = PigDataDtoProvider.getTable(catalogName, databaseName, child11, 'amajumdar', child11Uri) + initializeParentChildRelDefinitionMetadata(child11TableDto, parent2FullName, parent2UUID, "random_uuid") + api.createTable(catalogName, databaseName, child11, child11TableDto) + then: + e = thrown(Exception) + assert e.message.contains("Cannot have a child table having more than one parent") + + /* + Step 4: create another table with the same name different uuid without specifying the parent child relation but should fail because the table already exists - This test the revert should not impact the previous record + This test the revert during creation should not impact the previous record */ when: child11TableDto = PigDataDtoProvider.getTable(catalogName, databaseName, child11, 'amajumdar', child11Uri) @@ -2080,6 +2093,10 @@ class MetacatSmokeSpec extends Specification { e = thrown(Exception) assert e.message.contains("already exists") + when: + parent1Table = api.getTable(catalogName, databaseName, parent1, true, true, false) + child11Table = api.getTable(catalogName, databaseName, child11, true, true, false) + then: // Test Parent 1 parentChildInfo assert parent1Table.definitionMetadata.get("parentChildRelationInfo").get("isParent").booleanValue() assert parentChildRelV1.getChildren(catalogName, databaseName, parent1) == [new ChildInfoDto("embedded-fast-hive-metastore/iceberg_db/child11", "CLONE", "c11_uuid")] as Set @@ -2087,14 +2104,14 @@ class MetacatSmokeSpec extends Specification { // Test Child11 parentChildInfo assert !child11Table.definitionMetadata.get("parentChildRelationInfo").has("isParent") assert child11Table.definitionMetadata.get("random_key").asText() == "random_value" - JSONAssert.assertEquals(child11ParentChildRelationInfo.toString(), + JSONAssert.assertEquals(child11Table.definitionMetadata.get("parentChildRelationInfo").toString(), '{"parentInfos":[{"name":"embedded-fast-hive-metastore/iceberg_db/parent1","relationType":"CLONE", "uuid":"p1_uuid"}]}', false) assert parentChildRelV1.getChildren(catalogName, databaseName, child11).isEmpty() /* - Step 4: Create a second child (child12) pointing to parent = parent1 + Step 5: Create a second child (child12) pointing to parent = parent1 */ when: // Create Child2 Table @@ -2103,7 +2120,6 @@ class MetacatSmokeSpec extends Specification { api.createTable(catalogName, databaseName, child12, child12TableDto) parent1Table = api.getTable(catalogName, databaseName, parent1, true, true, false) def child12Table = api.getTable(catalogName, databaseName, child12, true, true, false) - def child12ParentChildRelationInfo = child12Table.definitionMetadata.get("parentChildRelationInfo") then: // Test Parent 1 parentChildInfo @@ -2115,13 +2131,13 @@ class MetacatSmokeSpec extends Specification { // Test Child12 parentChildInfo assert !child12Table.definitionMetadata.get("parentChildRelationInfo").has("isParent") - JSONAssert.assertEquals(child12ParentChildRelationInfo.toString(), + JSONAssert.assertEquals(child12Table.definitionMetadata.get("parentChildRelationInfo").toString(), '{"parentInfos":[{"name":"embedded-fast-hive-metastore/iceberg_db/parent1","relationType":"CLONE","uuid":"p1_uuid"}]}', false) assert parentChildRelV1.getChildren(catalogName, databaseName, child12).isEmpty() /* - Step 5: create a parent table on top of another parent table should fail + Step 6: create a parent table on top of another parent table should fail */ when: def grandParent1TableDto = PigDataDtoProvider.getTable(catalogName, databaseName, grandParent1, 'amajumdar', grantParent1Uri) @@ -2134,41 +2150,14 @@ class MetacatSmokeSpec extends Specification { e = thrown(Exception) assert e.message.contains("Cannot create a parent table on top of another parent") - /* - Step 6: create another table with the same parent1 name but should fail because the table already exists - Test the revert should not impact the previous record - */ - when: - api.createTable(catalogName, databaseName, parent1, parent1TableDto) - then: - e = thrown(Exception) - assert e.message.contains("already exists") - - // Test Parent 1 parentChildInfo - assert parent1Table.definitionMetadata.get("parentChildRelationInfo").get("isParent").booleanValue() - assert parentChildRelV1.getChildren(catalogName, databaseName, parent1) == [ - new ChildInfoDto("embedded-fast-hive-metastore/iceberg_db/child11", "CLONE", "c11_uuid"), - new ChildInfoDto("embedded-fast-hive-metastore/iceberg_db/child12", "CLONE", "c12_uuid") - ] as Set - - // Test Child11 parentChildInfo - assert !child11Table.definitionMetadata.get("parentChildRelationInfo").has("isParent") - assert child11Table.definitionMetadata.get("random_key").asText() == "random_value" - JSONAssert.assertEquals(child11ParentChildRelationInfo.toString(), - '{"parentInfos":[{"name":"embedded-fast-hive-metastore/iceberg_db/parent1","relationType":"CLONE", "uuid":"p1_uuid"}]}', - false) - assert parentChildRelV1.getChildren(catalogName, databaseName, child11).isEmpty() - /* Step 7: Create one grandChild As a Parent of A child table should fail */ when: def grandchild121TableDto = PigDataDtoProvider.getTable(catalogName, databaseName, grandChild121, 'amajumdar', null) initializeParentChildRelDefinitionMetadata(grandchild121TableDto, child11FullName, child11UUID, grandChild121UUID) - api.createTable(catalogName, databaseName, grandChild121, grandchild121TableDto) assert parentChildRelV1.getChildren(catalogName, databaseName, grandChild121).isEmpty() - then: e = thrown(Exception) assert e.message.contains("Cannot create a child table as parent") @@ -2187,7 +2176,6 @@ class MetacatSmokeSpec extends Specification { api.createTable(catalogName, databaseName, child21, child21TableDto) def parent2Table = api.getTable(catalogName, databaseName, parent2, true, true, false) def child21Table = api.getTable(catalogName, databaseName, child21, true, true, false) - def child21ParentChildRelationInfo = child21Table.definitionMetadata.get("parentChildRelationInfo") then: // Test Parent 2 parentChildInfo @@ -2197,21 +2185,107 @@ class MetacatSmokeSpec extends Specification { ] as Set // Test Child21 parentChildInfo - JSONAssert.assertEquals(child21ParentChildRelationInfo.toString(), + JSONAssert.assertEquals(child21Table.definitionMetadata.get("parentChildRelationInfo").toString(), '{"parentInfos":[{"name":"embedded-fast-hive-metastore/iceberg_db/parent2","relationType":"CLONE","uuid":"p2_uuid"}]}', false) assert parentChildRelV1.getChildren(catalogName, databaseName, child21).isEmpty() /* - Step 9: Rename parent1 to newParent1 + Step 9: Create a table newParent1 without any parent child rel info + and attempt to rename parent1 to newParent1 should fail + Test the parentChildRelationship record remain the same after the revert */ when: + def newParent1TableDto = PigDataDtoProvider.getTable(catalogName, databaseName, renameParent1, 'amajumdar', renameParent1Uri) + api.createTable(catalogName, databaseName, renameParent1, newParent1TableDto) + api.renameTable(catalogName, databaseName, parent1, renameParent1) + then: + e = thrown(Exception) + assert e.message.contains("already exists") + + when: + parent1Table = api.getTable(catalogName, databaseName, parent1, true, true, false) + child11Table = api.getTable(catalogName, databaseName, child11, true, true, false) + child12Table = api.getTable(catalogName, databaseName, child12, true, true, false) + + then: + // Test Parent 1 parentChildInfo + assert parent1Table.definitionMetadata.get("parentChildRelationInfo").get("isParent").booleanValue() + assert parentChildRelV1.getChildren(catalogName, databaseName, parent1) == [ + new ChildInfoDto("embedded-fast-hive-metastore/iceberg_db/child11", "CLONE", "c11_uuid"), + new ChildInfoDto("embedded-fast-hive-metastore/iceberg_db/child12", "CLONE", "c12_uuid") + ] as Set + // Test Child11 parentChildInfo + assert !child11Table.definitionMetadata.get("parentChildRelationInfo").has("isParent") + assert child11Table.definitionMetadata.get("random_key").asText() == "random_value" + JSONAssert.assertEquals(child11Table.definitionMetadata.get("parentChildRelationInfo").toString(), + '{"parentInfos":[{"name":"embedded-fast-hive-metastore/iceberg_db/parent1","relationType":"CLONE", "uuid":"p1_uuid"}]}', + false) + assert parentChildRelV1.getChildren(catalogName, databaseName, child11).isEmpty() + // Test Child12 parentChildInfo + assert !child12Table.definitionMetadata.get("parentChildRelationInfo").has("isParent") + JSONAssert.assertEquals(child12Table.definitionMetadata.get("parentChildRelationInfo").toString(), + '{"parentInfos":[{"name":"embedded-fast-hive-metastore/iceberg_db/parent1","relationType":"CLONE","uuid":"p1_uuid"}]}', + false) + assert parentChildRelV1.getChildren(catalogName, databaseName, child12).isEmpty() + + /* + Step 10: Attempt to rename parent1 to parent2 wbich has parent child relationship and should fail + Test the parentChildRelationship record remain the same after the revert + */ + when: + api.renameTable(catalogName, databaseName, parent1, parent2) + then: + e = thrown(Exception) + assert e.message.contains("already exists in parent child relationship service") + + when: + parent1Table = api.getTable(catalogName, databaseName, parent1, true, true, false) + child11Table = api.getTable(catalogName, databaseName, child11, true, true, false) + child12Table = api.getTable(catalogName, databaseName, child12, true, true, false) + parent2Table = api.getTable(catalogName, databaseName, parent2, true, true, false) + child21Table = api.getTable(catalogName, databaseName, child21, true, true, false) + then: + // Test Parent 1 parentChildInfo + assert parent1Table.definitionMetadata.get("parentChildRelationInfo").get("isParent").booleanValue() + assert parentChildRelV1.getChildren(catalogName, databaseName, parent1) == [ + new ChildInfoDto("embedded-fast-hive-metastore/iceberg_db/child11", "CLONE", "c11_uuid"), + new ChildInfoDto("embedded-fast-hive-metastore/iceberg_db/child12", "CLONE", "c12_uuid") + ] as Set + // Test Child11 parentChildInfo + assert !child11Table.definitionMetadata.get("parentChildRelationInfo").has("isParent") + assert child11Table.definitionMetadata.get("random_key").asText() == "random_value" + JSONAssert.assertEquals(child11Table.definitionMetadata.get("parentChildRelationInfo").toString(), + '{"parentInfos":[{"name":"embedded-fast-hive-metastore/iceberg_db/parent1","relationType":"CLONE", "uuid":"p1_uuid"}]}', + false) + assert parentChildRelV1.getChildren(catalogName, databaseName, child11).isEmpty() + // Test Child12 parentChildInfo + assert !child12Table.definitionMetadata.get("parentChildRelationInfo").has("isParent") + JSONAssert.assertEquals(child12Table.definitionMetadata.get("parentChildRelationInfo").toString(), + '{"parentInfos":[{"name":"embedded-fast-hive-metastore/iceberg_db/parent1","relationType":"CLONE","uuid":"p1_uuid"}]}', + false) + assert parentChildRelV1.getChildren(catalogName, databaseName, child12).isEmpty() + // Test Parent 2 parentChildInfo + assert parent2Table.definitionMetadata.get("parentChildRelationInfo").get("isParent").booleanValue() + assert parentChildRelV1.getChildren(catalogName, databaseName, parent2) == [ + new ChildInfoDto("embedded-fast-hive-metastore/iceberg_db/child21", "CLONE", "c21_uuid") + ] as Set + // Test Child21 parentChildInfo + JSONAssert.assertEquals(child21Table.definitionMetadata.get("parentChildRelationInfo").toString(), + '{"parentInfos":[{"name":"embedded-fast-hive-metastore/iceberg_db/parent2","relationType":"CLONE","uuid":"p2_uuid"}]}', + false) + assert parentChildRelV1.getChildren(catalogName, databaseName, child21).isEmpty() + + + /* + Step 11: First drop the newParent1 and then Rename parent1 to newParent1 should now succeed + */ + when: + api.deleteTable(catalogName, databaseName, renameParent1) api.renameTable(catalogName, databaseName, parent1, renameParent1) parent1Table = api.getTable(catalogName, databaseName, renameParent1, true, true, false) child11Table = api.getTable(catalogName, databaseName, child11, true, true, false) - child11ParentChildRelationInfo = child11Table.definitionMetadata.get("parentChildRelationInfo") child12Table = api.getTable(catalogName, databaseName, child12, true, true, false) - child12ParentChildRelationInfo = child12Table.definitionMetadata.get("parentChildRelationInfo") then: // Test Parent 1 parentChildInfo newName @@ -2223,14 +2297,14 @@ class MetacatSmokeSpec extends Specification { ] as Set // Test Child11 parentChildInfo assert !child11Table.definitionMetadata.get("parentChildRelationInfo").has("isParent") - JSONAssert.assertEquals(child11ParentChildRelationInfo.toString(), + JSONAssert.assertEquals(child11Table.definitionMetadata.get("parentChildRelationInfo").toString(), '{"parentInfos":[{"name":"embedded-fast-hive-metastore/iceberg_db/rename_parent1","relationType":"CLONE","uuid":"p1_uuid"}]}', false) assert parentChildRelV1.getChildren(catalogName, databaseName, child11).isEmpty() // Test Child12 parentChildInfo assert !child12Table.definitionMetadata.get("parentChildRelationInfo").has("isParent") - JSONAssert.assertEquals(child12ParentChildRelationInfo.toString(), + JSONAssert.assertEquals(child12Table.definitionMetadata.get("parentChildRelationInfo").toString(), '{"parentInfos":[{"name":"embedded-fast-hive-metastore/iceberg_db/rename_parent1","relationType":"CLONE","uuid":"p1_uuid"}]}', false) assert parentChildRelV1.getChildren(catalogName, databaseName, child12).isEmpty() @@ -2243,13 +2317,102 @@ class MetacatSmokeSpec extends Specification { assert e.message.contains("Unable to locate for") /* - Step 10: Rename child11 to renameChild11 + Step 12: Create a table renameChild11 without parent childInfo and then try to rename child11 to renameChild11, which should fail + This test to make sure the revert works properly. + */ + when: + def newChild1TableDto = PigDataDtoProvider.getTable(catalogName, databaseName, renameChild11, 'amajumdar', renameChild11Uri) + api.createTable(catalogName, databaseName, renameChild11, newChild1TableDto) + api.renameTable(catalogName, databaseName, child11, renameChild11) + + then: + e = thrown(Exception) + assert e.message.contains("already exists") + + when: + parent1Table = api.getTable(catalogName, databaseName, renameParent1, true, true, false) + child11Table = api.getTable(catalogName, databaseName, child11, true, true, false) + child12Table = api.getTable(catalogName, databaseName, child12, true, true, false) + then: + // Test Parent 1 parentChildInfo newName + assert parent1Table.definitionMetadata.get("parentChildRelationInfo").get("isParent").booleanValue() + assert parentChildRelV1.getChildren(catalogName, databaseName, renameParent1) == + [ + new ChildInfoDto("embedded-fast-hive-metastore/iceberg_db/child11", "CLONE", "c11_uuid"), + new ChildInfoDto("embedded-fast-hive-metastore/iceberg_db/child12", "CLONE", "c12_uuid") + ] as Set + // Test Child11 parentChildInfo + assert !child11Table.definitionMetadata.get("parentChildRelationInfo").has("isParent") + JSONAssert.assertEquals(child11Table.definitionMetadata.get("parentChildRelationInfo").toString(), + '{"parentInfos":[{"name":"embedded-fast-hive-metastore/iceberg_db/rename_parent1","relationType":"CLONE","uuid":"p1_uuid"}]}', + false) + assert parentChildRelV1.getChildren(catalogName, databaseName, child11).isEmpty() + + // Test Child12 parentChildInfo + assert !child12Table.definitionMetadata.get("parentChildRelationInfo").has("isParent") + JSONAssert.assertEquals(child12Table.definitionMetadata.get("parentChildRelationInfo").toString(), + '{"parentInfos":[{"name":"embedded-fast-hive-metastore/iceberg_db/rename_parent1","relationType":"CLONE","uuid":"p1_uuid"}]}', + false) + assert parentChildRelV1.getChildren(catalogName, databaseName, child12).isEmpty() + + /* + Step 13: Create a table renameChild11 with parent childInfo and then try to rename child11 to renameChild11, which should fail + This test to make sure the revert works properly. */ when: + api.deleteTable(catalogName, databaseName, renameChild11) + newChild1TableDto = PigDataDtoProvider.getTable(catalogName, databaseName, renameChild11, 'amajumdar', renameChild11Uri) + initializeParentChildRelDefinitionMetadata(newChild1TableDto, renameParent1FullName, parent1UUID, "random_uuid") + api.createTable(catalogName, databaseName, renameChild11, newChild1TableDto) + api.renameTable(catalogName, databaseName, child11, renameChild11) + + then: + e = thrown(Exception) + assert e.message.contains("already exists in parent child relationship service") + + when: + def renameChild11Table = api.getTable(catalogName, databaseName, renameChild11, true, true, false) + parent1Table = api.getTable(catalogName, databaseName, renameParent1, true, true, false) + child11Table = api.getTable(catalogName, databaseName, child11, true, true, false) + child12Table = api.getTable(catalogName, databaseName, child12, true, true, false) + then: + // Test Parent 1 parentChildInfo newName + assert parent1Table.definitionMetadata.get("parentChildRelationInfo").get("isParent").booleanValue() + assert parentChildRelV1.getChildren(catalogName, databaseName, renameParent1) == + [ + new ChildInfoDto("embedded-fast-hive-metastore/iceberg_db/child11", "CLONE", "c11_uuid"), + new ChildInfoDto("embedded-fast-hive-metastore/iceberg_db/child12", "CLONE", "c12_uuid"), + new ChildInfoDto("embedded-fast-hive-metastore/iceberg_db/rename_child11", "CLONE", "random_uuid") + ] as Set + // Test Child11 parentChildInfo + assert !child11Table.definitionMetadata.get("parentChildRelationInfo").has("isParent") + JSONAssert.assertEquals(child11Table.definitionMetadata.get("parentChildRelationInfo").toString(), + '{"parentInfos":[{"name":"embedded-fast-hive-metastore/iceberg_db/rename_parent1","relationType":"CLONE","uuid":"p1_uuid"}]}', + false) + assert parentChildRelV1.getChildren(catalogName, databaseName, child11).isEmpty() + // Test Child12 parentChildInfo + assert !child12Table.definitionMetadata.get("parentChildRelationInfo").has("isParent") + JSONAssert.assertEquals(child12Table.definitionMetadata.get("parentChildRelationInfo").toString(), + '{"parentInfos":[{"name":"embedded-fast-hive-metastore/iceberg_db/rename_parent1","relationType":"CLONE","uuid":"p1_uuid"}]}', + false) + assert parentChildRelV1.getChildren(catalogName, databaseName, child12).isEmpty() + // Test renameChild11Table parentChildInfo + assert !renameChild11Table.definitionMetadata.get("parentChildRelationInfo").has("isParent") + JSONAssert.assertEquals(renameChild11Table.definitionMetadata.get("parentChildRelationInfo").toString(), + '{"parentInfos":[{"name":"embedded-fast-hive-metastore/iceberg_db/rename_parent1","relationType":"CLONE","uuid":"p1_uuid"}]}', + false) + assert parentChildRelV1.getChildren(catalogName, databaseName, renameChild11).isEmpty() + + + /* + Step 14: Drop previous renameChild11 and Rename child11 to renameChild11 should now succeed + */ + when: + api.deleteTable(catalogName, databaseName, renameChild11) api.renameTable(catalogName, databaseName, child11, renameChild11) parent1Table = api.getTable(catalogName, databaseName, renameParent1, true, true, false) child11Table = api.getTable(catalogName, databaseName, renameChild11, true, true, false) - child11ParentChildRelationInfo = child11Table.definitionMetadata.get("parentChildRelationInfo") + then: // Test parent1Table parentChildInfo with newName assert parent1Table.definitionMetadata.get("parentChildRelationInfo").get("isParent").booleanValue() @@ -2260,7 +2423,7 @@ class MetacatSmokeSpec extends Specification { // Test Child11 parentChildInfo with newName assert !child11Table.definitionMetadata.get("parentChildRelationInfo").has("isParent") assert child11Table.definitionMetadata.get("random_key").asText() == "random_value" - JSONAssert.assertEquals(child11ParentChildRelationInfo.toString(), + JSONAssert.assertEquals(child11Table.definitionMetadata.get("parentChildRelationInfo").toString(), '{"parentInfos":[{"name":"embedded-fast-hive-metastore/iceberg_db/rename_parent1","relationType":"CLONE","uuid":"p1_uuid"}]}', false) assert parentChildRelV1.getChildren(catalogName, databaseName, renameChild11).isEmpty() @@ -2273,7 +2436,7 @@ class MetacatSmokeSpec extends Specification { assert e.message.contains("Unable to locate for") /* - Step 11: Drop parent renameParent1 + Step 15 Drop parent renameParent1 */ when: api.deleteTable(catalogName, databaseName, renameParent1) @@ -2283,7 +2446,7 @@ class MetacatSmokeSpec extends Specification { assert e.message.contains("because it still has") /* - Step 8: Drop renameChild11 should succeed + Step 16: Drop renameChild11 should succeed */ when: api.deleteTable(catalogName, databaseName, renameChild11) @@ -2297,7 +2460,7 @@ class MetacatSmokeSpec extends Specification { ] as Set /* - Step 12: Create renameChild11 and should expect random_key should appear at it is reattached + Step 17: Create renameChild11 and should expect random_key should appear at it is reattached but parent childInfo should not as it is always coming from the parent child relationship service which currently does not have any record */ @@ -2305,6 +2468,7 @@ class MetacatSmokeSpec extends Specification { child11TableDto = PigDataDtoProvider.getTable(catalogName, databaseName, renameChild11, 'amajumdar', child11Uri) api.createTable(catalogName, databaseName, renameChild11, child11TableDto) child11Table = api.getTable(catalogName, databaseName, renameChild11, true, true, false) + parent1Table = api.getTable(catalogName, databaseName, renameParent1, true, true, false) then: assert !child11Table.definitionMetadata.has("parentChildRelationInfo") assert child11Table.definitionMetadata.get("random_key").asText() == "random_value" @@ -2317,7 +2481,7 @@ class MetacatSmokeSpec extends Specification { ] as Set /* - Step 13: Drop child12 should succeed + Step 18: Drop child12 should succeed */ when: api.deleteTable(catalogName, databaseName, child12) @@ -2327,13 +2491,12 @@ class MetacatSmokeSpec extends Specification { assert parentChildRelV1.getChildren(catalogName, databaseName, renameParent1).isEmpty() /* - Step 14: Drop renameParent1 should succeed as there is no more child under it + Step 19: Drop renameParent1 should succeed as there is no more child under it */ when: api.deleteTable(catalogName, databaseName, renameParent1) parent2Table = api.getTable(catalogName, databaseName, parent2, true, true, false) child21Table = api.getTable(catalogName, databaseName, child21, true, true, false) - child21ParentChildRelationInfo = child21Table.definitionMetadata.get("parentChildRelationInfo") then: // Since all the operations above are on the first connected relationship, the second connected relationship @@ -2346,13 +2509,13 @@ class MetacatSmokeSpec extends Specification { // Test Child21 parentChildInfo assert !child21Table.definitionMetadata.get("parentChildRelationInfo").has("isParent") - JSONAssert.assertEquals(child21ParentChildRelationInfo.toString(), + JSONAssert.assertEquals(child21Table.definitionMetadata.get("parentChildRelationInfo").toString(), '{"parentInfos":[{"name":"embedded-fast-hive-metastore/iceberg_db/parent2","relationType":"CLONE","uuid":"p2_uuid"}]}', false) assert parentChildRelV1.getChildren(catalogName, databaseName, child21).isEmpty() /* - Step 15: update parent2 with random parentChildRelationInfo to test immutability + Step 20: update parent2 with random parentChildRelationInfo to test immutability */ when: def updateParent2Dto = parent2Table @@ -2361,7 +2524,6 @@ class MetacatSmokeSpec extends Specification { parent2Table = api.getTable(catalogName, databaseName, parent2, true, true, false) child21Table = api.getTable(catalogName, databaseName, child21, true, true, false) - child21ParentChildRelationInfo = child21Table.definitionMetadata.get("parentChildRelationInfo") then: assert parent2Table.definitionMetadata.get("parentChildRelationInfo").get("isParent").booleanValue() assert parentChildRelV1.getChildren(catalogName, databaseName, parent2) == [ @@ -2370,13 +2532,13 @@ class MetacatSmokeSpec extends Specification { // Test Child21 parentChildInfo assert !child21Table.definitionMetadata.get("parentChildRelationInfo").has("isParent") - JSONAssert.assertEquals(child21ParentChildRelationInfo.toString(), + JSONAssert.assertEquals(child21Table.definitionMetadata.get("parentChildRelationInfo").toString(), '{"parentInfos":[{"name":"embedded-fast-hive-metastore/iceberg_db/parent2","relationType":"CLONE","uuid":"p2_uuid"}]}', false) assert parentChildRelV1.getChildren(catalogName, databaseName, child21).isEmpty() /* - Step 16: update child21 with random parentChildRelationInfo to test immutability + Step 21: update child21 with random parentChildRelationInfo to test immutability */ when: def updateChild21Dto = child21Table @@ -2385,7 +2547,6 @@ class MetacatSmokeSpec extends Specification { parent2Table = api.getTable(catalogName, databaseName, parent2, true, true, false) child21Table = api.getTable(catalogName, databaseName, child21, true, true, false) - child21ParentChildRelationInfo = child21Table.definitionMetadata.get("parentChildRelationInfo") then: // Test Parent 2 parentChildInfo assert parent2Table.definitionMetadata.get("parentChildRelationInfo").get("isParent").booleanValue() @@ -2395,7 +2556,7 @@ class MetacatSmokeSpec extends Specification { // Test Child21 parentChildInfo assert !child21Table.definitionMetadata.get("parentChildRelationInfo").has("isParent") - JSONAssert.assertEquals(child21ParentChildRelationInfo.toString(), + JSONAssert.assertEquals(child21Table.definitionMetadata.get("parentChildRelationInfo").toString(), '{"parentInfos":[{"name":"embedded-fast-hive-metastore/iceberg_db/parent2","relationType":"CLONE","uuid":"p2_uuid"}]}', false) assert parentChildRelV1.getChildren(catalogName, databaseName, child21).isEmpty() diff --git a/metacat-functional-tests/src/functionalTest/groovy/com/netflix/metacat/ParentChildRelMetadataServiceSpec.groovy b/metacat-functional-tests/src/functionalTest/groovy/com/netflix/metacat/ParentChildRelMetadataServiceSpec.groovy index 99f0d0284..8c14e5b1f 100644 --- a/metacat-functional-tests/src/functionalTest/groovy/com/netflix/metacat/ParentChildRelMetadataServiceSpec.groovy +++ b/metacat-functional-tests/src/functionalTest/groovy/com/netflix/metacat/ParentChildRelMetadataServiceSpec.groovy @@ -29,6 +29,16 @@ class ParentChildRelMetadataServiceSpec extends Specification{ @Shared private String database = "testpc" + @Shared + static final String SQL_CREATE_PARENT_CHILD_RELATIONS = + "INSERT INTO parent_child_relation (parent, parent_uuid, child, child_uuid, relation_type) " + + "VALUES (?, ?, ?, ?, ?)" + + private void insertNewParentChildRecord(final String pName, final String pUUID, + final String child, final String childUUID, final String type) { + jdbcTemplate.update(SQL_CREATE_PARENT_CHILD_RELATIONS, pName, pUUID, child, childUUID, type) + } + def setupSpec() { String jdbcUrl = "jdbc:mysql://localhost:3306/metacat" String username = "metacat_user" @@ -197,9 +207,13 @@ class ParentChildRelMetadataServiceSpec extends Specification{ service.createParentChildRelation(parent, parentUUID, child, childUUID, type) when: - service.rename(parent, newParent) + def pair = service.rename(parent, newParent) + service.drop(parent, Optional.of(pair)) then: + assert pair.getLeft().size() == 1 + assert pair.getRight().isEmpty() + // Test Old Parent Name assert service.getParents(parent).isEmpty() assert service.getChildren(parent).isEmpty() @@ -216,10 +230,13 @@ class ParentChildRelMetadataServiceSpec extends Specification{ // rename back when: - service.rename(newParent, parent) + pair = service.rename(newParent, parent) + service.drop(newParent, Optional.of(pair)) child_parent_expected = [new ParentInfo(parent.toString(), type, parentUUID)] as Set then: + assert pair.getLeft().size() == 1 + assert pair.getRight().isEmpty() // Test new Parent Name assert service.getParents(newParent).isEmpty() assert service.getChildren(newParent).isEmpty() @@ -246,11 +263,22 @@ class ParentChildRelMetadataServiceSpec extends Specification{ service.createParentChildRelation(parent, parentUUID, child2, child2UUID, type) def newParent = QualifiedName.ofTable(catalog, database, "np") def child_parent_expected = [new ParentInfo(newParent.toString(), type, parentUUID)] as Set + def newParent_children_expected = [ + new ChildInfo(child1.toString(), type, child1UUID), + new ChildInfo(child2.toString(), type, child2UUID) + ] as Set when: - service.rename(parent, newParent) + def pair = service.rename(parent, newParent) + service.drop(parent, Optional.of(pair)) then: + assert pair.getLeft().size() == 1 + assert pair.getRight().isEmpty() + + // Test newParent + assert service.getParents(newParent).isEmpty() + assert service.getChildren(newParent) == newParent_children_expected // Test Child1 assert service.getParents(child1) == child_parent_expected //Test Child2 @@ -268,8 +296,12 @@ class ParentChildRelMetadataServiceSpec extends Specification{ def newChild = QualifiedName.ofTable(catalog, database, "nc") when: - service.rename(child, newChild) + def pair = service.rename(child, newChild) + service.drop(child, Optional.of(pair)) + then: + assert pair.getLeft().isEmpty() + assert pair.getRight().size() == 1 // Test Parent assert service.getParents(parent).isEmpty() def parent_children_expected = [new ChildInfo(newChild.toString(), type, childUUID)] as Set @@ -286,10 +318,13 @@ class ParentChildRelMetadataServiceSpec extends Specification{ // rename back when: - service.rename(newChild, child) + pair = service.rename(newChild, child) + service.drop(newChild, Optional.of(pair)) parent_children_expected = [new ChildInfo(child.toString(), type, childUUID)] as Set then: + assert pair.getLeft().isEmpty() + assert pair.getRight().size() == 1 // Test Parent assert service.getParents(parent).isEmpty() assert parent_children_expected == service.getChildren(parent) @@ -313,7 +348,7 @@ class ParentChildRelMetadataServiceSpec extends Specification{ def type = "clone"; service.createParentChildRelation(parent, parentUUID, child, childUUID, type) when: - service.drop(child) + service.drop(child, Optional.empty()) then: // Test Parent @@ -336,8 +371,9 @@ class ParentChildRelMetadataServiceSpec extends Specification{ service.createParentChildRelation(parent, parentUUID, child, childUUID, type) when: - service.rename(child, newChild) - service.drop(newChild) + def pair = service.rename(child, newChild) + service.drop(child, Optional.of(pair)) + service.drop(newChild, Optional.empty()) then: // Test Parent @@ -352,4 +388,165 @@ class ParentChildRelMetadataServiceSpec extends Specification{ assert service.getParents(newChild).isEmpty() assert service.getChildren(newChild).isEmpty() } + def "Test Parent Rename failed and remove newName"() { + setup: + def parent = QualifiedName.ofTable(catalog, database, "p") + def parentUUID = "p_uuid" + def child = QualifiedName.ofTable(catalog, database, "c") + def childUUID = "c_uuid" + def type = "clone"; + def newParent = QualifiedName.ofTable(catalog, database, "np") + service.createParentChildRelation(parent, parentUUID, child, childUUID, type) + + when: + def pair = service.rename(parent, newParent) + service.drop(newParent, Optional.of(pair)) + + then: + // Test Old Parent Name + assert service.getParents(newParent).isEmpty() + assert service.getChildren(newParent).isEmpty() + + // Test New Parent Name + assert service.getParents(parent).isEmpty() + def newParent_children_expected = [new ChildInfo(child.toString(), type, childUUID)] as Set + assert service.getChildren(parent) == newParent_children_expected + + // Test Child + def child_parent_expected = [new ParentInfo(parent.toString(), type, parentUUID)] as Set + assert child_parent_expected == service.getParents(child) + assert service.getChildren(child).isEmpty() + } + + def "Test Child Rename failed and remove newName"() { + setup: + def parent = QualifiedName.ofTable(catalog, database, "p") + def parentUUID = "p_uuid" + def child = QualifiedName.ofTable(catalog, database, "c") + def childUUID = "c_uuid" + def type = "clone" + service.createParentChildRelation(parent, parentUUID, child, childUUID, type) + def newChild = QualifiedName.ofTable(catalog, database, "nc") + + when: + def pair = service.rename(child, newChild) + service.drop(newChild, Optional.of(pair)) + then: + // Test Parent + assert service.getParents(parent).isEmpty() + def parent_children_expected = [new ChildInfo(child.toString(), type, childUUID)] as Set + assert parent_children_expected == service.getChildren(parent) + + // Test New Child + assert service.getParents(newChild).isEmpty() + assert service.getChildren(newChild).isEmpty() + + // Test Child + def child_parent_expected = [new ParentInfo(parent.toString(), type, parentUUID)] as Set + assert child_parent_expected == service.getParents(child) + assert service.getChildren(child).isEmpty() + } + + def "Test rename to an existing table in the parent child rel service"() { + given: + def parent = QualifiedName.ofTable(catalog, database, "p") + def parentUUID = "p_uuid" + def child = QualifiedName.ofTable(catalog, database, "c") + def childUUID = "c_uuid" + def type = "clone" + service.createParentChildRelation(parent, parentUUID, child, childUUID, type) + def newParentQualifiedName = QualifiedName.ofTable(catalog, database, "np_name") + def newChildQualifiedName = QualifiedName.ofTable(catalog, database, "nc_name") + insertNewParentChildRecord(newParentQualifiedName.toString(), "np_uuid", newChildQualifiedName.toString(), "nc_uuid", "random") + + when: + service.rename(parent, newParentQualifiedName) + then: + def e = thrown(RuntimeException) + assert e.message.contains("already exists") + + when: + service.rename(child, newChildQualifiedName) + + then: + e = thrown(RuntimeException) + assert e.message.contains("already exists") + } + + // Add a test where between rename and drop old/new name with the same name but different uuid is added + // only those get from rename should be drop + // This case is not possible in reality but wanted to add a test to prove this + def "Simulate Rename child: drop only impacted uuids"() { + setup: + def parent = QualifiedName.ofTable(catalog, database, "p") + def parentUUID = "p_uuid" + def child = QualifiedName.ofTable(catalog, database, "c") + def childUUID = "c_uuid" + def type = "clone" + service.createParentChildRelation(parent, parentUUID, child, childUUID, type) + def newChild = QualifiedName.ofTable(catalog, database, "nc") + + when: + def pair = service.rename(child, newChild) + def child_parent_expected = [new ParentInfo(parent.toString(), type, parentUUID)] as Set + // Add a record with the same child name but different uuid and since this record is added after rename + // this record should not be dropped during the service.drop + insertNewParentChildRecord(parent.toString(), parentUUID, child.toString(), "c_uuid2", type) + service.drop(child, Optional.of(pair)) + then: + // Test Parent + assert service.getParents(parent).isEmpty() + def parent_children_expected = [new ChildInfo(newChild.toString(), type, childUUID), + new ChildInfo(child.toString(), type, "c_uuid2")] as Set + assert parent_children_expected == service.getChildren(parent) + + // Test new child + assert service.getParents(newChild) == child_parent_expected + assert service.getChildren(newChild).isEmpty() + + // Test old child + assert service.getParents(child) == child_parent_expected + assert service.getChildren(child).isEmpty() + } + + // Add a test where between rename and drop old/new name same name but different uuid is added + // only those get from rename should be drop + // This case is not possible in reality but wanted to add a test to prove this + def "Simulate Rename Parent: drop only impacted uuids"() { + setup: + def parent = QualifiedName.ofTable(catalog, database, "p") + def parentUUID = "p_uuid" + def child = QualifiedName.ofTable(catalog, database, "c") + def childUUID = "c_uuid" + def type = "clone" + service.createParentChildRelation(parent, parentUUID, child, childUUID, type) + def newParent = QualifiedName.ofTable(catalog, database, "np") + + when: + def pair = service.rename(parent, newParent) + // Add a record with the same parent name but different uuid and since this record is added after rename + // this record should not be dropped during the service.drop + insertNewParentChildRecord(parent.toString(), "p_uuid2", child.toString(), "c_uuid1", "clone") + service.drop(parent, Optional.of(pair)) + then: + + // Test p + assert service.getParents(parent).isEmpty() + def parent_children_expected = [new ChildInfo(child.toString(), "clone", "c_uuid1")] as Set + assert parent_children_expected == service.getChildren(parent) + + // Test np + assert service.getParents(newParent).isEmpty() + def new_parent_children_expected = [new ChildInfo(child.toString(), "clone", childUUID)] as Set + assert new_parent_children_expected == service.getChildren(newParent) + + // Test child + assert service.getChildren((child)).isEmpty() + def child_parent_expected = [ + new ParentInfo(newParent.toString(), "clone", parentUUID), + new ParentInfo(parent.toString(), "clone", "p_uuid2"), + ] as Set + assert service.getParents(child) == child_parent_expected + } + } diff --git a/metacat-main/src/main/java/com/netflix/metacat/main/services/impl/TableServiceImpl.java b/metacat-main/src/main/java/com/netflix/metacat/main/services/impl/TableServiceImpl.java index 32a6bc8b8..d64693839 100644 --- a/metacat-main/src/main/java/com/netflix/metacat/main/services/impl/TableServiceImpl.java +++ b/metacat-main/src/main/java/com/netflix/metacat/main/services/impl/TableServiceImpl.java @@ -69,6 +69,8 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; +import org.apache.commons.lang3.tuple.Pair; + import javax.annotation.Nullable; import java.util.List; import java.util.Map; @@ -398,7 +400,7 @@ public TableDto deleteAndReturn(final QualifiedName name, final boolean isMView) } try { - parentChildRelMetadataService.drop(name); + parentChildRelMetadataService.drop(name, Optional.empty()); } catch (Exception e) { log.error("parentChildRelMetadataService: Failed to drop relation for table={}", name, e); } @@ -568,23 +570,38 @@ public void rename( //Ignore if the operation is not supported, so that we can at least go ahead and save the user metadata eventBus.post(new MetacatRenameTablePreEvent(oldName, metacatRequestContext, this, newName)); - // Before rename, first rename its parent child relation - parentChildRelMetadataService.rename(oldName, newName); - + // Before rename, first + // duplicate record with newName in parent child relation and get all uuids associated with this newName + // that are impacted so later during + // either dropping the oldName after successful connectorTableServiceProxy.rename + // or dropping the newName after failed connectorTableServiceProxy.rename + // we only touch records with those specific uuids + final Pair, Set> parentChildUuidsPair = + parentChildRelMetadataService.rename(oldName, newName); try { connectorTableServiceProxy.rename(oldName, newName, isMView); } catch (Exception e) { try { - // if rename operation fail, rename back the parent child relation - parentChildRelMetadataService.rename(newName, oldName); + // if rename operation fail, remove the soft inserted record + parentChildRelMetadataService.drop(newName, Optional.of(parentChildUuidsPair)); } catch (Exception renameException) { - log.error("parentChildRelMetadataService: Failed to rename parent child relation " + log.error("parentChildRelMetadataService: Fail to rename parent child relation " + "after table fail to rename from {} to {} " - + "with the following parameters oldName={} to newName={}", - oldName, newName, oldName, newName, renameException); + + "with the following parameters oldName={} to newName={} and uuid pairs = {}", + oldName, newName, oldName, newName, parentChildUuidsPair, renameException); } throw e; } + // if rename everything succeeds, remove records with the oldName + try { + parentChildRelMetadataService.drop(oldName, Optional.of(parentChildUuidsPair)); + } catch (Exception e) { + log.error("parentChildRelMetadataService: Fail to rename parent child relation " + + "after table successfully rename from {} to {} " + + "with the following parameters oldName={} to newName={} and uuid pairs = {}", + oldName, newName, oldName, newName, parentChildUuidsPair, e); + } + userMetadataService.renameDefinitionMetadataKey(oldName, newName); tagService.renameTableTags(oldName, newName.getTableName()); diff --git a/metacat-main/src/test/groovy/com/netflix/metacat/main/services/impl/TableServiceImplSpec.groovy b/metacat-main/src/test/groovy/com/netflix/metacat/main/services/impl/TableServiceImplSpec.groovy index be3366222..164755472 100644 --- a/metacat-main/src/test/groovy/com/netflix/metacat/main/services/impl/TableServiceImplSpec.groovy +++ b/metacat-main/src/test/groovy/com/netflix/metacat/main/services/impl/TableServiceImplSpec.groovy @@ -53,7 +53,8 @@ import com.netflix.spectator.api.DefaultRegistry import com.netflix.spectator.api.NoopRegistry import spock.lang.Specification import spock.lang.Unroll -import com.netflix.metacat.common.server.usermetadata.ParentChildRelMetadataService; +import com.netflix.metacat.common.server.usermetadata.ParentChildRelMetadataService +import org.apache.commons.lang3.tuple.Pair /** * Tests for the TableServiceImpl. @@ -306,19 +307,32 @@ class TableServiceImplSpec extends Specification { thrown(RuntimeException) } - def "Test Rename - Clone Table Fail to update parent child relation"() { + def "Test Rename for clone"() { given: def oldName = QualifiedName.ofTable("clone", "clone", "oldChild") def newName = QualifiedName.ofTable("clone", "clone", "newChild") + def parentUuids = ["uuid1"] as Set + def childUuids = [""] as Set + def pair = Pair.of(parentUuids, childUuids) + when: service.rename(oldName, newName, false) then: 1 * config.getNoTableRenameOnTags() >> [] - 1 * parentChildRelSvc.rename(oldName, newName) + 1 * parentChildRelSvc.rename(oldName, newName) >> pair 1 * connectorTableServiceProxy.rename(oldName, newName, _) >> {throw new RuntimeException("Fail to rename")} - 1 * parentChildRelSvc.rename(newName, oldName) + 1 * parentChildRelSvc.drop(newName, Optional.of(pair)) thrown(RuntimeException) + + when: + service.rename(oldName, newName, false) + then: + 1 * config.getNoTableRenameOnTags() >> [] + 1 * parentChildRelSvc.rename(oldName, newName) >> pair + 1 * connectorTableServiceProxy.rename(oldName, newName, _) + 1 * parentChildRelSvc.drop(oldName, Optional.of(pair)) + noExceptionThrown() } def "Test Drop - Clone Table Fail to drop parent child relation"() { diff --git a/metacat-metadata-mysql/src/main/java/com/netflix/metacat/metadata/mysql/MySqlParentChildRelMetaDataService.java b/metacat-metadata-mysql/src/main/java/com/netflix/metacat/metadata/mysql/MySqlParentChildRelMetaDataService.java index 89f0ef6d6..f36cc9873 100644 --- a/metacat-metadata-mysql/src/main/java/com/netflix/metacat/metadata/mysql/MySqlParentChildRelMetaDataService.java +++ b/metacat-metadata-mysql/src/main/java/com/netflix/metacat/metadata/mysql/MySqlParentChildRelMetaDataService.java @@ -9,17 +9,26 @@ import com.netflix.metacat.common.server.usermetadata.ParentChildRelServiceException; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import lombok.extern.slf4j.Slf4j; +import org.apache.commons.lang3.tuple.ImmutablePair; +import org.apache.commons.lang3.tuple.Pair; +import org.springframework.jdbc.core.PreparedStatementSetter; +import org.springframework.jdbc.core.ResultSetExtractor; +import org.springframework.transaction.annotation.Isolation; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.transaction.annotation.Transactional; import java.sql.PreparedStatement; import java.util.Set; +import java.util.Optional; import java.util.List; import java.util.ArrayList; import java.util.HashSet; import java.util.stream.Collectors; +import java.sql.ResultSet; +import java.sql.SQLException; + /** * Parent Child Relationship Metadata Service. * This stores the parent child relationship of two entities as first class citizen in Metacat. @@ -51,6 +60,25 @@ public class MySqlParentChildRelMetaDataService implements ParentChildRelMetadat static final String SQL_GET_CHILDREN = "SELECT child, child_uuid, relation_type " + "FROM parent_child_relation WHERE parent = ?"; + static final String SQL_RENAME_SOFT_PARENT_INSERT = + "INSERT INTO parent_child_relation (parent, parent_uuid, child, child_uuid, relation_type) " + + "SELECT ?, parent_uuid, child, child_uuid, relation_type " + + "FROM parent_child_relation " + + "WHERE parent = ?"; + static final String SQL_RENAME_SOFT_CHILD_INSERT = + "INSERT INTO parent_child_relation (parent, parent_uuid, child, child_uuid, relation_type) " + + "SELECT parent, parent_uuid, ?, child_uuid, relation_type " + + "FROM parent_child_relation " + + "WHERE child = ?"; + + static final String SQL_CHECK_TABLE_EXIST = + "SELECT 1 FROM parent_child_relation WHERE parent = ? OR child = ? LIMIT 1"; + + static final String SQL_GET_PARENT_UUIDS = "SELECT DISTINCT parent_uuid FROM parent_child_relation " + + "where parent = ?"; + + static final String SQL_GET_CHILDREN_UUIDS = "SELECT DISTINCT child_uuid FROM parent_child_relation " + + "where child = ?"; private final JdbcTemplate jdbcTemplate; private final ConverterUtil converterUtil; @@ -156,37 +184,50 @@ public void deleteParentChildRelation(final QualifiedName parentName, } @Override - public void rename(final QualifiedName oldName, final QualifiedName newName) { - renameParent(oldName, newName); - renameChild(oldName, newName); - log.info("Successfully rename parent child relationship for oldName={}, newName={}", - oldName, newName - ); + @Transactional(isolation = Isolation.SERIALIZABLE) + public Pair, Set> rename(final QualifiedName oldName, final QualifiedName newName) { + if (tableExist(newName.toString())) { + throw new ParentChildRelServiceException(String.format( + "Fail to rename table from %s to %s: since %s already exists in parent child relationship service", + oldName, newName, newName)); + } + + final Set parentUuids = renameParent(oldName, newName); + final Set childUuids = renameChild(oldName, newName); + + if (!parentUuids.isEmpty() || !childUuids.isEmpty()) { + log.info("Successfully rename parent child relationship for oldName={}, newName={}", + oldName, newName + ); + } + return new ImmutablePair<>(parentUuids, childUuids); } - private void renameParent(final QualifiedName oldName, final QualifiedName newName) { + private Set renameParent(final QualifiedName oldName, final QualifiedName newName) { try { jdbcTemplate.update(connection -> { - final PreparedStatement ps = connection.prepareStatement(SQL_RENAME_PARENT_ENTITY); + final PreparedStatement ps = connection.prepareStatement(SQL_RENAME_SOFT_PARENT_INSERT); ps.setString(1, newName.toString()); ps.setString(2, oldName.toString()); return ps; }); + return getParentUuidsForParent(newName.toString()); } catch (Exception e) { throw new ParentChildRelServiceException("Fail to rename parent from oldName:" + oldName + " to " + newName + " in MySqlParentChildRelMetadataService", e); } } - private void renameChild(final QualifiedName oldName, final QualifiedName newName) { + private Set renameChild(final QualifiedName oldName, final QualifiedName newName) { try { jdbcTemplate.update(connection -> { - final PreparedStatement ps = connection.prepareStatement(SQL_RENAME_CHILD_ENTITY); + final PreparedStatement ps = connection.prepareStatement(SQL_RENAME_SOFT_CHILD_INSERT); ps.setString(1, newName.toString()); ps.setString(2, oldName.toString()); return ps; }); + return getChildUuidsForChild(newName.toString()); } catch (Exception e) { throw new ParentChildRelServiceException("Fail to rename child from oldName:" + oldName + " to " + newName + " in MySqlParentChildRelMetadataService", e); @@ -194,35 +235,56 @@ private void renameChild(final QualifiedName oldName, final QualifiedName newNam } @Override - public void drop(final QualifiedName name) { - dropParent(name); - dropChild(name); - log.info("Successfully drop parent child relationship for name={}", name); - } + public void drop(final QualifiedName name, final Optional, Set>> uuids) { + Optional> parentUUIDs = Optional.empty(); + Optional> childUUIDs = Optional.empty(); - private void dropParent(final QualifiedName name) { - try { - jdbcTemplate.update(connection -> { - final PreparedStatement ps = connection.prepareStatement(SQL_DROP_PARENT); - ps.setString(1, name.toString()); - return ps; - }); - } catch (Exception e) { - throw new ParentChildRelServiceException("Fail to drop parent:" + name - + " in MySqlParentChildRelMetadataService", e); + if (uuids.isPresent()) { + parentUUIDs = Optional.of(uuids.get().getLeft()); + childUUIDs = Optional.of(uuids.get().getRight()); + } + + if (drop(name, parentUUIDs, SQL_DROP_PARENT) > 0 || drop(name, childUUIDs, SQL_DROP_CHILD) > 0) { + log.info("Successfully drop parent child relationship for name={} and parentUUIDs = {} and childUUIDs = {}", + name, parentUUIDs, childUUIDs); } } - private void dropChild(final QualifiedName name) { + private int drop(final QualifiedName name, final Optional> uuids, final String sql) { + final String sqlWithInClause; + final String uuidColumn = sql.equals(SQL_DROP_PARENT) ? "parent_uuid" : "child_uuid"; + + if (uuids.isPresent() && !uuids.get().isEmpty()) { + final String inClause = uuids.get().stream() + .map(uuid -> "?") + .collect(Collectors.joining(", ")); + sqlWithInClause = sql + " AND " + uuidColumn + " IN (" + inClause + ")"; + } else { + sqlWithInClause = sql; + } + try { - jdbcTemplate.update(connection -> { - final PreparedStatement ps = connection.prepareStatement(SQL_DROP_CHILD); + return jdbcTemplate.update(connection -> { + final PreparedStatement ps = connection.prepareStatement(sqlWithInClause); ps.setString(1, name.toString()); + + if (uuids.isPresent()) { + int i = 1; + for (String uuid : uuids.get()) { + ps.setString(++i, uuid); + } + } + return ps; }); } catch (Exception e) { - throw new ParentChildRelServiceException("Fail to drop child:" + name - + " in MySqlParentChildRelMetadataService", e); + if (sql.equals(SQL_DROP_PARENT)) { + throw new ParentChildRelServiceException("Fail to drop parent:" + name + + " in MySqlParentChildRelMetadataService", e); + } else { + throw new ParentChildRelServiceException("Fail to drop child:" + name + + " in MySqlParentChildRelMetadataService", e); + } } } @@ -261,4 +323,38 @@ public Set getChildrenDto(final QualifiedName name) { return getChildren(name).stream() .map(converterUtil::toChildInfoDto).collect(Collectors.toSet()); } + + private boolean tableExist(final String tableName) { + return jdbcTemplate.query(SQL_CHECK_TABLE_EXIST, + new PreparedStatementSetter() { + @Override + public void setValues(final PreparedStatement ps) throws SQLException { + ps.setString(1, tableName); + ps.setString(2, tableName); + } + }, + new ResultSetExtractor() { + @Override + public Boolean extractData(final ResultSet rs) throws SQLException { + return rs.next(); + } + } + ); + } + + private Set getParentUuidsForParent(final String parentName) { + return new HashSet<>(jdbcTemplate.query(connection -> { + final PreparedStatement ps = connection.prepareStatement(SQL_GET_PARENT_UUIDS); + ps.setString(1, parentName); + return ps; + }, (rs, rowNum) -> rs.getString("parent_uuid"))); + } + + private Set getChildUuidsForChild(final String childName) { + return new HashSet<>(jdbcTemplate.query(connection -> { + final PreparedStatement ps = connection.prepareStatement(SQL_GET_CHILDREN_UUIDS); + ps.setString(1, childName); + return ps; + }, (rs, rowNum) -> rs.getString("child_uuid"))); + } }