This is an automated email from the ASF dual-hosted git repository.
amashenkov pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/ignite-3.git
The following commit(s) were added to refs/heads/main by this push:
new 19d1ff5a17 IGNITE-19304 Forbid creating index on the duplicate columns
(#2174)
19d1ff5a17 is described below
commit 19d1ff5a17cff8b1b21c37fd8531fe671b574329
Author: Andrew V. Mashenkov <[email protected]>
AuthorDate: Mon Jun 12 17:17:15 2023 +0300
IGNITE-19304 Forbid creating index on the duplicate columns (#2174)
---
.../internal/catalog/CatalogServiceImpl.java | 15 +++++++---
.../internal/catalog/CatalogServiceSelfTest.java | 32 ++++++++++++++++++++++
.../java/org/apache/ignite/lang/ErrorGroups.java | 3 ++
.../internal/sql/api/ItSqlAsynchronousApiTest.java | 12 ++++++++
.../internal/sql/api/ItSqlSynchronousApiTest.java | 12 ++++++++
.../sql/engine/exec/ddl/DdlCommandHandler.java | 32 ++++++++++++++++++----
6 files changed, 96 insertions(+), 10 deletions(-)
diff --git
a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogServiceImpl.java
b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogServiceImpl.java
index 133904021e..2852a1e652 100644
---
a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogServiceImpl.java
+++
b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogServiceImpl.java
@@ -288,6 +288,14 @@ public class CatalogServiceImpl extends
Producer<CatalogEvent, CatalogEventParam
throw new TableAlreadyExistsException(schemaName,
params.tableName());
}
+
params.columns().stream().map(ColumnParams::name).filter(Predicate.not(new
HashSet<>()::add))
+ .findAny().ifPresent(columnName -> {
+ throw new IgniteInternalException(
+
ErrorGroups.Index.INVALID_INDEX_DEFINITION_ERR, "Can't create table with
duplicate columns: "
+ +
params.columns().stream().map(ColumnParams::name).collect(Collectors.joining(",
"))
+ );
+ });
+
CatalogTableDescriptor table =
CatalogUtils.fromParams(catalog.objectIdGenState(), params);
return List.of(
@@ -395,8 +403,7 @@ public class CatalogServiceImpl extends
Producer<CatalogEvent, CatalogEventParam
.ifPresent(columnName -> {
throw new SqlException(
Sql.DROP_IDX_COLUMN_CONSTRAINT_ERR,
- "Can't drop indexed column:
columnName=" + columnName + ", indexName="
- + index.name()
+ "Can't drop indexed column:
columnName=" + columnName + ", indexName=" + index.name()
);
}));
@@ -520,7 +527,7 @@ public class CatalogServiceImpl extends
Producer<CatalogEvent, CatalogEventParam
} else if (duplicateValidator.test(columnName)) {
throw new IgniteInternalException(
ErrorGroups.Index.INVALID_INDEX_DEFINITION_ERR,
- "Can't create index on duplicate columns:
columnName=" + columnName
+ "Can't create index on duplicate columns: " +
String.join(", ", params.columns())
);
}
}
@@ -574,7 +581,7 @@ public class CatalogServiceImpl extends
Producer<CatalogEvent, CatalogEventParam
} else if (duplicateValidator.test(columnName)) {
throw new IgniteInternalException(
ErrorGroups.Index.INVALID_INDEX_DEFINITION_ERR,
- "Can't create index on duplicate columns:
columnName=" + columnName
+ "Can't create index on duplicate columns: " +
String.join(", ", params.columns())
);
}
}
diff --git
a/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/CatalogServiceSelfTest.java
b/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/CatalogServiceSelfTest.java
index 2d09e3f26c..1894a49db5 100644
---
a/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/CatalogServiceSelfTest.java
+++
b/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/CatalogServiceSelfTest.java
@@ -267,6 +267,24 @@ public class CatalogServiceSelfTest {
assertSame(schema, service.activeSchema(clock.nowLong()));
}
+ @Test
+ public void testCreateTableWithDuplicateColumns() {
+ CreateTableParams params = CreateTableParams.builder()
+ .schemaName(SCHEMA_NAME)
+ .tableName(TABLE_NAME)
+ .zone(ZONE_NAME)
+ .columns(List.of(
+
ColumnParams.builder().name("key").type(ColumnType.INT32).build(),
+
ColumnParams.builder().name("val").type(ColumnType.INT32).build(),
+
ColumnParams.builder().name("val").type(ColumnType.INT32).nullable(true).build()
+ ))
+ .primaryKeyColumns(List.of("key"))
+ .build();
+
+ assertThat(service.createTable(params),
willThrowFast(IgniteInternalException.class,
+ "Can't create table with duplicate columns: key, val, val"));
+ }
+
@Test
public void testDropTable() {
assertThat(service.createTable(simpleTable(TABLE_NAME)),
willBe((Object) null));
@@ -1045,6 +1063,20 @@ public class CatalogServiceSelfTest {
assertThat(service.createIndex(params),
willThrow(IndexAlreadyExistsException.class));
}
+ @Test
+ public void testCreateIndexOnDuplicateColumns() {
+ assertThat(service.createTable(simpleTable(TABLE_NAME)),
willBe((Object) null));
+
+ CreateHashIndexParams params = CreateHashIndexParams.builder()
+ .indexName(INDEX_NAME)
+ .tableName(TABLE_NAME)
+ .columns(List.of("VAL", "VAL"))
+ .build();
+
+ assertThat(service.createIndex(params),
+ willThrow(IgniteInternalException.class, "Can't create index
on duplicate columns: VAL, VAL"));
+ }
+
@Test
public void operationWillBeRetriedFiniteAmountOfTimes() {
UpdateLog updateLogMock = Mockito.mock(UpdateLog.class);
diff --git a/modules/core/src/main/java/org/apache/ignite/lang/ErrorGroups.java
b/modules/core/src/main/java/org/apache/ignite/lang/ErrorGroups.java
index 9c14a0032a..d924c06b6d 100755
--- a/modules/core/src/main/java/org/apache/ignite/lang/ErrorGroups.java
+++ b/modules/core/src/main/java/org/apache/ignite/lang/ErrorGroups.java
@@ -65,6 +65,9 @@ public class ErrorGroups {
/** Table is stopping. */
public static final int TABLE_STOPPING_ERR =
TABLE_ERR_GROUP.registerErrorCode(5);
+
+ /** Table definition is incorrect. */
+ public static final int TABLE_DEFINITION_ERR =
TABLE_ERR_GROUP.registerErrorCode(6);
}
/** Client error group. */
diff --git
a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlAsynchronousApiTest.java
b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlAsynchronousApiTest.java
index 6c70d570f8..cef5e436d1 100644
---
a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlAsynchronousApiTest.java
+++
b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlAsynchronousApiTest.java
@@ -132,6 +132,12 @@ public class ItSqlAsynchronousApiTest extends
ClusterPerClassIntegrationTest {
ses,
"CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)"
);
+ checkError(
+ IgniteException.class,
+ "Can't create table with duplicate columns: ID, VAL, VAL",
+ ses,
+ "CREATE TABLE TEST1(ID INT PRIMARY KEY, VAL INT, VAL INT)"
+ );
checkDdl(false, ses, "CREATE TABLE IF NOT EXISTS TEST(ID INT PRIMARY
KEY, VAL VARCHAR)");
// ADD COLUMN
@@ -167,6 +173,12 @@ public class ItSqlAsynchronousApiTest extends
ClusterPerClassIntegrationTest {
checkDdl(true, ses, "CREATE INDEX TEST_IDX1 ON TEST(VAL0)");
checkDdl(true, ses, "CREATE INDEX TEST_IDX2 ON TEST(VAL0)");
checkDdl(true, ses, "CREATE INDEX TEST_IDX3 ON TEST(ID, VAL0, VAL1)");
+ checkError(
+ SqlException.class,
+ "Can't create index on duplicate columns: VAL0, VAL0",
+ ses,
+ "CREATE INDEX TEST_IDX4 ON TEST(VAL0, VAL0)"
+ );
checkError(
SqlException.class,
diff --git
a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlSynchronousApiTest.java
b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlSynchronousApiTest.java
index 976ea3c716..13533f4825 100644
---
a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlSynchronousApiTest.java
+++
b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlSynchronousApiTest.java
@@ -114,6 +114,12 @@ public class ItSqlSynchronousApiTest extends
ClusterPerClassIntegrationTest {
ses,
"CREATE TABLE TEST(ID INT PRIMARY KEY, VAL0 INT)"
);
+ checkError(
+ SqlException.class,
+ "Can't create table with duplicate columns: ID, VAL, VAL",
+ ses,
+ "CREATE TABLE TEST1(ID INT PRIMARY KEY, VAL INT, VAL INT)"
+ );
checkDdl(false, ses, "CREATE TABLE IF NOT EXISTS TEST(ID INT PRIMARY
KEY, VAL VARCHAR)");
// ADD COLUMN
@@ -149,6 +155,12 @@ public class ItSqlSynchronousApiTest extends
ClusterPerClassIntegrationTest {
checkDdl(true, ses, "CREATE INDEX TEST_IDX1 ON TEST(VAL0)");
checkDdl(true, ses, "CREATE INDEX TEST_IDX2 ON TEST(VAL0)");
checkDdl(true, ses, "CREATE INDEX TEST_IDX3 ON TEST(ID, VAL0, VAL1)");
+ checkError(
+ SqlException.class,
+ "Can't create index on duplicate columns: VAL0, VAL0",
+ ses,
+ "CREATE INDEX TEST_IDX4 ON TEST(VAL0, VAL0)"
+ );
checkError(
SqlException.class,
diff --git
a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ddl/DdlCommandHandler.java
b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ddl/DdlCommandHandler.java
index b5fe932de4..8f8b0b6b6d 100644
---
a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ddl/DdlCommandHandler.java
+++
b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ddl/DdlCommandHandler.java
@@ -38,6 +38,7 @@ import java.util.concurrent.CompletionException;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.BiFunction;
import java.util.function.Consumer;
+import java.util.function.Predicate;
import java.util.stream.Collectors;
import org.apache.ignite.configuration.NamedListView;
import
org.apache.ignite.internal.distributionzones.DistributionZoneConfigurationParameters;
@@ -92,6 +93,8 @@ import org.apache.ignite.lang.ColumnAlreadyExistsException;
import org.apache.ignite.lang.ColumnNotFoundException;
import org.apache.ignite.lang.DistributionZoneAlreadyExistsException;
import org.apache.ignite.lang.DistributionZoneNotFoundException;
+import org.apache.ignite.lang.ErrorGroups;
+import org.apache.ignite.lang.ErrorGroups.Table;
import org.apache.ignite.lang.IgniteInternalCheckedException;
import org.apache.ignite.lang.IgniteStringBuilder;
import org.apache.ignite.lang.IgniteStringFormatter;
@@ -257,6 +260,15 @@ public class DdlCommandHandler {
/** Handles create table command. */
private CompletableFuture<Boolean> handleCreateTable(CreateTableCommand
cmd) {
+ cmd.columns().stream()
+ .map(ColumnDefinition::name)
+ .filter(Predicate.not(new HashSet<>()::add))
+ .findAny()
+ .ifPresent(col -> {
+ throw new SqlException(Table.TABLE_DEFINITION_ERR, "Can't
create table with duplicate columns: "
+ +
cmd.columns().stream().map(ColumnDefinition::name).collect(Collectors.joining(",
")));
+ });
+
Consumer<TableChange> tblChanger = tableChange -> {
tableChange.changeColumns(columnsChange -> {
for (var col : cmd.columns()) {
@@ -284,7 +296,7 @@ public class DdlCommandHandler {
zoneName = DEFAULT_ZONE_NAME;
}
- return tableManager.createTableAsync(cmd.tableName(), zoneName,
tblChanger)
+ return tableManager.createTableAsync(cmd.tableName(), zoneName,
tblChanger)
.thenApply(Objects::nonNull)
.handle(handleModificationResult(cmd.ifTableExists(),
TableAlreadyExistsException.class));
}
@@ -334,6 +346,14 @@ public class DdlCommandHandler {
/** Handles create index command. */
private CompletableFuture<Boolean> handleCreateIndex(CreateIndexCommand
cmd) {
+ cmd.columns().stream()
+ .filter(Predicate.not(new HashSet<>()::add))
+ .findAny()
+ .ifPresent(col -> {
+ throw new
SqlException(ErrorGroups.Index.INVALID_INDEX_DEFINITION_ERR,
+ "Can't create index on duplicate columns: " +
String.join(", ", cmd.columns()));
+ });
+
Consumer<TableIndexChange> indexChanger = tableIndexChange -> {
switch (cmd.type()) {
case SORTED:
@@ -350,11 +370,11 @@ public class DdlCommandHandler {
};
return indexManager.createIndexAsync(
- cmd.schemaName(),
- cmd.indexName(),
- cmd.tableName(),
- !cmd.ifNotExists(),
- indexChanger);
+ cmd.schemaName(),
+ cmd.indexName(),
+ cmd.tableName(),
+ !cmd.ifNotExists(),
+ indexChanger);
}
/** Handles drop index command. */
