Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove ON DELETE CASCADE #623

Open
wants to merge 41 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
4a5d863
Build test
Dec 3, 2024
116a6c4
fix checkstyle
Dec 3, 2024
59b28b1
fix checkstyle
Dec 3, 2024
6eb29c3
see if the test error is due to schema change
Dec 4, 2024
612b0b3
check tests
Dec 4, 2024
f232642
check tests
Dec 4, 2024
a1fd485
checkstyle
Dec 4, 2024
5e2824c
test
Dec 4, 2024
20d3853
test
Dec 4, 2024
12bf322
test
Dec 4, 2024
7808048
test
Dec 4, 2024
720b7e3
test
Dec 4, 2024
14c8ea4
test
Dec 4, 2024
b9ee9fd
test
Dec 4, 2024
6a3b2bc
test hopefully last
Dec 4, 2024
725faa3
test
Dec 4, 2024
79fa3e1
checkstyle
Dec 4, 2024
588bbda
already exists
Dec 4, 2024
bc407d0
checkstyle
Dec 4, 2024
89dabd1
test
Dec 4, 2024
91a425d
move test
Dec 4, 2024
8275c09
fix moved test
Dec 4, 2024
fd0085f
fix test final
Dec 4, 2024
5efabf5
minor changes
Dec 4, 2024
9a9e372
minor changes
Dec 4, 2024
60f2522
minor changes
Dec 4, 2024
6d5c382
minor changes
Dec 4, 2024
fdb9a14
minor changes
Dec 4, 2024
c75b3ad
minor changes
Dec 4, 2024
ae6f57f
comment out catch
Dec 4, 2024
d5c33c2
comment out catch
Dec 4, 2024
eb95f84
comment out catch
Dec 4, 2024
231aa96
final supposed structure
Dec 4, 2024
94ccd3d
check if table still exists after db deletion
Dec 7, 2024
9c137dc
check if table still exists after db deletion
Dec 7, 2024
41d021a
check if table exist and db exists after db delete was blocked
Dec 7, 2024
7ee77fe
check if table exist and db exists after db delete was blocked
Dec 7, 2024
839599c
check if table exist and db exists after db delete was blocked
Dec 7, 2024
ef7db85
checkstyle: check if table exist and db exists after db delete was bl…
Dec 7, 2024
bfc6da8
print: check if table exist and db exists after db delete was blocked
Dec 7, 2024
81d3fc4
check reproduceable: check if table exist and db exists after db dele…
Dec 7, 2024
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 @@ -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(
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}


Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -163,7 +176,7 @@ public List<QualifiedName> listNames(
try {
final String dbPrefix = prefix == null ? "" : prefix.getDatabaseName();
final List<QualifiedName> 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));
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);
}

/**
Expand Down Expand Up @@ -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));

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