korlov42 commented on code in PR #4972:
URL: https://github.com/apache/ignite-3/pull/4972#discussion_r1898508531
##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CatalogUtils.java:
##########
@@ -145,7 +147,11 @@ public class CatalogUtils {
FUNCTIONAL_DEFAULT_FUNCTIONS.put("RAND_UUID", ColumnType.UUID);
}
- public static final List<String> SYSTEM_SCHEMAS =
List.of(SYSTEM_SCHEMA_NAME);
+ public static final List<String> SYSTEM_SCHEMAS = List.of(
Review Comment:
I think, it worth to convert this collection to `Set`, wdyt?
##########
modules/catalog/src/test/java/org/apache/ignite/internal/catalog/CatalogSchemaTest.java:
##########
@@ -17,33 +17,160 @@
package org.apache.ignite.internal.catalog;
+import static org.apache.ignite.internal.catalog.CatalogTestUtils.columnParams;
+import static
org.apache.ignite.internal.catalog.descriptors.CatalogColumnCollation.ASC_NULLS_LAST;
+import static org.apache.ignite.internal.lang.IgniteStringFormatter.format;
import static
org.apache.ignite.internal.testframework.matchers.CompletableFutureExceptionMatcher.willThrowFast;
import static
org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully;
+import static org.apache.ignite.sql.ColumnType.INT32;
import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.hasSize;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.notNullValue;
+import static org.hamcrest.Matchers.nullValue;
import static org.junit.jupiter.api.Assertions.assertNotNull;
+import java.util.List;
import org.apache.ignite.internal.catalog.commands.CreateSchemaCommand;
+import org.apache.ignite.internal.catalog.commands.DropSchemaCommand;
+import org.apache.ignite.internal.catalog.descriptors.CatalogSchemaDescriptor;
import org.apache.ignite.internal.sql.SqlCommon;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
/** Tests for schema related commands. */
public class CatalogSchemaTest extends BaseCatalogManagerTest {
+ private static final String TEST_SCHEMA = "S1";
@Test
public void testCreateSchema() {
- String schemaName = "S1";
+
assertThat(manager.execute(CreateSchemaCommand.builder().name(TEST_SCHEMA).build()),
willCompleteSuccessfully());
-
assertThat(manager.execute(CreateSchemaCommand.builder().name(schemaName).build()),
willCompleteSuccessfully());
+ Catalog latestCatalog = latestCatalog();
- Catalog latestCatalog =
manager.catalog(manager.activeCatalogVersion(clock.nowLong()));
-
- assertNotNull(latestCatalog);
- assertNotNull(latestCatalog.schema(schemaName));
+ assertNotNull(latestCatalog.schema(TEST_SCHEMA));
assertNotNull(latestCatalog.schema(SqlCommon.DEFAULT_SCHEMA_NAME));
assertThat(
-
manager.execute(CreateSchemaCommand.builder().name(schemaName).build()),
+
manager.execute(CreateSchemaCommand.builder().name(TEST_SCHEMA).build()),
willThrowFast(CatalogValidationException.class, "Schema with
name 'S1' already exists")
);
}
+
+ @Test
+ public void testDropEmpty() {
+ int initialSchemasCount = latestCatalog().schemas().size();
+
+
assertThat(manager.execute(CreateSchemaCommand.builder().name(TEST_SCHEMA).build()),
willCompleteSuccessfully());
+
+ assertThat(latestCatalog().schemas(), hasSize(initialSchemasCount +
1));
+
+ CatalogCommand cmd =
DropSchemaCommand.builder().name(TEST_SCHEMA).build();
+
+ assertThat(manager.execute(cmd), willCompleteSuccessfully());
+ assertThat(latestCatalog().schema(TEST_SCHEMA), nullValue());
+ assertThat(latestCatalog().schemas(), hasSize(initialSchemasCount));
+
+ assertThat(
+
manager.execute(DropSchemaCommand.builder().name(TEST_SCHEMA).build()),
+ willThrowFast(CatalogValidationException.class, "Schema with
name 'S1' not found")
+ );
+ }
+
+ @Test
+ public void testDropDefaultSchemaIsAllowed() {
+ CatalogCommand cmd =
DropSchemaCommand.builder().name(SqlCommon.DEFAULT_SCHEMA_NAME).build();
+
+ assertThat(manager.execute(cmd), willCompleteSuccessfully());
+ assertThat(latestCatalog().schema(SqlCommon.DEFAULT_SCHEMA_NAME),
nullValue());
+
+ assertThat(
+ manager.execute(simpleTable("test")),
+ willThrowFast(CatalogValidationException.class, "Schema with
name 'PUBLIC' not found")
+ );
+ }
+
+ @Test
+ public void testDropNonEmpty() {
+ CatalogCommand newSchemaCmd =
CreateSchemaCommand.builder().name(TEST_SCHEMA).build();
+ CatalogCommand newTableCmd = newTableCommand("T1");
+ CatalogCommand idxCmd = newIndexCommand("T1", "I1");
+
+ assertThat(
+ manager.execute(List.of(newSchemaCmd, newTableCmd, idxCmd)),
+ willCompleteSuccessfully()
+ );
+
+ // RESTRICT
+ {
+ assertThat(
+
manager.execute(DropSchemaCommand.builder().name(TEST_SCHEMA).build()),
+ willThrowFast(CatalogValidationException.class, "Schema
'S1' is not empty. Use CASCADE to drop it anyway.")
+ );
+
+ CatalogSchemaDescriptor schemaDescriptor =
latestCatalog().schema(TEST_SCHEMA);
+
+ assertThat(schemaDescriptor, is(notNullValue()));
+ assertThat(schemaDescriptor.tables().length, is(1));
+ }
+
+ // CASCADE
+ {
+ assertThat(latestCatalog().tables(), hasSize(1));
+ assertThat(latestCatalog().indexes(), hasSize(2));
+
+ CatalogCommand dropCmd =
DropSchemaCommand.builder().name(TEST_SCHEMA).cascade(true).build();
+
+ assertThat(manager.execute(dropCmd), willCompleteSuccessfully());
+
+ assertThat(latestCatalog().schema(TEST_SCHEMA), nullValue());
+ assertThat(latestCatalog().tables(), hasSize(0));
+ assertThat(latestCatalog().indexes(), hasSize(0));
+ }
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {
+ CatalogService.SYSTEM_SCHEMA_NAME,
+ CatalogService.INFORMATION_SCHEMA,
+ CatalogService.DEFINITION_SCHEMA
+ })
+ public void testDropSystemSchemaIsForbidden(String schemaName) {
Review Comment:
I would suggest to move all validation tests to `<CommanName>ValidationTest`
class for consistency
##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/DropSchemaCommand.java:
##########
@@ -0,0 +1,115 @@
+/*
+ * 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.
+ */
+
+package org.apache.ignite.internal.catalog.commands;
+
+import static
org.apache.ignite.internal.catalog.commands.CatalogUtils.schemaOrThrow;
+
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.ignite.internal.catalog.Catalog;
+import org.apache.ignite.internal.catalog.CatalogCommand;
+import org.apache.ignite.internal.catalog.CatalogValidationException;
+import org.apache.ignite.internal.catalog.descriptors.CatalogIndexDescriptor;
+import org.apache.ignite.internal.catalog.descriptors.CatalogSchemaDescriptor;
+import org.apache.ignite.internal.catalog.descriptors.CatalogTableDescriptor;
+import org.apache.ignite.internal.catalog.storage.DropIndexEntry;
+import org.apache.ignite.internal.catalog.storage.DropSchemaEntry;
+import org.apache.ignite.internal.catalog.storage.DropTableEntry;
+import org.apache.ignite.internal.catalog.storage.UpdateEntry;
+
+/**
+ * A command that drops a schema with specified name.
+ */
+public class DropSchemaCommand implements CatalogCommand {
+ /** Returns builder to create a command to drop schema with specified
name. */
+ public static DropSchemaCommandBuilder builder() {
+ return new Builder();
+ }
+
+ private final String schemaName;
+
+ private final boolean cascade;
+
+ /**
+ * Constructor.
+ *
+ * @param schemaName Name of the schema.
+ * @param cascade Flag indicating forced deletion of a non-empty schema.
+ * @throws CatalogValidationException if any of restrictions above is
violated.
+ */
+ private DropSchemaCommand(String schemaName, boolean cascade) throws
CatalogValidationException {
+ this.schemaName = schemaName;
Review Comment:
let's add validation for schema name (and also tests for this). See
`org.apache.ignite.internal.catalog.CatalogParamsValidationUtils#validateIdentifier`
for reference
##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/DropSchemaCommand.java:
##########
@@ -0,0 +1,115 @@
+/*
+ * 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.
+ */
+
+package org.apache.ignite.internal.catalog.commands;
+
+import static
org.apache.ignite.internal.catalog.commands.CatalogUtils.schemaOrThrow;
+
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.ignite.internal.catalog.Catalog;
+import org.apache.ignite.internal.catalog.CatalogCommand;
+import org.apache.ignite.internal.catalog.CatalogValidationException;
+import org.apache.ignite.internal.catalog.descriptors.CatalogIndexDescriptor;
+import org.apache.ignite.internal.catalog.descriptors.CatalogSchemaDescriptor;
+import org.apache.ignite.internal.catalog.descriptors.CatalogTableDescriptor;
+import org.apache.ignite.internal.catalog.storage.DropIndexEntry;
+import org.apache.ignite.internal.catalog.storage.DropSchemaEntry;
+import org.apache.ignite.internal.catalog.storage.DropTableEntry;
+import org.apache.ignite.internal.catalog.storage.UpdateEntry;
+
+/**
+ * A command that drops a schema with specified name.
+ */
+public class DropSchemaCommand implements CatalogCommand {
+ /** Returns builder to create a command to drop schema with specified
name. */
+ public static DropSchemaCommandBuilder builder() {
+ return new Builder();
+ }
+
+ private final String schemaName;
+
+ private final boolean cascade;
+
+ /**
+ * Constructor.
+ *
+ * @param schemaName Name of the schema.
+ * @param cascade Flag indicating forced deletion of a non-empty schema.
+ * @throws CatalogValidationException if any of restrictions above is
violated.
+ */
+ private DropSchemaCommand(String schemaName, boolean cascade) throws
CatalogValidationException {
+ this.schemaName = schemaName;
+ this.cascade = cascade;
+ }
+
+ @Override
+ public List<UpdateEntry> get(Catalog catalog) {
+ if (CatalogUtils.isSystemSchema(schemaName)) {
+ throw new CatalogValidationException("System schema can't be
dropped [name={}].", schemaName);
+ }
+
+ CatalogSchemaDescriptor schema = schemaOrThrow(catalog, schemaName);
+
+ if (!cascade && (schema.indexes().length > 0 || schema.tables().length
> 0)) {
Review Comment:
does it make sense to introduce `isEmpty()` method on schema object?
##########
modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/DropSchemaCommand.java:
##########
@@ -0,0 +1,115 @@
+/*
+ * 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.
+ */
+
+package org.apache.ignite.internal.catalog.commands;
+
+import static
org.apache.ignite.internal.catalog.commands.CatalogUtils.schemaOrThrow;
+
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.ignite.internal.catalog.Catalog;
+import org.apache.ignite.internal.catalog.CatalogCommand;
+import org.apache.ignite.internal.catalog.CatalogValidationException;
+import org.apache.ignite.internal.catalog.descriptors.CatalogIndexDescriptor;
+import org.apache.ignite.internal.catalog.descriptors.CatalogSchemaDescriptor;
+import org.apache.ignite.internal.catalog.descriptors.CatalogTableDescriptor;
+import org.apache.ignite.internal.catalog.storage.DropIndexEntry;
+import org.apache.ignite.internal.catalog.storage.DropSchemaEntry;
+import org.apache.ignite.internal.catalog.storage.DropTableEntry;
+import org.apache.ignite.internal.catalog.storage.UpdateEntry;
+
+/**
+ * A command that drops a schema with specified name.
+ */
+public class DropSchemaCommand implements CatalogCommand {
+ /** Returns builder to create a command to drop schema with specified
name. */
+ public static DropSchemaCommandBuilder builder() {
+ return new Builder();
+ }
+
+ private final String schemaName;
+
+ private final boolean cascade;
+
+ /**
+ * Constructor.
+ *
+ * @param schemaName Name of the schema.
+ * @param cascade Flag indicating forced deletion of a non-empty schema.
+ * @throws CatalogValidationException if any of restrictions above is
violated.
+ */
+ private DropSchemaCommand(String schemaName, boolean cascade) throws
CatalogValidationException {
+ this.schemaName = schemaName;
+ this.cascade = cascade;
+ }
+
+ @Override
+ public List<UpdateEntry> get(Catalog catalog) {
+ if (CatalogUtils.isSystemSchema(schemaName)) {
+ throw new CatalogValidationException("System schema can't be
dropped [name={}].", schemaName);
+ }
+
+ CatalogSchemaDescriptor schema = schemaOrThrow(catalog, schemaName);
Review Comment:
let's add validation test on drop non-existing schema
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
