You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by am...@apache.org on 2023/07/06 15:22:53 UTC
[ignite-3] 01/01: Improve test coverage for CREATE TABLE.
This is an automated email from the ASF dual-hosted git repository.
amashenkov pushed a commit to branch ignite-19927
in repository https://gitbox.apache.org/repos/asf/ignite-3.git
commit 91973f02c4e6c4783588a9b963fe3ceba2a8559e
Author: amashenkov <an...@gmail.com>
AuthorDate: Tue Jul 4 23:45:34 2023 +0300
Improve test coverage for CREATE TABLE.
---
.../internal/catalog/CatalogServiceImpl.java | 67 ++++++++++-
.../internal/catalog/CatalogServiceSelfTest.java | 123 +++++++++++++++++----
.../internal/sql/engine/ItCreateTableDdlTest.java | 39 ++++++-
.../schema/configuration/TableValidatorImpl.java | 7 +-
4 files changed, 209 insertions(+), 27 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 22d3563dc5..67be512782 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
@@ -19,6 +19,7 @@ package org.apache.ignite.internal.catalog;
import static java.util.concurrent.CompletableFuture.completedFuture;
import static java.util.concurrent.CompletableFuture.failedFuture;
+import static java.util.function.Predicate.not;
import static java.util.stream.Collectors.joining;
import static org.apache.ignite.internal.catalog.commands.CreateZoneParams.INFINITE_TIMER_VALUE;
import static org.apache.ignite.lang.ErrorGroups.Sql.UNSUPPORTED_DDL_OPERATION_ERR;
@@ -31,10 +32,12 @@ import java.util.List;
import java.util.Map.Entry;
import java.util.NavigableMap;
import java.util.Objects;
+import java.util.Set;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ConcurrentSkipListMap;
import java.util.function.LongSupplier;
import java.util.function.Predicate;
+import java.util.stream.Collectors;
import org.apache.ignite.internal.catalog.commands.AlterColumnParams;
import org.apache.ignite.internal.catalog.commands.AlterTableAddColumnParams;
import org.apache.ignite.internal.catalog.commands.AlterTableDropColumnParams;
@@ -737,9 +740,15 @@ public class CatalogServiceImpl extends Producer<CatalogEvent, CatalogEventParam
}
private static void validateCreateTableParams(CreateTableParams params) {
+ // Table must have columns.
+ if (params.columns().isEmpty()) {
+ throw new IgniteInternalException(ErrorGroups.Index.INVALID_INDEX_DEFINITION_ERR, "Table must include at least one column.");
+ }
+
+ // Column names must be unique.
params.columns().stream()
.map(ColumnParams::name)
- .filter(Predicate.not(new HashSet<>()::add))
+ .filter(not(new HashSet<>()::add))
.findAny()
.ifPresent(columnName -> {
throw new IgniteInternalException(
@@ -749,12 +758,64 @@ public class CatalogServiceImpl extends Producer<CatalogEvent, CatalogEventParam
);
});
+ // Table must have PK columns.
if (params.primaryKeyColumns().isEmpty()) {
throw new IgniteInternalException(
ErrorGroups.Index.INVALID_INDEX_DEFINITION_ERR,
- "Missing primary key columns"
+ "Table without primary key is not supported."
);
}
+
+ // PK columns must be valid columns.
+ Set<String> columns = params.columns().stream().map(ColumnParams::name).collect(Collectors.toSet());
+ params.primaryKeyColumns().stream()
+ .filter(not(columns::contains))
+ .findAny()
+ .ifPresent(columnName -> {
+ throw new IgniteInternalException(
+ ErrorGroups.Index.INVALID_INDEX_DEFINITION_ERR,
+ "Invalid primary key columns: {}",
+ params.columns().stream().map(ColumnParams::name).collect(joining(", "))
+ );
+ });
+
+ // PK column names must be unique.
+ params.primaryKeyColumns().stream()
+ .filter(not(new HashSet<>()::add))
+ .findAny()
+ .ifPresent(columnName -> {
+ throw new IgniteInternalException(
+ ErrorGroups.Index.INVALID_INDEX_DEFINITION_ERR,
+ "Primary key columns contains duplicates: {}",
+ String.join(", ", params.primaryKeyColumns())
+ );
+ });
+
+ List<String> colocationCols = params.colocationColumns();
+ if (colocationCols != null) {
+ // Colocation columns must be unique
+ colocationCols.stream()
+ .filter(not(new HashSet<>()::add))
+ .findAny()
+ .ifPresent(columnName -> {
+ throw new IgniteInternalException(
+ ErrorGroups.Index.INVALID_INDEX_DEFINITION_ERR,
+ "Colocation columns contains duplicates: {}",
+ String.join(", ", colocationCols)
+ );
+ });
+
+ // Colocation column must be valid PK column
+ Set<String> pkColumns = new HashSet<>(params.primaryKeyColumns());
+ List<String> outstandingColumns = colocationCols.stream()
+ .filter(not(pkColumns::contains))
+ .collect(Collectors.toList());
+ if (!outstandingColumns.isEmpty()) {
+ throw new IgniteInternalException(
+ ErrorGroups.Index.INVALID_INDEX_DEFINITION_ERR,
+ "Colocation columns must be subset of primary key: outstandingColumns=" + outstandingColumns);
+ }
+ }
}
private static void validateAlterTableDropColumnParams(
@@ -826,7 +887,7 @@ public class CatalogServiceImpl extends Producer<CatalogEvent, CatalogEventParam
);
}
- Predicate<String> duplicateValidator = Predicate.not(new HashSet<>()::add);
+ Predicate<String> duplicateValidator = not(new HashSet<>()::add);
for (String columnName : indexColumns) {
CatalogTableColumnDescriptor columnDescriptor = table.columnDescriptor(columnName);
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 b2326f0142..5365b85d16 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
@@ -305,24 +305,6 @@ 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(nullValue()));
@@ -1738,7 +1720,8 @@ public class CatalogServiceSelfTest {
}
@Test
- void testCreateTableWithoutPkColumns() {
+ void testCreateTableErrors() {
+ // Table must have at least one column.
assertThat(
service.createTable(
CreateTableParams.builder()
@@ -1749,8 +1732,108 @@ public class CatalogServiceSelfTest {
.primaryKeyColumns(List.of())
.build()
),
- willThrowFast(IgniteInternalException.class, "Missing primary key columns")
+ willThrowFast(IgniteInternalException.class, "Table must include at least one column.")
);
+
+ // Table must have PK columns.
+ assertThat(
+ service.createTable(
+ CreateTableParams.builder()
+ .schemaName(SCHEMA_NAME)
+ .zone(ZONE_NAME)
+ .tableName(TABLE_NAME)
+ .columns(List.of(
+ ColumnParams.builder().name("val").type(ColumnType.INT32).build()
+ ))
+ .primaryKeyColumns(List.of())
+ .build()
+ ),
+ willThrowFast(IgniteInternalException.class, "Table without primary key is not supported.")
+ );
+
+ // PK column must be a valid column
+ assertThat(
+ service.createTable(
+ CreateTableParams.builder()
+ .schemaName(SCHEMA_NAME)
+ .zone(ZONE_NAME)
+ .tableName(TABLE_NAME)
+ .columns(List.of(
+ ColumnParams.builder().name("val").type(ColumnType.INT32).build()
+ ))
+ .primaryKeyColumns(List.of("key"))
+ .build()
+ ),
+ willThrowFast(IgniteInternalException.class, "Invalid primary key columns: val")
+ );
+
+ // Column names must be unique.
+ assertThat(
+ service.createTable(
+ 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()
+ ),
+ willThrowFast(IgniteInternalException.class, "Can't create table with duplicate columns: key, val, val"));
+
+ // PK column names must be unique.
+ assertThat(
+ service.createTable(
+ CreateTableParams.builder()
+ .schemaName(SCHEMA_NAME)
+ .tableName(TABLE_NAME)
+ .zone(ZONE_NAME)
+ .columns(List.of(
+ ColumnParams.builder().name("key1").type(ColumnType.INT32).build(),
+ ColumnParams.builder().name("key2").type(ColumnType.INT32).build()
+ ))
+ .primaryKeyColumns(List.of("key1", "key2", "key1"))
+ .build()
+ ),
+ willThrowFast(IgniteInternalException.class, "Primary key columns contains duplicates: key1, key2, key1"));
+
+ // Colocated columns names must be unique.
+ assertThat(
+ service.createTable(
+ CreateTableParams.builder()
+ .schemaName(SCHEMA_NAME)
+ .tableName(TABLE_NAME)
+ .zone(ZONE_NAME)
+ .columns(List.of(
+ ColumnParams.builder().name("key1").type(ColumnType.INT32).build(),
+ ColumnParams.builder().name("key2").type(ColumnType.INT32).build(),
+ ColumnParams.builder().name("val").type(ColumnType.INT32).build()
+ ))
+ .primaryKeyColumns(List.of("key1", "key2"))
+ .colocationColumns(List.of("key1", "key2", "key1"))
+ .build()
+ ),
+ willThrowFast(IgniteInternalException.class, "Colocation columns contains duplicates: key1, key2, key1"));
+
+ // Colocated columns must be valid primary key columns.
+ assertThat(
+ service.createTable(
+ 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()
+ ))
+ .primaryKeyColumns(List.of("key"))
+ .colocationColumns(List.of("val"))
+ .build()
+ ),
+ willThrowFast(IgniteInternalException.class, "Colocation columns must be subset of primary key: outstandingColumns=[val]"));
}
private CompletableFuture<Void> changeColumn(
diff --git a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItCreateTableDdlTest.java b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItCreateTableDdlTest.java
index 7237f5267b..9ee1ea729d 100644
--- a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItCreateTableDdlTest.java
+++ b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItCreateTableDdlTest.java
@@ -70,6 +70,43 @@ public class ItCreateTableDdlTest extends ClusterPerClassIntegrationTest {
);
}
+ @Test
+ public void pkWithInvalidColumns() {
+ assertThrows(
+ IgniteException.class,
+ () -> sql("CREATE TABLE T0(ID0 INT, ID1 INT, VAL INT, PRIMARY KEY (ID2, ID0))"),
+ "Primary key cannot contain nullable column [col=ID0]"
+ );
+ }
+
+ @Test
+ public void emptyPk() {
+ assertThrows(
+ IgniteException.class,
+ () -> sql("CREATE TABLE T0(ID0 INT, ID1 INT, VAL INT, PRIMARY KEY ())"),
+ "Table without primary key is not supported."
+ );
+ assertThrows(
+ IgniteException.class,
+ () -> sql("CREATE TABLE T0(ID0 INT, ID1 INT, VAL INT)"),
+ "Table without primary key is not supported."
+ );
+ }
+
+ @Test
+ public void tableWithInvalidColumns() {
+ assertThrows(
+ IgniteException.class,
+ () -> sql("CREATE TABLE T0()"),
+ "Table must include at least one column."
+ );
+ assertThrows(
+ IgniteException.class,
+ () -> sql("CREATE TABLE T0(ID0 INT, ID1 INT, ID0 INT)"),
+ "Can't create table with duplicate columns"
+ );
+ }
+
@Test
public void pkWithFunctionalDefault() {
sql("create table t (id varchar default gen_random_uuid primary key, val int)");
@@ -111,7 +148,7 @@ public class ItCreateTableDdlTest extends ClusterPerClassIntegrationTest {
IgniteException.class,
() -> sql("CREATE TABLE T0(ID0 INT, ID1 INT, VAL INT, PRIMARY KEY (ID1, ID0)) COLOCATE (ID1, ID0, ID1)")
).getMessage(),
- containsString("Colocation columns contains duplicates")
+ containsString("Colocation columns contains duplicates: [duplicates=[ID1]]]")
);
}
diff --git a/modules/schema/src/main/java/org/apache/ignite/internal/schema/configuration/TableValidatorImpl.java b/modules/schema/src/main/java/org/apache/ignite/internal/schema/configuration/TableValidatorImpl.java
index 7f3c6f56eb..e074098f35 100644
--- a/modules/schema/src/main/java/org/apache/ignite/internal/schema/configuration/TableValidatorImpl.java
+++ b/modules/schema/src/main/java/org/apache/ignite/internal/schema/configuration/TableValidatorImpl.java
@@ -26,6 +26,7 @@ import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
+import java.util.stream.Collectors;
import org.apache.ignite.configuration.NamedListView;
import org.apache.ignite.configuration.validation.ValidationContext;
import org.apache.ignite.configuration.validation.ValidationIssue;
@@ -55,7 +56,7 @@ public class TableValidatorImpl implements Validator<TableValidator, NamedListVi
TableView newTable = newTables.get(tableName);
if (newTable.columns() == null || newTable.columns().size() == 0) {
- ctx.addIssue(new ValidationIssue(newTable.name(), "Table should include at least one column"));
+ ctx.addIssue(new ValidationIssue(newTable.name(), "Table must include at least one column."));
// no further validation required
return;
@@ -65,7 +66,7 @@ public class TableValidatorImpl implements Validator<TableValidator, NamedListVi
if (!columnsDups.isEmpty()) {
ctx.addIssue(new ValidationIssue(newTable.name(),
- "Some columns are specified more than once [duplicates=" + new HashSet<>(columnsDups) + "]"));
+ "Can't create table with duplicate columns " + String.join(",", columnsDups)));
}
Set<String> columns = new HashSet<>(newTable.columns().namedListKeys());
@@ -113,7 +114,7 @@ public class TableValidatorImpl implements Validator<TableValidator, NamedListVi
if (!colocationDups.isEmpty()) {
ctx.addIssue(new ValidationIssue(newTable.name(),
- "Colocation columns contains duplicates [duplicates=" + colocationDups + "]"));
+ "Colocation columns contains duplicates: [duplicates=" + colocationDups + "]"));
}
}
}