From 2cad5f6c4747f64a16bb68ff4c303cf89438701d Mon Sep 17 00:00:00 2001 From: zstan Date: Mon, 20 Jan 2025 17:56:19 +0300 Subject: [PATCH 1/9] IGNITE-24022 Sql. Fix CreateSchemaCommand catalog command validation --- .../internal/catalog/CatalogManagerImpl.java | 3 +- .../catalog/commands/CreateSchemaCommand.java | 5 + .../commands/CreateSystemSchemaCommand.java | 99 +++++++++++++++++++ .../CreateSystemSchemaCommandBuilder.java | 31 ++++++ .../catalog/CatalogSchemaValidationTest.java | 39 ++++++-- .../CreateSchemaCommandValidationTest.java | 3 + ...eateSystemSchemaCommandValidationTest.java | 78 +++++++++++++++ .../sql/engine/ItCreateTableDdlTest.java | 48 ++++++++- 8 files changed, 296 insertions(+), 10 deletions(-) create mode 100644 modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSystemSchemaCommand.java create mode 100644 modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSystemSchemaCommandBuilder.java create mode 100644 modules/catalog/src/test/java/org/apache/ignite/internal/catalog/commands/CreateSystemSchemaCommandValidationTest.java diff --git a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogManagerImpl.java b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogManagerImpl.java index 3008862672af..ea94bed37512 100644 --- a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogManagerImpl.java +++ b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogManagerImpl.java @@ -40,6 +40,7 @@ import java.util.function.LongSupplier; import org.apache.ignite.internal.catalog.commands.AlterZoneSetDefaultCommand; import org.apache.ignite.internal.catalog.commands.CreateSchemaCommand; +import org.apache.ignite.internal.catalog.commands.CreateSystemSchemaCommand; import org.apache.ignite.internal.catalog.commands.CreateZoneCommand; import org.apache.ignite.internal.catalog.commands.StorageProfileParams; import org.apache.ignite.internal.catalog.descriptors.CatalogIndexDescriptor; @@ -391,7 +392,7 @@ private CompletableFuture initCatalog(Catalog emptyCatalog) { .build(), // Add schemas CreateSchemaCommand.builder().name(SqlCommon.DEFAULT_SCHEMA_NAME).build(), - CreateSchemaCommand.builder().name(SYSTEM_SCHEMA_NAME).build() + CreateSystemSchemaCommand.builder().name(SYSTEM_SCHEMA_NAME).build() ); List entries = new BulkUpdateProducer(initCommands).get(emptyCatalog); diff --git a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSchemaCommand.java b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSchemaCommand.java index 590a2d41dce7..60b05d5cb797 100644 --- a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSchemaCommand.java +++ b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSchemaCommand.java @@ -24,6 +24,7 @@ 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.SchemaExistsException; import org.apache.ignite.internal.catalog.descriptors.CatalogIndexDescriptor; import org.apache.ignite.internal.catalog.descriptors.CatalogSchemaDescriptor; @@ -56,6 +57,10 @@ public boolean ifNotExists() { /** {@inheritDoc} */ @Override public List get(Catalog catalog) { + if (CatalogUtils.isSystemSchema(schemaName)) { + throw new CatalogValidationException("System schema can't be created [name={}].", schemaName); + } + int id = catalog.objectIdGenState(); CatalogSchemaDescriptor schema = catalog.schema(schemaName); diff --git a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSystemSchemaCommand.java b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSystemSchemaCommand.java new file mode 100644 index 000000000000..40a1b495c825 --- /dev/null +++ b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSystemSchemaCommand.java @@ -0,0 +1,99 @@ +/* + * 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.CatalogManagerImpl.INITIAL_CAUSALITY_TOKEN; +import static org.apache.ignite.internal.catalog.CatalogParamsValidationUtils.validateIdentifier; +import static org.apache.ignite.internal.lang.IgniteStringFormatter.format; + +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.CatalogSystemViewDescriptor; +import org.apache.ignite.internal.catalog.descriptors.CatalogTableDescriptor; +import org.apache.ignite.internal.catalog.storage.NewSchemaEntry; +import org.apache.ignite.internal.catalog.storage.ObjectIdGenUpdateEntry; +import org.apache.ignite.internal.catalog.storage.UpdateEntry; + +/** + * Command to create a system schema. + */ +public class CreateSystemSchemaCommand implements CatalogCommand { + private final String schemaName; + + private CreateSystemSchemaCommand(String schemaName) { + validateIdentifier(schemaName, "Name of the schema"); + + if (!CatalogUtils.isSystemSchema(schemaName)) { + throw new CatalogValidationException(format("Not a system schema, schema: '{}'", schemaName)); + } + + this.schemaName = schemaName; + } + + /** {@inheritDoc} */ + @Override + public List get(Catalog catalog) { + int id = catalog.objectIdGenState(); + + if (catalog.schema(schemaName) != null) { + throw new CatalogValidationException(format("Schema with name '{}' already exists", schemaName)); + } + + CatalogSchemaDescriptor schema = new CatalogSchemaDescriptor( + id, + schemaName, + new CatalogTableDescriptor[0], + new CatalogIndexDescriptor[0], + new CatalogSystemViewDescriptor[0], + INITIAL_CAUSALITY_TOKEN + ); + + return List.of( + new NewSchemaEntry(schema), + new ObjectIdGenUpdateEntry(1) + ); + } + + /** Returns builder to create a command to create a system schema. */ + public static CreateSystemSchemaCommand.Builder builder() { + return new CreateSystemSchemaCommand.Builder(); + } + + /** Implementation of {@link CreateSchemaCommandBuilder}. */ + public static class Builder implements CreateSystemSchemaCommandBuilder { + + private String name; + + /** {@inheritDoc} */ + @Override + public CreateSystemSchemaCommandBuilder name(String name) { + this.name = name; + return this; + } + + /** {@inheritDoc} */ + @Override + public CatalogCommand build() { + return new CreateSystemSchemaCommand(name); + } + } +} diff --git a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSystemSchemaCommandBuilder.java b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSystemSchemaCommandBuilder.java new file mode 100644 index 000000000000..f3bc8aef8f31 --- /dev/null +++ b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSystemSchemaCommandBuilder.java @@ -0,0 +1,31 @@ +/* + * 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 org.apache.ignite.internal.catalog.CatalogCommand; + +/** + * Builder for a {@link CreateSystemSchemaCommand}. + */ +public interface CreateSystemSchemaCommandBuilder { + /** Sets schema name. Should not be null or blank. */ + CreateSystemSchemaCommandBuilder name(String name); + + /** Creates new schema command. */ + CatalogCommand build(); +} diff --git a/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/CatalogSchemaValidationTest.java b/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/CatalogSchemaValidationTest.java index 08989d8204f8..1a2f8c73558a 100644 --- a/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/CatalogSchemaValidationTest.java +++ b/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/CatalogSchemaValidationTest.java @@ -17,18 +17,22 @@ package org.apache.ignite.internal.catalog; +import static org.apache.ignite.internal.catalog.commands.CatalogUtils.SYSTEM_SCHEMAS; 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.willSucceedFast; import static org.hamcrest.MatcherAssert.assertThat; +import java.util.Locale; +import java.util.stream.Stream; import org.apache.ignite.internal.catalog.commands.CreateSchemaCommand; import org.apache.ignite.internal.catalog.commands.DropSchemaCommand; import org.apache.ignite.internal.sql.SqlCommon; import org.apache.ignite.internal.testframework.IgniteTestUtils; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.ValueSource; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; /** Validation tests for schema related commands. */ public class CatalogSchemaValidationTest extends BaseCatalogManagerTest { @@ -86,11 +90,17 @@ public void testCreateSchemaWithNullOrEmptyNameIsRejected() { } @ParameterizedTest - @ValueSource(strings = { - CatalogService.SYSTEM_SCHEMA_NAME, - CatalogService.INFORMATION_SCHEMA, - CatalogService.DEFINITION_SCHEMA - }) + @MethodSource("reservedSchemaNames") + public void testCreateSystemSchemaIsRejected(String schemaName) { + CatalogCommand createCmd = CreateSchemaCommand.builder().name(schemaName).build(); + + assertThat(manager.execute(createCmd), + willThrowFast(CatalogValidationException.class, format("System schema can't be created [name={}]", schemaName)) + ); + } + + @ParameterizedTest + @MethodSource("reservedSchemaNames") public void testDropSystemSchemaIsForbidden(String schemaName) { CatalogCommand dropCmd = DropSchemaCommand.builder().name(schemaName).build(); @@ -100,6 +110,23 @@ public void testDropSystemSchemaIsForbidden(String schemaName) { ); } + private static Stream reservedSchemaNames() { + return SYSTEM_SCHEMAS.stream().map(Arguments::of); + } + + @ParameterizedTest + @MethodSource("reservedSchemaNames") + public void testDropNotExistSchemas(String schemaName) { + schemaName = schemaName.toLowerCase(Locale.ROOT); + + CatalogCommand dropCmd = DropSchemaCommand.builder().name(schemaName).build(); + + assertThat( + manager.execute(dropCmd), + willThrowFast(CatalogValidationException.class, format("Schema with name '{}' not found", schemaName)) + ); + } + @Test @SuppressWarnings("ThrowableNotThrown") public void testDropSchemaWithNullOrEmptyNameIsRejected() { diff --git a/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/commands/CreateSchemaCommandValidationTest.java b/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/commands/CreateSchemaCommandValidationTest.java index ff85120de051..cffc03e9b053 100644 --- a/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/commands/CreateSchemaCommandValidationTest.java +++ b/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/commands/CreateSchemaCommandValidationTest.java @@ -28,6 +28,7 @@ /** * Tests to verify validation of {@link CreateSchemaCommand}. */ +@SuppressWarnings({"ThrowableNotThrown"}) public class CreateSchemaCommandValidationTest extends AbstractCommandValidationTest { @ParameterizedTest(name = "[{index}] ''{argumentsWithNames}''") @@ -55,6 +56,8 @@ void commandFailsWhenSchemaAlreadyExists() { () -> builder.build().get(catalog), "Schema with name 'TEST' already exists" ); + + builder.ifNotExists(true).build().get(catalog); } private static Catalog catalogWithSchema(String schemaName) { diff --git a/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/commands/CreateSystemSchemaCommandValidationTest.java b/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/commands/CreateSystemSchemaCommandValidationTest.java new file mode 100644 index 000000000000..35770c1a10d3 --- /dev/null +++ b/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/commands/CreateSystemSchemaCommandValidationTest.java @@ -0,0 +1,78 @@ +/* + * 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.CatalogService.INFORMATION_SCHEMA; +import static org.apache.ignite.internal.lang.IgniteStringFormatter.format; +import static org.apache.ignite.internal.testframework.IgniteTestUtils.assertThrows; + +import java.util.Locale; +import org.apache.ignite.internal.catalog.Catalog; +import org.apache.ignite.internal.catalog.CatalogValidationException; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + +/** + * Tests to verify validation of {@link CreateSystemSchemaCommand}. + */ +@SuppressWarnings({"ThrowableNotThrown"}) +public class CreateSystemSchemaCommandValidationTest extends AbstractCommandValidationTest { + + @ParameterizedTest(name = "[{index}] ''{argumentsWithNames}''") + @MethodSource("nullAndBlankStrings") + void schemaNameMustNotBeNullOrBlank(String name) { + CreateSystemSchemaCommandBuilder builder = CreateSystemSchemaCommand.builder().name(name); + + assertThrows( + CatalogValidationException.class, + builder::build, + "Name of the schema can't be null or blank" + ); + } + + @Test + void commandFailsWithNonSystemSchema() { + String schemaName = INFORMATION_SCHEMA.toLowerCase(Locale.ROOT); + + Catalog catalog = catalogWithSchema(INFORMATION_SCHEMA); + + assertThrows( + CatalogValidationException.class, + () -> CreateSystemSchemaCommand.builder().name(schemaName).build().get(catalog), + format("Not a system schema, schema: '{}'", schemaName) + ); + } + + @Test + void commandFailsWhenSchemaAlreadyExists() { + CreateSystemSchemaCommandBuilder builder = CreateSystemSchemaCommand.builder().name(INFORMATION_SCHEMA); + + Catalog catalog = catalogWithSchema(INFORMATION_SCHEMA); + + assertThrows( + CatalogValidationException.class, + () -> builder.build().get(catalog), + format("Schema with name '{}' already exists", INFORMATION_SCHEMA) + ); + } + + private static Catalog catalogWithSchema(String schemaName) { + return catalog(CreateSystemSchemaCommand.builder().name(schemaName).build()); + } +} diff --git a/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItCreateTableDdlTest.java b/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItCreateTableDdlTest.java index 990db0efebd4..270b600d0a69 100644 --- a/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItCreateTableDdlTest.java +++ b/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItCreateTableDdlTest.java @@ -27,6 +27,7 @@ import static org.apache.ignite.internal.sql.engine.util.SqlTestUtils.assertThrowsSqlException; import static org.apache.ignite.lang.ErrorGroups.Sql.STMT_PARSE_ERR; import static org.apache.ignite.lang.ErrorGroups.Sql.STMT_VALIDATION_ERR; +import static org.apache.ignite.table.QualifiedName.DEFAULT_SCHEMA_NAME; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; @@ -43,7 +44,7 @@ import org.apache.ignite.internal.app.IgniteImpl; import org.apache.ignite.internal.catalog.Catalog; import org.apache.ignite.internal.catalog.CatalogManager; -import org.apache.ignite.internal.catalog.commands.CreateSchemaCommand; +import org.apache.ignite.internal.catalog.commands.CreateSystemSchemaCommand; import org.apache.ignite.internal.catalog.descriptors.CatalogTableDescriptor; import org.apache.ignite.internal.catalog.descriptors.CatalogZoneDescriptor; import org.apache.ignite.internal.schema.Column; @@ -320,7 +321,6 @@ public void literalAsColumDefault() { } @Test - @SuppressWarnings("ThrowableNotThrown") public void doNotAllowFunctionsInNonPkColumns() { // SQL Standard 2016 feature E141-07 - Basic integrity constraints. Column defaults assertThrowsSqlException( @@ -336,7 +336,7 @@ public void testItIsNotPossibleToCreateTablesInSystemSchema(String schema) { if (DEFINITION_SCHEMA.equals(schema) || INFORMATION_SCHEMA.equals(schema)) { IgniteImpl igniteImpl = unwrapIgniteImpl(CLUSTER.aliveNode()); - IgniteTestUtils.await(igniteImpl.catalogManager().execute(CreateSchemaCommand.builder().name(schema).build())); + IgniteTestUtils.await(igniteImpl.catalogManager().execute(CreateSystemSchemaCommand.builder().name(schema).build())); } assertThrowsSqlException( @@ -345,6 +345,48 @@ public void testItIsNotPossibleToCreateTablesInSystemSchema(String schema) { () -> sql(format("CREATE TABLE {}.SYS_TABLE (NAME VARCHAR PRIMARY KEY, SIZE BIGINT)", schema.toLowerCase()))); } + @ParameterizedTest + @MethodSource("reservedSchemaNames") + public void testCreateSystemSchemas(String schema) { + if (!DEFAULT_SCHEMA_NAME.equals(schema)) { + assertThrowsSqlException( + STMT_VALIDATION_ERR, + format("System schema can't be created [name={}]", schema), + () -> sql(format("CREATE SCHEMA {}", schema.toLowerCase()))); + + assertThrowsSqlException( + STMT_VALIDATION_ERR, + format("System schema can't be created [name={}]", schema), + () -> sql(format("CREATE SCHEMA {}", schema))); + + assertThrowsSqlException( + STMT_VALIDATION_ERR, + format("System schema can't be created [name={}]", schema), + () -> sql(format("CREATE SCHEMA \"{}\"", schema))); + } + } + + @ParameterizedTest + @MethodSource("reservedSchemaNames") + public void testDropSystemSchemas(String schema) { + if (!DEFAULT_SCHEMA_NAME.equals(schema)) { + assertThrowsSqlException( + STMT_VALIDATION_ERR, + format("System schema can't be dropped [name={}]", schema), + () -> sql(format("DROP SCHEMA {}", schema.toLowerCase()))); + + assertThrowsSqlException( + STMT_VALIDATION_ERR, + format("System schema can't be dropped [name={}]", schema), + () -> sql(format("DROP SCHEMA {}", schema))); + + assertThrowsSqlException( + STMT_VALIDATION_ERR, + format("System schema can't be dropped [name={}]", schema), + () -> sql(format("DROP SCHEMA \"{}\"", schema))); + } + } + private static Stream reservedSchemaNames() { return SYSTEM_SCHEMAS.stream().map(Arguments::of); } From 8161473d4ee6045f64d2c92cba50583b353f5bbf Mon Sep 17 00:00:00 2001 From: zstan Date: Tue, 21 Jan 2025 14:19:41 +0300 Subject: [PATCH 2/9] change error message --- .../internal/catalog/commands/CreateSchemaCommand.java | 2 +- .../internal/catalog/CatalogSchemaValidationTest.java | 3 ++- .../ignite/internal/sql/engine/ItCreateTableDdlTest.java | 6 +++--- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSchemaCommand.java b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSchemaCommand.java index 60b05d5cb797..073aff9ce739 100644 --- a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSchemaCommand.java +++ b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSchemaCommand.java @@ -58,7 +58,7 @@ public boolean ifNotExists() { @Override public List get(Catalog catalog) { if (CatalogUtils.isSystemSchema(schemaName)) { - throw new CatalogValidationException("System schema can't be created [name={}].", schemaName); + throw new CatalogValidationException("Reserved system schema with name '{}' can't be created.", schemaName); } int id = catalog.objectIdGenState(); diff --git a/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/CatalogSchemaValidationTest.java b/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/CatalogSchemaValidationTest.java index 1a2f8c73558a..43d3f7961a79 100644 --- a/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/CatalogSchemaValidationTest.java +++ b/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/CatalogSchemaValidationTest.java @@ -95,7 +95,8 @@ public void testCreateSystemSchemaIsRejected(String schemaName) { CatalogCommand createCmd = CreateSchemaCommand.builder().name(schemaName).build(); assertThat(manager.execute(createCmd), - willThrowFast(CatalogValidationException.class, format("System schema can't be created [name={}]", schemaName)) + willThrowFast(CatalogValidationException.class, format("Reserved system schema with name '{}' can't be created.", + schemaName)) ); } diff --git a/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItCreateTableDdlTest.java b/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItCreateTableDdlTest.java index 270b600d0a69..0ae45a23e0bc 100644 --- a/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItCreateTableDdlTest.java +++ b/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItCreateTableDdlTest.java @@ -351,17 +351,17 @@ public void testCreateSystemSchemas(String schema) { if (!DEFAULT_SCHEMA_NAME.equals(schema)) { assertThrowsSqlException( STMT_VALIDATION_ERR, - format("System schema can't be created [name={}]", schema), + format("Reserved system schema with name '{}' can't be created.", schema), () -> sql(format("CREATE SCHEMA {}", schema.toLowerCase()))); assertThrowsSqlException( STMT_VALIDATION_ERR, - format("System schema can't be created [name={}]", schema), + format("Reserved system schema with name '{}' can't be created.", schema), () -> sql(format("CREATE SCHEMA {}", schema))); assertThrowsSqlException( STMT_VALIDATION_ERR, - format("System schema can't be created [name={}]", schema), + format("Reserved system schema with name '{}' can't be created.", schema), () -> sql(format("CREATE SCHEMA \"{}\"", schema))); } } From c2c93d8fe91e64f8b47e467608c99c4d5aa19398 Mon Sep 17 00:00:00 2001 From: zstan Date: Thu, 23 Jan 2025 11:41:43 +0300 Subject: [PATCH 3/9] fix after review --- .../sql/engine/ItCreateTableDdlTest.java | 61 +++++++++---------- 1 file changed, 28 insertions(+), 33 deletions(-) diff --git a/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItCreateTableDdlTest.java b/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItCreateTableDdlTest.java index 0ae45a23e0bc..a4eed9b5fef2 100644 --- a/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItCreateTableDdlTest.java +++ b/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItCreateTableDdlTest.java @@ -27,7 +27,6 @@ import static org.apache.ignite.internal.sql.engine.util.SqlTestUtils.assertThrowsSqlException; import static org.apache.ignite.lang.ErrorGroups.Sql.STMT_PARSE_ERR; import static org.apache.ignite.lang.ErrorGroups.Sql.STMT_VALIDATION_ERR; -import static org.apache.ignite.table.QualifiedName.DEFAULT_SCHEMA_NAME; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; @@ -348,43 +347,39 @@ public void testItIsNotPossibleToCreateTablesInSystemSchema(String schema) { @ParameterizedTest @MethodSource("reservedSchemaNames") public void testCreateSystemSchemas(String schema) { - if (!DEFAULT_SCHEMA_NAME.equals(schema)) { - assertThrowsSqlException( - STMT_VALIDATION_ERR, - format("Reserved system schema with name '{}' can't be created.", schema), - () -> sql(format("CREATE SCHEMA {}", schema.toLowerCase()))); - - assertThrowsSqlException( - STMT_VALIDATION_ERR, - format("Reserved system schema with name '{}' can't be created.", schema), - () -> sql(format("CREATE SCHEMA {}", schema))); - - assertThrowsSqlException( - STMT_VALIDATION_ERR, - format("Reserved system schema with name '{}' can't be created.", schema), - () -> sql(format("CREATE SCHEMA \"{}\"", schema))); - } + assertThrowsSqlException( + STMT_VALIDATION_ERR, + format("Reserved system schema with name '{}' can't be created.", schema), + () -> sql(format("CREATE SCHEMA {}", schema.toLowerCase()))); + + assertThrowsSqlException( + STMT_VALIDATION_ERR, + format("Reserved system schema with name '{}' can't be created.", schema), + () -> sql(format("CREATE SCHEMA {}", schema))); + + assertThrowsSqlException( + STMT_VALIDATION_ERR, + format("Reserved system schema with name '{}' can't be created.", schema), + () -> sql(format("CREATE SCHEMA \"{}\"", schema))); } @ParameterizedTest @MethodSource("reservedSchemaNames") public void testDropSystemSchemas(String schema) { - if (!DEFAULT_SCHEMA_NAME.equals(schema)) { - assertThrowsSqlException( - STMT_VALIDATION_ERR, - format("System schema can't be dropped [name={}]", schema), - () -> sql(format("DROP SCHEMA {}", schema.toLowerCase()))); - - assertThrowsSqlException( - STMT_VALIDATION_ERR, - format("System schema can't be dropped [name={}]", schema), - () -> sql(format("DROP SCHEMA {}", schema))); - - assertThrowsSqlException( - STMT_VALIDATION_ERR, - format("System schema can't be dropped [name={}]", schema), - () -> sql(format("DROP SCHEMA \"{}\"", schema))); - } + assertThrowsSqlException( + STMT_VALIDATION_ERR, + format("System schema can't be dropped [name={}]", schema), + () -> sql(format("DROP SCHEMA {}", schema.toLowerCase()))); + + assertThrowsSqlException( + STMT_VALIDATION_ERR, + format("System schema can't be dropped [name={}]", schema), + () -> sql(format("DROP SCHEMA {}", schema))); + + assertThrowsSqlException( + STMT_VALIDATION_ERR, + format("System schema can't be dropped [name={}]", schema), + () -> sql(format("DROP SCHEMA \"{}\"", schema))); } private static Stream reservedSchemaNames() { From a06914225148d8a52ea2097655558d0705d6e8c1 Mon Sep 17 00:00:00 2001 From: zstan Date: Thu, 30 Jan 2025 09:35:47 +0300 Subject: [PATCH 4/9] rework --- .../internal/catalog/CatalogManagerImpl.java | 3 +- .../catalog/commands/CreateSchemaCommand.java | 37 ++++++- .../commands/CreateSchemaCommandBuilder.java | 11 +-- .../commands/CreateSystemSchemaCommand.java | 99 ------------------- .../CreateSystemSchemaCommandBuilder.java | 2 +- ...eateSystemSchemaCommandValidationTest.java | 10 +- .../sql/engine/ItCreateTableDdlTest.java | 4 +- 7 files changed, 44 insertions(+), 122 deletions(-) delete mode 100644 modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSystemSchemaCommand.java diff --git a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogManagerImpl.java b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogManagerImpl.java index ea94bed37512..ca396f9bb886 100644 --- a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogManagerImpl.java +++ b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogManagerImpl.java @@ -40,7 +40,6 @@ import java.util.function.LongSupplier; import org.apache.ignite.internal.catalog.commands.AlterZoneSetDefaultCommand; import org.apache.ignite.internal.catalog.commands.CreateSchemaCommand; -import org.apache.ignite.internal.catalog.commands.CreateSystemSchemaCommand; import org.apache.ignite.internal.catalog.commands.CreateZoneCommand; import org.apache.ignite.internal.catalog.commands.StorageProfileParams; import org.apache.ignite.internal.catalog.descriptors.CatalogIndexDescriptor; @@ -392,7 +391,7 @@ private CompletableFuture initCatalog(Catalog emptyCatalog) { .build(), // Add schemas CreateSchemaCommand.builder().name(SqlCommon.DEFAULT_SCHEMA_NAME).build(), - CreateSystemSchemaCommand.builder().name(SYSTEM_SCHEMA_NAME).build() + CreateSchemaCommand.systemSchemaBuilder().name(SYSTEM_SCHEMA_NAME).build() ); List entries = new BulkUpdateProducer(initCommands).get(emptyCatalog); diff --git a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSchemaCommand.java b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSchemaCommand.java index 073aff9ce739..0915a8475ca9 100644 --- a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSchemaCommand.java +++ b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSchemaCommand.java @@ -43,11 +43,18 @@ public class CreateSchemaCommand implements CatalogCommand { private final boolean ifNotExists; - private CreateSchemaCommand(String schemaName, boolean ifNotExists) { + private final boolean systemSchemaCommand; + + private CreateSchemaCommand(String schemaName, boolean ifNotExists, boolean systemSchemaCommand) { validateIdentifier(schemaName, "Name of the schema"); + if (systemSchemaCommand && !CatalogUtils.isSystemSchema(schemaName)) { + throw new CatalogValidationException(format("Not a system schema, schema: '{}'", schemaName)); + } + this.schemaName = schemaName; this.ifNotExists = ifNotExists; + this.systemSchemaCommand = systemSchemaCommand; } public boolean ifNotExists() { @@ -57,7 +64,7 @@ public boolean ifNotExists() { /** {@inheritDoc} */ @Override public List get(Catalog catalog) { - if (CatalogUtils.isSystemSchema(schemaName)) { + if (!systemSchemaCommand && CatalogUtils.isSystemSchema(schemaName)) { throw new CatalogValidationException("Reserved system schema with name '{}' can't be created.", schemaName); } @@ -115,7 +122,31 @@ public CreateSchemaCommandBuilder ifNotExists(boolean value) { /** {@inheritDoc} */ @Override public CatalogCommand build() { - return new CreateSchemaCommand(name, ifNotExists); + return new CreateSchemaCommand(name, ifNotExists, false); + } + } + + /** Returns builder to create a command to create a system schema. */ + public static SystemSchemaBuilder systemSchemaBuilder() { + return new SystemSchemaBuilder(); + } + + /** Implementation of {@link CreateSystemSchemaCommandBuilder}. */ + public static class SystemSchemaBuilder implements CreateSystemSchemaCommandBuilder { + + private String name; + + /** {@inheritDoc} */ + @Override + public CreateSystemSchemaCommandBuilder name(String name) { + this.name = name; + return this; + } + + /** {@inheritDoc} */ + @Override + public CatalogCommand build() { + return new CreateSchemaCommand(name, false, true); } } } diff --git a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSchemaCommandBuilder.java b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSchemaCommandBuilder.java index 7d06f5cbf4a4..435413409c86 100644 --- a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSchemaCommandBuilder.java +++ b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSchemaCommandBuilder.java @@ -17,19 +17,10 @@ package org.apache.ignite.internal.catalog.commands; -import org.apache.ignite.internal.catalog.CatalogCommand; - /** * Builder for a {@link CreateSchemaCommand}. */ -public interface CreateSchemaCommandBuilder { - - /** Sets schema name. Should not be null or blank. */ - CreateSchemaCommandBuilder name(String name); - +public interface CreateSchemaCommandBuilder extends CreateSystemSchemaCommandBuilder { /** Sets a flag indicating whether {@code IF NOT EXISTS} option was specified. */ CreateSchemaCommandBuilder ifNotExists(boolean value); - - /** Creates new schema command. */ - CatalogCommand build(); } diff --git a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSystemSchemaCommand.java b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSystemSchemaCommand.java deleted file mode 100644 index 40a1b495c825..000000000000 --- a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSystemSchemaCommand.java +++ /dev/null @@ -1,99 +0,0 @@ -/* - * 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.CatalogManagerImpl.INITIAL_CAUSALITY_TOKEN; -import static org.apache.ignite.internal.catalog.CatalogParamsValidationUtils.validateIdentifier; -import static org.apache.ignite.internal.lang.IgniteStringFormatter.format; - -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.CatalogSystemViewDescriptor; -import org.apache.ignite.internal.catalog.descriptors.CatalogTableDescriptor; -import org.apache.ignite.internal.catalog.storage.NewSchemaEntry; -import org.apache.ignite.internal.catalog.storage.ObjectIdGenUpdateEntry; -import org.apache.ignite.internal.catalog.storage.UpdateEntry; - -/** - * Command to create a system schema. - */ -public class CreateSystemSchemaCommand implements CatalogCommand { - private final String schemaName; - - private CreateSystemSchemaCommand(String schemaName) { - validateIdentifier(schemaName, "Name of the schema"); - - if (!CatalogUtils.isSystemSchema(schemaName)) { - throw new CatalogValidationException(format("Not a system schema, schema: '{}'", schemaName)); - } - - this.schemaName = schemaName; - } - - /** {@inheritDoc} */ - @Override - public List get(Catalog catalog) { - int id = catalog.objectIdGenState(); - - if (catalog.schema(schemaName) != null) { - throw new CatalogValidationException(format("Schema with name '{}' already exists", schemaName)); - } - - CatalogSchemaDescriptor schema = new CatalogSchemaDescriptor( - id, - schemaName, - new CatalogTableDescriptor[0], - new CatalogIndexDescriptor[0], - new CatalogSystemViewDescriptor[0], - INITIAL_CAUSALITY_TOKEN - ); - - return List.of( - new NewSchemaEntry(schema), - new ObjectIdGenUpdateEntry(1) - ); - } - - /** Returns builder to create a command to create a system schema. */ - public static CreateSystemSchemaCommand.Builder builder() { - return new CreateSystemSchemaCommand.Builder(); - } - - /** Implementation of {@link CreateSchemaCommandBuilder}. */ - public static class Builder implements CreateSystemSchemaCommandBuilder { - - private String name; - - /** {@inheritDoc} */ - @Override - public CreateSystemSchemaCommandBuilder name(String name) { - this.name = name; - return this; - } - - /** {@inheritDoc} */ - @Override - public CatalogCommand build() { - return new CreateSystemSchemaCommand(name); - } - } -} diff --git a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSystemSchemaCommandBuilder.java b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSystemSchemaCommandBuilder.java index f3bc8aef8f31..bef61b8fafc0 100644 --- a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSystemSchemaCommandBuilder.java +++ b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSystemSchemaCommandBuilder.java @@ -20,7 +20,7 @@ import org.apache.ignite.internal.catalog.CatalogCommand; /** - * Builder for a {@link CreateSystemSchemaCommand}. + * Builder for a {@link CreateSchemaCommand} with system schemas usage. */ public interface CreateSystemSchemaCommandBuilder { /** Sets schema name. Should not be null or blank. */ diff --git a/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/commands/CreateSystemSchemaCommandValidationTest.java b/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/commands/CreateSystemSchemaCommandValidationTest.java index 35770c1a10d3..20a0b6254a23 100644 --- a/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/commands/CreateSystemSchemaCommandValidationTest.java +++ b/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/commands/CreateSystemSchemaCommandValidationTest.java @@ -29,7 +29,7 @@ import org.junit.jupiter.params.provider.MethodSource; /** - * Tests to verify validation of {@link CreateSystemSchemaCommand}. + * Tests to verify validation of {@link CreateSchemaCommand} for system schemas. */ @SuppressWarnings({"ThrowableNotThrown"}) public class CreateSystemSchemaCommandValidationTest extends AbstractCommandValidationTest { @@ -37,7 +37,7 @@ public class CreateSystemSchemaCommandValidationTest extends AbstractCommandVali @ParameterizedTest(name = "[{index}] ''{argumentsWithNames}''") @MethodSource("nullAndBlankStrings") void schemaNameMustNotBeNullOrBlank(String name) { - CreateSystemSchemaCommandBuilder builder = CreateSystemSchemaCommand.builder().name(name); + CreateSystemSchemaCommandBuilder builder = CreateSchemaCommand.systemSchemaBuilder().name(name); assertThrows( CatalogValidationException.class, @@ -54,14 +54,14 @@ void commandFailsWithNonSystemSchema() { assertThrows( CatalogValidationException.class, - () -> CreateSystemSchemaCommand.builder().name(schemaName).build().get(catalog), + () -> CreateSchemaCommand.systemSchemaBuilder().name(schemaName).build().get(catalog), format("Not a system schema, schema: '{}'", schemaName) ); } @Test void commandFailsWhenSchemaAlreadyExists() { - CreateSystemSchemaCommandBuilder builder = CreateSystemSchemaCommand.builder().name(INFORMATION_SCHEMA); + CreateSystemSchemaCommandBuilder builder = CreateSchemaCommand.systemSchemaBuilder().name(INFORMATION_SCHEMA); Catalog catalog = catalogWithSchema(INFORMATION_SCHEMA); @@ -73,6 +73,6 @@ void commandFailsWhenSchemaAlreadyExists() { } private static Catalog catalogWithSchema(String schemaName) { - return catalog(CreateSystemSchemaCommand.builder().name(schemaName).build()); + return catalog(CreateSchemaCommand.systemSchemaBuilder().name(schemaName).build()); } } diff --git a/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItCreateTableDdlTest.java b/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItCreateTableDdlTest.java index a4eed9b5fef2..7e31ec73a411 100644 --- a/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItCreateTableDdlTest.java +++ b/modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItCreateTableDdlTest.java @@ -43,7 +43,7 @@ import org.apache.ignite.internal.app.IgniteImpl; import org.apache.ignite.internal.catalog.Catalog; import org.apache.ignite.internal.catalog.CatalogManager; -import org.apache.ignite.internal.catalog.commands.CreateSystemSchemaCommand; +import org.apache.ignite.internal.catalog.commands.CreateSchemaCommand; import org.apache.ignite.internal.catalog.descriptors.CatalogTableDescriptor; import org.apache.ignite.internal.catalog.descriptors.CatalogZoneDescriptor; import org.apache.ignite.internal.schema.Column; @@ -335,7 +335,7 @@ public void testItIsNotPossibleToCreateTablesInSystemSchema(String schema) { if (DEFINITION_SCHEMA.equals(schema) || INFORMATION_SCHEMA.equals(schema)) { IgniteImpl igniteImpl = unwrapIgniteImpl(CLUSTER.aliveNode()); - IgniteTestUtils.await(igniteImpl.catalogManager().execute(CreateSystemSchemaCommand.builder().name(schema).build())); + IgniteTestUtils.await(igniteImpl.catalogManager().execute(CreateSchemaCommand.systemSchemaBuilder().name(schema).build())); } assertThrowsSqlException( From 2ebdaf5d13edb72f2e671b9f4451003fcd944c07 Mon Sep 17 00:00:00 2001 From: zstan Date: Thu, 30 Jan 2025 11:38:05 +0300 Subject: [PATCH 5/9] fix tests --- .../commands/CreateSchemaCommandValidationTest.java | 2 +- ...tionTest.java => CreateSystemSchemaValidationTest.java} | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) rename modules/catalog/src/test/java/org/apache/ignite/internal/catalog/commands/{CreateSystemSchemaCommandValidationTest.java => CreateSystemSchemaValidationTest.java} (91%) diff --git a/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/commands/CreateSchemaCommandValidationTest.java b/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/commands/CreateSchemaCommandValidationTest.java index 7652ee30d44c..1b236aa4d69d 100644 --- a/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/commands/CreateSchemaCommandValidationTest.java +++ b/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/commands/CreateSchemaCommandValidationTest.java @@ -58,7 +58,7 @@ void commandFailsWhenSchemaAlreadyExists() { "Schema with name 'TEST' already exists" ); - builder.ifNotExists(true).build().get(catalog); + builder.ifNotExists(true).build().get(new UpdateContext(catalog)); } private static Catalog catalogWithSchema(String schemaName) { diff --git a/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/commands/CreateSystemSchemaCommandValidationTest.java b/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/commands/CreateSystemSchemaValidationTest.java similarity index 91% rename from modules/catalog/src/test/java/org/apache/ignite/internal/catalog/commands/CreateSystemSchemaCommandValidationTest.java rename to modules/catalog/src/test/java/org/apache/ignite/internal/catalog/commands/CreateSystemSchemaValidationTest.java index 20a0b6254a23..fcaf53deb8e4 100644 --- a/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/commands/CreateSystemSchemaCommandValidationTest.java +++ b/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/commands/CreateSystemSchemaValidationTest.java @@ -24,6 +24,7 @@ import java.util.Locale; import org.apache.ignite.internal.catalog.Catalog; import org.apache.ignite.internal.catalog.CatalogValidationException; +import org.apache.ignite.internal.catalog.UpdateContext; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; @@ -32,7 +33,7 @@ * Tests to verify validation of {@link CreateSchemaCommand} for system schemas. */ @SuppressWarnings({"ThrowableNotThrown"}) -public class CreateSystemSchemaCommandValidationTest extends AbstractCommandValidationTest { +public class CreateSystemSchemaValidationTest extends AbstractCommandValidationTest { @ParameterizedTest(name = "[{index}] ''{argumentsWithNames}''") @MethodSource("nullAndBlankStrings") @@ -54,7 +55,7 @@ void commandFailsWithNonSystemSchema() { assertThrows( CatalogValidationException.class, - () -> CreateSchemaCommand.systemSchemaBuilder().name(schemaName).build().get(catalog), + () -> CreateSchemaCommand.systemSchemaBuilder().name(schemaName).build().get(new UpdateContext(catalog)), format("Not a system schema, schema: '{}'", schemaName) ); } @@ -67,7 +68,7 @@ void commandFailsWhenSchemaAlreadyExists() { assertThrows( CatalogValidationException.class, - () -> builder.build().get(catalog), + () -> builder.build().get(new UpdateContext(catalog)), format("Schema with name '{}' already exists", INFORMATION_SCHEMA) ); } From b133a332d28be63fefae568a9d04e1fdaeb94860 Mon Sep 17 00:00:00 2001 From: zstan Date: Thu, 30 Jan 2025 11:55:36 +0300 Subject: [PATCH 6/9] add long schemas test --- .../identifiers/test_long_identifiers.test | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/modules/sql-engine/src/integrationTest/sql/group1/identifiers/test_long_identifiers.test b/modules/sql-engine/src/integrationTest/sql/group1/identifiers/test_long_identifiers.test index 15d3f69b4ff3..5e3777e345e3 100644 --- a/modules/sql-engine/src/integrationTest/sql/group1/identifiers/test_long_identifiers.test +++ b/modules/sql-engine/src/integrationTest/sql/group1/identifiers/test_long_identifiers.test @@ -2,8 +2,6 @@ # description: SQL feature F391 (Long identifiers) # group: [identifiers] -# TODO: IGNITE-19703 Add cases for long identifiers for schema names. - statement ok PRAGMA enable_verification @@ -13,6 +11,21 @@ tableName_veryLooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooo statement error: Non-query expression encountered in illegal context. SELECT 1; tableName_veryLooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooongIdentifierOf129Characters +# Create schema with long identifiers +statement ok +CREATE SCHEMA identifier_veryLooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooongIdentifierOf128Characters; + +statement error: Length of identifier +CREATE SCHEMA identifier_veryLoooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooongIdentifierOf129Characters; + +# Create schema and table with long identifiers +statement ok +CREATE TABLE identifier_veryLooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooongIdentifierOf128Characters.tableName_veryLoooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooongIdentifierOf128Characters (keyColumnName_veryLoooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooongIdentifierOf128Characters INTEGER, valueColumnName_veryLoooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooongIdentifierOf128Characters INTEGER, PRIMARY KEY (keyColumnName_veryLoooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooongIdentifierOf128Characters)); + +statement ok +DROP SCHEMA identifier_veryLooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooongIdentifierOf128Characters CASCADE; + + # Create table with short identifiers for test simplicity purpose statement ok CREATE TABLE t (id INTEGER, val INTEGER, PRIMARY KEY (id)) From e63365ba1278a3ebc13137ba1dcb7a6923f9a52f Mon Sep 17 00:00:00 2001 From: zstan Date: Mon, 3 Feb 2025 08:45:49 +0300 Subject: [PATCH 7/9] fix after review --- .../catalog/commands/CreateSchemaCommand.java | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSchemaCommand.java b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSchemaCommand.java index 4c7573e5d04e..212de5772005 100644 --- a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSchemaCommand.java +++ b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSchemaCommand.java @@ -49,8 +49,14 @@ public class CreateSchemaCommand implements CatalogCommand { private CreateSchemaCommand(String schemaName, boolean ifNotExists, boolean systemSchemaCommand) { validateIdentifier(schemaName, "Name of the schema"); - if (systemSchemaCommand && !CatalogUtils.isSystemSchema(schemaName)) { - throw new CatalogValidationException(format("Not a system schema, schema: '{}'", schemaName)); + if (systemSchemaCommand) { + if (!CatalogUtils.isSystemSchema(schemaName)) { + throw new CatalogValidationException(format("Not a system schema, schema: '{}'", schemaName)); + } + } else { + if (CatalogUtils.isSystemSchema(schemaName)) { + throw new CatalogValidationException("Reserved system schema with name '{}' can't be created.", schemaName); + } } this.schemaName = schemaName; @@ -65,10 +71,6 @@ public boolean ifNotExists() { /** {@inheritDoc} */ @Override public List get(UpdateContext updateContext) { - if (!systemSchemaCommand && CatalogUtils.isSystemSchema(schemaName)) { - throw new CatalogValidationException("Reserved system schema with name '{}' can't be created.", schemaName); - } - Catalog catalog = updateContext.catalog(); int id = catalog.objectIdGenState(); From 89920e8cf313995d13bbcc65746dc68e5b26c7a8 Mon Sep 17 00:00:00 2001 From: zstan Date: Mon, 3 Feb 2025 09:20:03 +0300 Subject: [PATCH 8/9] fix --- .../internal/catalog/CatalogSchemaValidationTest.java | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/CatalogSchemaValidationTest.java b/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/CatalogSchemaValidationTest.java index 43d3f7961a79..e69650394f2e 100644 --- a/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/CatalogSchemaValidationTest.java +++ b/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/CatalogSchemaValidationTest.java @@ -26,6 +26,7 @@ import java.util.Locale; import java.util.stream.Stream; import org.apache.ignite.internal.catalog.commands.CreateSchemaCommand; +import org.apache.ignite.internal.catalog.commands.CreateSchemaCommandBuilder; import org.apache.ignite.internal.catalog.commands.DropSchemaCommand; import org.apache.ignite.internal.sql.SqlCommon; import org.apache.ignite.internal.testframework.IgniteTestUtils; @@ -89,14 +90,16 @@ public void testCreateSchemaWithNullOrEmptyNameIsRejected() { ); } + @SuppressWarnings("ThrowableNotThrown") @ParameterizedTest @MethodSource("reservedSchemaNames") public void testCreateSystemSchemaIsRejected(String schemaName) { - CatalogCommand createCmd = CreateSchemaCommand.builder().name(schemaName).build(); + CreateSchemaCommandBuilder createCmd = CreateSchemaCommand.builder().name(schemaName); - assertThat(manager.execute(createCmd), - willThrowFast(CatalogValidationException.class, format("Reserved system schema with name '{}' can't be created.", - schemaName)) + IgniteTestUtils.assertThrows( + CatalogValidationException.class, + () -> manager.execute(createCmd.build()), + format("Reserved system schema with name '{}' can't be created.", schemaName) ); } From a73d851113653cdf33b327e5b8b7ab58d7b203d1 Mon Sep 17 00:00:00 2001 From: zstan Date: Mon, 3 Feb 2025 09:57:49 +0300 Subject: [PATCH 9/9] tests --- .../ignite/internal/catalog/commands/CreateSchemaCommand.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSchemaCommand.java b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSchemaCommand.java index 212de5772005..0d325602e893 100644 --- a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSchemaCommand.java +++ b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateSchemaCommand.java @@ -44,8 +44,6 @@ public class CreateSchemaCommand implements CatalogCommand { private final boolean ifNotExists; - private final boolean systemSchemaCommand; - private CreateSchemaCommand(String schemaName, boolean ifNotExists, boolean systemSchemaCommand) { validateIdentifier(schemaName, "Name of the schema"); @@ -61,7 +59,6 @@ private CreateSchemaCommand(String schemaName, boolean ifNotExists, boolean syst this.schemaName = schemaName; this.ifNotExists = ifNotExists; - this.systemSchemaCommand = systemSchemaCommand; } public boolean ifNotExists() {