diff --git a/metacat-common-server/src/main/java/com/netflix/metacat/common/server/connectors/exception/ConnectorException.java b/metacat-common-server/src/main/java/com/netflix/metacat/common/server/connectors/exception/ConnectorException.java index ed4125774..d45dec070 100644 --- a/metacat-common-server/src/main/java/com/netflix/metacat/common/server/connectors/exception/ConnectorException.java +++ b/metacat-common-server/src/main/java/com/netflix/metacat/common/server/connectors/exception/ConnectorException.java @@ -50,7 +50,7 @@ public ConnectorException(final String message, @Nullable final Throwable cause) * * @param message message * @param cause cause - * @param enableSuppression eable suppression + * @param enableSuppression enable suppression * @param writableStackTrace stacktrace */ public ConnectorException( diff --git a/metacat-common-server/src/main/java/com/netflix/metacat/common/server/connectors/exception/DatabasePreconditionFailedException.java b/metacat-common-server/src/main/java/com/netflix/metacat/common/server/connectors/exception/DatabasePreconditionFailedException.java new file mode 100644 index 000000000..7feb99269 --- /dev/null +++ b/metacat-common-server/src/main/java/com/netflix/metacat/common/server/connectors/exception/DatabasePreconditionFailedException.java @@ -0,0 +1,48 @@ +package com.netflix.metacat.common.server.connectors.exception; + +/* + * + * Copyright 2024 Netflix, Inc. + * + * 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 + * + * http://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. + * + */ + +import com.netflix.metacat.common.QualifiedName; +import lombok.Getter; + +import javax.annotation.Nullable; + +/** + * Exception when database can't be deleted because ON DELETE CASCADE is + * disabled and a table still exists in the database. + * + * @author gtret + */ +@Getter +public class DatabasePreconditionFailedException extends ConnectorException { + /** + * Constructor. + * + * @param name qualified name of the database + * @param message error description + * @param error stacktrace + */ + public DatabasePreconditionFailedException(final QualifiedName name, + @Nullable final String message, + @Nullable final Throwable error) { + super(String.format("Precondition failed to update database %s. %s", name, message), error); + } +} + + diff --git a/metacat-connector-polaris/src/functionalTest/resources/application-polaris_functional_test.yml b/metacat-connector-polaris/src/functionalTest/resources/application-polaris_functional_test.yml index 5c6a59feb..c2c999ff0 100644 --- a/metacat-connector-polaris/src/functionalTest/resources/application-polaris_functional_test.yml +++ b/metacat-connector-polaris/src/functionalTest/resources/application-polaris_functional_test.yml @@ -17,7 +17,7 @@ spring: ddl-auto: none properties: hibernate: - dialect: org.hibernate.dialect.PostgreSQLDialect + dialect: org.hibernate.dialect.PostgreSQLDialect show_sql: true sql: init: diff --git a/metacat-connector-polaris/src/functionalTest/resources/schema.sql b/metacat-connector-polaris/src/functionalTest/resources/schema.sql index aae7a9c23..0f6ce05a6 100644 --- a/metacat-connector-polaris/src/functionalTest/resources/schema.sql +++ b/metacat-connector-polaris/src/functionalTest/resources/schema.sql @@ -24,5 +24,5 @@ create table TBLS ( created_date TIMESTAMP not null, last_updated_by STRING(255), last_updated_date TIMESTAMP not null, - foreign key (db_name) references DBS(name) ON DELETE CASCADE ON UPDATE CASCADE + foreign key (db_name) references DBS(name) ON DELETE RESTRICT ON UPDATE CASCADE ); diff --git a/metacat-connector-polaris/src/main/java/com/netflix/metacat/connector/polaris/PolarisConnectorDatabaseService.java b/metacat-connector-polaris/src/main/java/com/netflix/metacat/connector/polaris/PolarisConnectorDatabaseService.java index 94d67780e..8b5b5eddc 100644 --- a/metacat-connector-polaris/src/main/java/com/netflix/metacat/connector/polaris/PolarisConnectorDatabaseService.java +++ b/metacat-connector-polaris/src/main/java/com/netflix/metacat/connector/polaris/PolarisConnectorDatabaseService.java @@ -10,6 +10,7 @@ import com.netflix.metacat.common.server.connectors.exception.ConnectorException; import com.netflix.metacat.common.server.connectors.exception.DatabaseAlreadyExistsException; import com.netflix.metacat.common.server.connectors.exception.DatabaseNotFoundException; +import com.netflix.metacat.common.server.connectors.exception.DatabasePreconditionFailedException; import com.netflix.metacat.common.server.connectors.exception.InvalidMetaException; import com.netflix.metacat.common.server.connectors.model.DatabaseInfo; import com.netflix.metacat.connector.polaris.common.PolarisUtils; @@ -20,6 +21,7 @@ import org.springframework.dao.DataIntegrityViolationException; import javax.annotation.Nullable; +import java.sql.SQLException; import java.util.ArrayList; import java.util.List; import java.util.stream.Collectors; @@ -87,6 +89,17 @@ public void delete(final ConnectorRequestContext context, final QualifiedName na try { this.polarisStoreService.deleteDatabase(name.getDatabaseName()); } catch (DataIntegrityViolationException exception) { + if (exception.getMessage().contains("violates foreign key constraint") + || (exception.getCause() instanceof SQLException + && "23503".equals(((SQLException) exception.getCause()).getSQLState()))) { + + final String errorMessage = String.format( + "Failed to delete database %s due to foreign key constraint violation. " + + "Ensure all dependent tables are removed first. Error: %s", + name, exception.getMessage() + ); + throw new DatabasePreconditionFailedException(name, errorMessage, exception); + } throw new InvalidMetaException(name, exception); } catch (Exception exception) { throw new ConnectorException( @@ -163,7 +176,7 @@ public List listNames( try { final String dbPrefix = prefix == null ? "" : prefix.getDatabaseName(); final List qualifiedNames = polarisStoreService.getDatabaseNames( - dbPrefix, sort, this.connectorContext.getConfig().getListDatabaseNamesPageSize()) + dbPrefix, sort, this.connectorContext.getConfig().getListDatabaseNamesPageSize()) .stream() .map(dbName -> QualifiedName.ofDatabase(name.getCatalogName(), dbName)) .collect(Collectors.toCollection(ArrayList::new)); diff --git a/metacat-connector-polaris/src/test/java/com/netflix/metacat/connector/polaris/PolarisConnectorDatabaseServiceTest.java b/metacat-connector-polaris/src/test/java/com/netflix/metacat/connector/polaris/PolarisConnectorDatabaseServiceTest.java index e297704f8..5011e0b2b 100644 --- a/metacat-connector-polaris/src/test/java/com/netflix/metacat/connector/polaris/PolarisConnectorDatabaseServiceTest.java +++ b/metacat-connector-polaris/src/test/java/com/netflix/metacat/connector/polaris/PolarisConnectorDatabaseServiceTest.java @@ -1,6 +1,6 @@ - package com.netflix.metacat.connector.polaris; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.Maps; import com.netflix.metacat.common.QualifiedName; import com.netflix.metacat.common.server.connectors.ConnectorContext; @@ -9,9 +9,18 @@ import com.netflix.metacat.common.server.connectors.exception.DatabaseNotFoundException; import com.netflix.metacat.common.server.connectors.model.AuditInfo; import com.netflix.metacat.common.server.connectors.model.DatabaseInfo; +import com.netflix.metacat.common.server.connectors.model.TableInfo; import com.netflix.metacat.common.server.properties.DefaultConfigImpl; import com.netflix.metacat.common.server.properties.MetacatProperties; +import com.netflix.metacat.common.server.util.ThreadServiceManager; +import com.netflix.metacat.connector.hive.converters.HiveConnectorInfoConverter; +import com.netflix.metacat.connector.hive.converters.HiveTypeConverter; +import com.netflix.metacat.connector.hive.iceberg.IcebergTableCriteriaImpl; +import com.netflix.metacat.connector.hive.iceberg.IcebergTableHandler; +import com.netflix.metacat.connector.hive.iceberg.IcebergTableOpWrapper; +import com.netflix.metacat.connector.hive.iceberg.IcebergTableOpsProxy; import com.netflix.metacat.connector.polaris.configs.PolarisPersistenceConfig; +import com.netflix.metacat.connector.polaris.mappers.PolarisTableMapper; import com.netflix.metacat.connector.polaris.store.PolarisStoreService; import com.netflix.spectator.api.NoopRegistry; import lombok.Getter; @@ -20,6 +29,7 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mockito; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.orm.jpa.AutoConfigureDataJpa; import org.springframework.boot.test.context.SpringBootTest; @@ -56,17 +66,41 @@ public class PolarisConnectorDatabaseServiceTest { @Shared private ConnectorRequestContext requestContext = new ConnectorRequestContext(); + @Shared + private ThreadServiceManager serviceManager = Mockito.mock(ThreadServiceManager.class); + @Shared private PolarisConnectorDatabaseService polarisDBService; + @Shared + private PolarisConnectorTableService polarisTableService; + /** * Initialization. */ @BeforeEach public void init() { connectorContext = new ConnectorContext(CATALOG_NAME, CATALOG_NAME, "polaris", - new DefaultConfigImpl(new MetacatProperties(null)), new NoopRegistry(), null, Maps.newHashMap()); + new DefaultConfigImpl( + new MetacatProperties(null) + ), + new NoopRegistry(), + null, + Maps.newHashMap() + ); polarisDBService = new PolarisConnectorDatabaseService(polarisStoreService, connectorContext); + + polarisTableService = new PolarisConnectorTableService( + polarisStoreService, + CATALOG_NAME, + polarisDBService, + new HiveConnectorInfoConverter(new HiveTypeConverter()), + new IcebergTableHandler(connectorContext, + new IcebergTableCriteriaImpl(connectorContext), + new IcebergTableOpWrapper(connectorContext, serviceManager), + new IcebergTableOpsProxy()), + new PolarisTableMapper(CATALOG_NAME), + connectorContext); } /** @@ -166,5 +200,28 @@ public void testDeleteDb() { polarisDBService.delete(requestContext, DB1_QUALIFIED_NAME); Assert.assertFalse(polarisDBService.exists(requestContext, DB1_QUALIFIED_NAME)); } -} + @Test + public void testDeleteDbNoCascades() { + final DatabaseInfo info = DatabaseInfo.builder().name(DB1_QUALIFIED_NAME).build(); + polarisDBService.create(requestContext, info); + Assert.assertTrue(polarisDBService.exists(requestContext, DB1_QUALIFIED_NAME)); + + final QualifiedName qualifiedName = QualifiedName.ofTable( + CATALOG_NAME, DB1_QUALIFIED_NAME.getDatabaseName(), "table1"); + final TableInfo tableInfo = TableInfo.builder() + .name(qualifiedName) + .metadata(ImmutableMap.of("table_type", "ICEBERG", "metadata_location", "loc1")) + .build(); + polarisTableService.create(requestContext, tableInfo); + Assert.assertTrue(polarisTableService.exists(requestContext, qualifiedName)); + + polarisDBService.delete(requestContext, DB1_QUALIFIED_NAME); + + Assert.assertTrue(polarisTableService.exists(requestContext, qualifiedName)); + System.out.println("testDeleteDbNoCascades Table:"); + System.out.println(polarisTableService.get(requestContext, qualifiedName)); + Assert.assertTrue(polarisDBService.exists(requestContext, DB1_QUALIFIED_NAME)); + + } +} diff --git a/metacat-connector-polaris/src/test/java/com/netflix/metacat/connector/polaris/store/PolarisStoreConnectorTest.java b/metacat-connector-polaris/src/test/java/com/netflix/metacat/connector/polaris/store/PolarisStoreConnectorTest.java index 9479c6e14..f31b58846 100644 --- a/metacat-connector-polaris/src/test/java/com/netflix/metacat/connector/polaris/store/PolarisStoreConnectorTest.java +++ b/metacat-connector-polaris/src/test/java/com/netflix/metacat/connector/polaris/store/PolarisStoreConnectorTest.java @@ -16,6 +16,7 @@ import org.springframework.boot.test.autoconfigure.orm.jpa.AutoConfigureDataJpa; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.dao.DataAccessException; +import org.springframework.dao.DataIntegrityViolationException; import org.springframework.dao.OptimisticLockingFailureException; import org.springframework.test.context.ActiveProfiles; import org.springframework.test.context.junit.jupiter.SpringExtension; @@ -127,6 +128,21 @@ public void testTableCreationAndDeletion() { Assert.assertFalse(polarisConnector.tableExistsById(tblEntity.getTblId())); } + /** + * Test database deletion if table exists and ON DELETE CASCADE is disabled. + */ + @Test + public void testDbDeletionNoCascade() { + final String dbName = generateDatabaseName(); + final String tblName = generateTableName(); + final PolarisDatabaseEntity dbEntity = createDB(dbName); + final PolarisTableEntity tblEntity = createTable(dbName, tblName); + + Assertions.assertThrows(DataIntegrityViolationException.class, () -> + polarisConnector.deleteDatabase(dbName)); + Assert.assertTrue(polarisConnector.databaseExists(dbName)); + } + /** * Test to verify that table name can be updated. */ diff --git a/metacat-connector-polaris/src/test/resources/h2db/schema.sql b/metacat-connector-polaris/src/test/resources/h2db/schema.sql index 93b389a56..36d3bc6f9 100644 --- a/metacat-connector-polaris/src/test/resources/h2db/schema.sql +++ b/metacat-connector-polaris/src/test/resources/h2db/schema.sql @@ -24,7 +24,7 @@ create table TBLS ( created_date TIMESTAMP not null, last_updated_by varchar(255), last_updated_date TIMESTAMP not null, - foreign key (db_name) references DBS(name) ON DELETE CASCADE ON UPDATE CASCADE + foreign key (db_name) references DBS(name) ON DELETE RESTRICT ON UPDATE CASCADE ); CREATE INDEX DB_NAME_IDX ON TBLS(db_name); diff --git a/metacat-functional-tests/metacat-test-cluster/datastores/crdb/sql/schema.sql b/metacat-functional-tests/metacat-test-cluster/datastores/crdb/sql/schema.sql index 98887559b..2d947fdb1 100644 --- a/metacat-functional-tests/metacat-test-cluster/datastores/crdb/sql/schema.sql +++ b/metacat-functional-tests/metacat-test-cluster/datastores/crdb/sql/schema.sql @@ -24,5 +24,5 @@ create table TBLS ( last_updated_by STRING(255), last_updated_date TIMESTAMP not null, constraint uniq_name unique(db_name, tbl_name), - foreign key (db_name) references DBS(name) ON DELETE CASCADE ON UPDATE CASCADE + foreign key (db_name) references DBS(name) ON DELETE RESTRICT ON UPDATE CASCADE );