>From Hussain Towaileb <[email protected]>: Hussain Towaileb has submitted this change. ( https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20866?usp=email )
Change subject: [NO ISSUE][EXT]: ensure no dependent collections on catalog drop ...................................................................... [NO ISSUE][EXT]: ensure no dependent collections on catalog drop Ext-ref: MB-70463 Change-Id: I52bafb87fbe5fb856a6220ee11b2bf985bc3b70c Reviewed-on: https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20866 Reviewed-by: Hussain Towaileb <[email protected]> Integration-Tests: Jenkins <[email protected]> Tested-by: Jenkins <[email protected]> Reviewed-by: Ali Alsuliman <[email protected]> --- M asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/handlers/CatalogStatementHandler.java A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/iceberg/catalog/negative/catalog-already-exists/test.001.ddl.sqlpp A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/iceberg/catalog/negative/drop-has-dependent-collections/test.000.ddl.sqlpp A asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/iceberg/catalog/negative/drop-has-dependent-collections/test.001.ddl.sqlpp M asterixdb/asterix-app/src/test/resources/runtimets/testsuite_iceberg.xml M asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/MetadataNode.java 6 files changed, 141 insertions(+), 22 deletions(-) Approvals: Ali Alsuliman: Looks good to me, approved Jenkins: Verified; Verified Hussain Towaileb: Looks good to me, but someone else must approve Anon. E. Moose #1000171: diff --git a/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/handlers/CatalogStatementHandler.java b/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/handlers/CatalogStatementHandler.java index 8c33407..3f88893 100644 --- a/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/handlers/CatalogStatementHandler.java +++ b/asterixdb/asterix-app/src/main/java/org/apache/asterix/app/translator/handlers/CatalogStatementHandler.java @@ -49,7 +49,6 @@ import org.apache.asterix.object.base.AdmObjectNode; import org.apache.asterix.translator.SessionConfig; import org.apache.hyracks.algebricks.common.exceptions.AlgebricksException; -import org.apache.hyracks.api.exceptions.SourceLocation; public class CatalogStatementHandler { @@ -119,8 +118,8 @@ validateCatalogType(statement.getCatalogType()); validateCatalogProperties(statement, mdTxnCtx, metadataProvider); - beforeAddTxnCommit(metadataProvider, creator, EntityDetails.newCatalog(catalogName)); MetadataManager.INSTANCE.addCatalog(mdTxnCtx, getCatalog(statement)); + beforeAddTxnCommit(metadataProvider, creator, EntityDetails.newCatalog(catalogName)); MetadataManager.INSTANCE.commitTransaction(mdTxnCtx); return true; } catch (Exception e) { @@ -149,24 +148,8 @@ metadataProvider.setMetadataTxnContext(mdTxnCtx); try { String catalogName = statement.getCatalogName(); - Catalog catalog = MetadataManager.INSTANCE.getCatalog(mdTxnCtx, catalogName); - if (catalog == null) { - if (statement.getIfExists()) { - MetadataManager.INSTANCE.commitTransaction(mdTxnCtx); - return false; - } else { - SourceLocation srcLoc = statement.getSourceLocation(); - throw new CompilationException(ErrorCode.UNKNOWN_CATALOG, srcLoc, catalogName); - } - } - - // if we have any collections on catalog without cascade passed, we fail the drop, so check one only - // otherwise, list all so we drop them all - // TODO(htowaileb): list collections on catalog to drop - boolean firstOnly = !statement.isCascade(); - - beforeDropTxnCommit(metadataProvider, creator, EntityDetails.newCatalog(catalogName)); MetadataManager.INSTANCE.dropCatalog(mdTxnCtx, catalogName); + beforeDropTxnCommit(metadataProvider, creator, EntityDetails.newCatalog(catalogName)); MetadataManager.INSTANCE.commitTransaction(mdTxnCtx); return true; } catch (Exception e) { @@ -220,7 +203,7 @@ */ protected IcebergCatalog createIcebergCatalog(CatalogCreateStatement statement) throws AlgebricksException { IcebergCatalogCreateStatement icebergStatement = (IcebergCatalogCreateStatement) statement; - IcebergCatalogDetailsDecl detailsDecl = (IcebergCatalogDetailsDecl) icebergStatement.getCatalogDetailsDecl(); + IcebergCatalogDetailsDecl detailsDecl = icebergStatement.getCatalogDetailsDecl(); Map<String, String> properties = createCatalogDetailsProperties(icebergStatement); IcebergCatalogDetails details = new IcebergCatalogDetails(detailsDecl.getAdapter(), properties); return new IcebergCatalog(statement.getCatalogName(), statement.getCatalogType(), details, @@ -230,8 +213,7 @@ protected Map<String, String> createCatalogDetailsProperties(IcebergCatalogCreateStatement statement) throws AlgebricksException { AdmObjectNode withObjectNode = statement.getWithObjectNode(); - IcebergCatalogDetailsDecl icebergCatalogDetailsDecl = - (IcebergCatalogDetailsDecl) statement.getCatalogDetailsDecl(); + IcebergCatalogDetailsDecl icebergCatalogDetailsDecl = statement.getCatalogDetailsDecl(); Map<String, String> properties = new HashMap<>(icebergCatalogDetailsDecl.getProperties()); Map<String, String> withClauseProperties = ExternalDataUtils .convertStringArrayParamIntoNumberedParameters(withObjectNode, statement.getSourceLocation()); diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/iceberg/catalog/negative/catalog-already-exists/test.001.ddl.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/iceberg/catalog/negative/catalog-already-exists/test.001.ddl.sqlpp new file mode 100644 index 0000000..75d7436 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/iceberg/catalog/negative/catalog-already-exists/test.001.ddl.sqlpp @@ -0,0 +1,20 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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. + */ + +DROP CATALOG myGlueCatalog; \ No newline at end of file diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/iceberg/catalog/negative/drop-has-dependent-collections/test.000.ddl.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/iceberg/catalog/negative/drop-has-dependent-collections/test.000.ddl.sqlpp new file mode 100644 index 0000000..7532b5e --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/iceberg/catalog/negative/drop-has-dependent-collections/test.000.ddl.sqlpp @@ -0,0 +1,59 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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. + */ + +CREATE CATALOG myGlueCatalog +TYPE Iceberg +SOURCE AWS_GLUE +WITH { + "warehouse": "s3://cbas-iceberg-playground/my-glue-catalog/warehouse/", + "accessKeyId" : "myAccessKeyId", + "secretAccessKey" : "mySecretAccessKey", + "region" : "us-west-2" +}; + +CREATE TYPE test AS OPEN {}; +CREATE EXTERNAL COLLECTION test(test) USING S3 +( + ("table-format"="iceberg"), + ("namespace"="glue_namespace"), + ("tableName"="users"), + ("accessKeyId"="fakeAccessKeyId"), + ("secretAccessKey"="fakeSecretAccessKey"), + ("region"="eu-central-1"), + ("catalogName"="myGlueCatalog"), + ("format"="parquet"), + ("decimal-to-double"="true") + +); + +CREATE EXTERNAL COLLECTION test2(test) USING S3 +( + ("table-format"="iceberg"), + ("namespace"="glue_namespace"), + ("tableName"="users"), + ("accessKeyId"="fakeAccessKeyId"), + ("secretAccessKey"="fakeSecretAccessKey"), + ("region"="eu-central-1"), + ("catalogName"="myGlueCatalog"), + ("format"="parquet"), + ("decimal-to-double"="true") + +); + +DROP CATALOG myGlueCatalog; \ No newline at end of file diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/iceberg/catalog/negative/drop-has-dependent-collections/test.001.ddl.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/iceberg/catalog/negative/drop-has-dependent-collections/test.001.ddl.sqlpp new file mode 100644 index 0000000..6ee0d09 --- /dev/null +++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/iceberg/catalog/negative/drop-has-dependent-collections/test.001.ddl.sqlpp @@ -0,0 +1,22 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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. + */ + +DROP COLLECTION test IF EXISTS; +DROP COLLECTION test2 IF EXISTS; +DROP CATALOG myGlueCatalog IF EXISTS; \ No newline at end of file diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_iceberg.xml b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_iceberg.xml index e314d27..d567e21 100644 --- a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_iceberg.xml +++ b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_iceberg.xml @@ -59,5 +59,11 @@ <expected-error>ASX1227: Cannot find catalog with name myDoesNotExistGlueCatalog</expected-error> </compilation-unit> </test-case> + <test-case FilePath="iceberg/catalog/negative"> + <compilation-unit name="drop-has-dependent-collections"> + <output-dir compare="Text">drop-has-dependent-collections</output-dir> + <expected-error>ASX1148: Cannot drop catalog myGlueCatalog being used by collection Default.Default.test</expected-error> + </compilation-unit> + </test-case> </test-group> </test-suite> diff --git a/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/MetadataNode.java b/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/MetadataNode.java index e1c1f6e..6060d94 100644 --- a/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/MetadataNode.java +++ b/asterixdb/asterix-metadata/src/main/java/org/apache/asterix/metadata/MetadataNode.java @@ -58,6 +58,7 @@ import static org.apache.asterix.common.exceptions.ErrorCode.UNKNOWN_SYNONYM; import static org.apache.asterix.common.exceptions.ErrorCode.UNKNOWN_TYPE; import static org.apache.asterix.common.utils.IdentifierUtil.dataset; +import static org.apache.asterix.external.util.iceberg.IcebergConstants.ICEBERG_CATALOG_NAME; import java.io.PrintStream; import java.rmi.RemoteException; @@ -72,10 +73,12 @@ import org.apache.asterix.common.api.IApplicationContext; import org.apache.asterix.common.api.IDatasetLifecycleManager; import org.apache.asterix.common.api.INcApplicationContext; +import org.apache.asterix.common.config.DatasetConfig; import org.apache.asterix.common.config.DatasetConfig.DatasetType; import org.apache.asterix.common.dataflow.LSMIndexUtil; import org.apache.asterix.common.exceptions.ACIDException; import org.apache.asterix.common.exceptions.AsterixException; +import org.apache.asterix.common.exceptions.CompilationException; import org.apache.asterix.common.exceptions.MetadataException; import org.apache.asterix.common.exceptions.NoOpWarningCollector; import org.apache.asterix.common.functions.FunctionSignature; @@ -117,6 +120,7 @@ import org.apache.asterix.metadata.entities.Datatype; import org.apache.asterix.metadata.entities.Dataverse; import org.apache.asterix.metadata.entities.DependencyKind; +import org.apache.asterix.metadata.entities.ExternalDatasetDetails; import org.apache.asterix.metadata.entities.Feed; import org.apache.asterix.metadata.entities.FeedConnection; import org.apache.asterix.metadata.entities.FeedPolicyEntity; @@ -3192,6 +3196,13 @@ @Override public void dropCatalog(TxnId txnId, String catalogName) throws AlgebricksException, RemoteException { try { + Catalog catalog = getCatalog(txnId, catalogName); + if (catalog == null) { + throw new CompilationException(org.apache.asterix.common.exceptions.ErrorCode.UNKNOWN_CATALOG, + catalogName); + } + confirmCatalogIsUnusedByCollections(txnId, catalogName); + // delete the catalog entry from the 'Catalog' collection ITupleReference searchKey = createTuple(catalogName); MetadataIndex catalogIndex = mdIndexesProvider.getCatalogEntity().getIndex(); @@ -3205,4 +3216,23 @@ } } } + + private void confirmCatalogIsUnusedByCollections(TxnId txnId, String catalogName) throws AlgebricksException { + List<Dataset> allCollections = getAllDatasets(txnId); + for (Dataset collection : allCollections) { + if (collection.getDatasetType() != DatasetConfig.DatasetType.EXTERNAL) { + continue; + } + ExternalDatasetDetails details = (ExternalDatasetDetails) collection.getDatasetDetails(); + String collectionCatalogName = details.getProperties().get(ICEBERG_CATALOG_NAME); + if (!catalogName.equals(collectionCatalogName)) { + continue; + } + String database = collection.getDatabaseName(); + String dataverseName = collection.getDataverseName().getCanonicalForm(); + String collectionName = collection.getDatasetName(); + throw new AsterixException(CANNOT_DROP_OBJECT_DEPENDENT_EXISTS, "catalog", catalogName, "collection", + String.join(".", database, dataverseName, collectionName)); + } + } } -- To view, visit https://asterix-gerrit.ics.uci.edu/c/asterixdb/+/20866?usp=email To unsubscribe, or for help writing mail filters, visit https://asterix-gerrit.ics.uci.edu/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Change-Id: I52bafb87fbe5fb856a6220ee11b2bf985bc3b70c Gerrit-Change-Number: 20866 Gerrit-PatchSet: 5 Gerrit-Owner: Hussain Towaileb <[email protected]> Gerrit-Reviewer: Ali Alsuliman <[email protected]> Gerrit-Reviewer: Anon. E. Moose #1000171 Gerrit-Reviewer: Hussain Towaileb <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-CC: Michael Blow <[email protected]>
