This is an automated email from the ASF dual-hosted git repository. tkalkirill 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 2dd63c8ec5 IGNITE-20269 Check and slightly refactor the validation of tables in the catalog (#2493) 2dd63c8ec5 is described below commit 2dd63c8ec531eb11724598b178dc55671b79fc9b Author: Kirill Tkalenko <tkalkir...@yandex.ru> AuthorDate: Fri Aug 25 12:03:00 2023 +0300 IGNITE-20269 Check and slightly refactor the validation of tables in the catalog (#2493) --- .../java/org/apache/ignite/sql/ColumnType.java | 1 - .../internal/catalog/CatalogManagerImpl.java | 192 ++----- .../catalog/CatalogParamsValidationUtils.java | 166 +++++- .../commands/AbstractTableCommandParams.java | 46 +- .../catalog/commands/AlterColumnParams.java | 101 ++-- .../commands/AlterTableAddColumnParams.java | 21 +- .../commands/AlterTableDropColumnParams.java | 25 +- .../internal/catalog/commands/ColumnParams.java | 53 +- .../catalog/commands/CreateTableParams.java | 59 +- .../internal/catalog/commands/DropTableParams.java | 17 +- .../internal/catalog/CatalogManagerSelfTest.java | 598 ++++++--------------- .../catalog/CatalogManagerValidationTest.java | 182 +++++++ .../internal/catalog/BaseCatalogManagerTest.java | 63 +++ .../exec/ddl/DdlToCatalogCommandConverter.java | 12 +- 14 files changed, 791 insertions(+), 745 deletions(-) diff --git a/modules/api/src/main/java/org/apache/ignite/sql/ColumnType.java b/modules/api/src/main/java/org/apache/ignite/sql/ColumnType.java index 3247f0c821..683dbe264f 100644 --- a/modules/api/src/main/java/org/apache/ignite/sql/ColumnType.java +++ b/modules/api/src/main/java/org/apache/ignite/sql/ColumnType.java @@ -92,7 +92,6 @@ public enum ColumnType { /** Null. */ NULL; - /** * Column type to Java class. */ 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 6d42b93a12..dfb522bcf4 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 @@ -19,12 +19,17 @@ package org.apache.ignite.internal.catalog; import static java.util.concurrent.CompletableFuture.completedFuture; import static java.util.concurrent.CompletableFuture.failedFuture; -import static java.util.stream.Collectors.joining; +import static java.util.stream.Collectors.toList; +import static org.apache.ignite.internal.catalog.CatalogParamsValidationUtils.validateAddColumnParams; +import static org.apache.ignite.internal.catalog.CatalogParamsValidationUtils.validateAlterColumnParams; import static org.apache.ignite.internal.catalog.CatalogParamsValidationUtils.validateAlterZoneParams; import static org.apache.ignite.internal.catalog.CatalogParamsValidationUtils.validateCreateHashIndexParams; import static org.apache.ignite.internal.catalog.CatalogParamsValidationUtils.validateCreateSortedIndexParams; +import static org.apache.ignite.internal.catalog.CatalogParamsValidationUtils.validateCreateTableParams; import static org.apache.ignite.internal.catalog.CatalogParamsValidationUtils.validateCreateZoneParams; +import static org.apache.ignite.internal.catalog.CatalogParamsValidationUtils.validateDropColumnParams; import static org.apache.ignite.internal.catalog.CatalogParamsValidationUtils.validateDropIndexParams; +import static org.apache.ignite.internal.catalog.CatalogParamsValidationUtils.validateDropTableParams; import static org.apache.ignite.internal.catalog.CatalogParamsValidationUtils.validateDropZoneParams; import static org.apache.ignite.internal.catalog.CatalogParamsValidationUtils.validateRenameZoneParams; import static org.apache.ignite.internal.catalog.commands.CatalogUtils.fromParams; @@ -35,18 +40,13 @@ import static org.apache.ignite.lang.IgniteStringFormatter.format; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; -import java.util.Collections; -import java.util.HashSet; 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.AbstractCreateIndexCommandParams; import org.apache.ignite.internal.catalog.commands.AlterColumnParams; import org.apache.ignite.internal.catalog.commands.AlterTableAddColumnParams; @@ -98,10 +98,10 @@ import org.apache.ignite.internal.manager.Producer; import org.apache.ignite.internal.util.PendingComparableValuesTracker; import org.apache.ignite.lang.ColumnAlreadyExistsException; import org.apache.ignite.lang.ColumnNotFoundException; -import org.apache.ignite.lang.ErrorGroups; import org.apache.ignite.lang.ErrorGroups.Common; import org.apache.ignite.lang.ErrorGroups.DistributionZones; import org.apache.ignite.lang.ErrorGroups.Index; +import org.apache.ignite.lang.ErrorGroups.Table; import org.apache.ignite.lang.IgniteException; import org.apache.ignite.lang.IgniteInternalException; import org.apache.ignite.lang.IndexAlreadyExistsException; @@ -315,13 +315,11 @@ public class CatalogManagerImpl extends Producer<CatalogEvent, CatalogEventParam @Override public CompletableFuture<Void> createTable(CreateTableParams params) { return saveUpdateAndWaitForActivation(catalog -> { - CatalogSchemaDescriptor schema = getSchema(catalog, params.schemaName()); + validateCreateTableParams(params); - if (schema.table(params.tableName()) != null) { - throw new TableAlreadyExistsException(schema.name(), params.tableName()); - } + CatalogSchemaDescriptor schema = getSchema(catalog, params.schemaName()); - validateCreateTableParams(params); + ensureNoTableOrIndexExistsWithSameName(schema, params.tableName()); CatalogZoneDescriptor zone = getZone(catalog, Objects.requireNonNullElse(params.zone(), DEFAULT_ZONE_NAME)); @@ -342,6 +340,8 @@ public class CatalogManagerImpl extends Producer<CatalogEvent, CatalogEventParam @Override public CompletableFuture<Void> dropTable(DropTableParams params) { return saveUpdateAndWaitForActivation(catalog -> { + validateDropTableParams(params); + CatalogSchemaDescriptor schema = getSchema(catalog, params.schemaName()); CatalogTableDescriptor table = getTable(schema, params.tableName()); @@ -360,11 +360,9 @@ public class CatalogManagerImpl extends Producer<CatalogEvent, CatalogEventParam @Override public CompletableFuture<Void> addColumn(AlterTableAddColumnParams params) { - if (params.columns().isEmpty()) { - return completedFuture(null); - } - return saveUpdateAndWaitForActivation(catalog -> { + validateAddColumnParams(params); + CatalogSchemaDescriptor schema = getSchema(catalog, params.schemaName()); CatalogTableDescriptor table = getTable(schema, params.tableName()); @@ -387,16 +385,14 @@ public class CatalogManagerImpl extends Producer<CatalogEvent, CatalogEventParam @Override public CompletableFuture<Void> dropColumn(AlterTableDropColumnParams params) { - if (params.columns().isEmpty()) { - return completedFuture(null); - } - return saveUpdateAndWaitForActivation(catalog -> { + validateDropColumnParams(params); + CatalogSchemaDescriptor schema = getSchema(catalog, params.schemaName()); CatalogTableDescriptor table = getTable(schema, params.tableName()); - validateAlterTableDropColumnParams(params, schema, table); + ensureColumnCanBeDropped(schema, table, params); return List.of( new DropColumnsEntry(table.id(), params.columns()) @@ -407,6 +403,8 @@ public class CatalogManagerImpl extends Producer<CatalogEvent, CatalogEventParam @Override public CompletableFuture<Void> alterColumn(AlterColumnParams params) { return saveUpdateAndWaitForActivation(catalog -> { + validateAlterColumnParams(params); + CatalogSchemaDescriptor schema = getSchema(catalog, params.schemaName()); CatalogTableDescriptor table = getTable(schema, params.tableName()); @@ -417,7 +415,7 @@ public class CatalogManagerImpl extends Producer<CatalogEvent, CatalogEventParam if (origin.equals(target)) { // No modifications required. - return Collections.emptyList(); + return List.of(); } boolean isPkColumn = table.isPrimaryKeyColumn(origin.name()); @@ -757,115 +755,6 @@ public class CatalogManagerImpl 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)) - .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(joining(", ")) - ); - }); - - // Table must have PK columns. - if (params.primaryKeyColumns().isEmpty()) { - throw new IgniteInternalException( - ErrorGroups.Index.INVALID_INDEX_DEFINITION_ERR, - "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(Predicate.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(Predicate.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(Predicate.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(Predicate.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( - AlterTableDropColumnParams params, - CatalogSchemaDescriptor schema, - CatalogTableDescriptor table - ) { - for (String columnName : params.columns()) { - if (table.column(columnName) == null) { - throw new ColumnNotFoundException(schema.name(), params.tableName(), columnName); - } - - if (table.isPrimaryKeyColumn(columnName)) { - throw new SqlException( - STMT_VALIDATION_ERR, - "Can't drop primary key column: [name=" + columnName + ']' - ); - } - } - - Arrays.stream(schema.indexes()) - .filter(index -> index.tableId() == table.id()) - .forEach(index -> params.columns().stream() - .filter(index::hasColumn) - .findAny() - .ifPresent(columnName -> { - throw new SqlException( - STMT_VALIDATION_ERR, - format("Can't drop indexed column: [columnName={}, indexName={}]", columnName, index.name())); - })); - } - private static void validateAlterTableColumn( CatalogTableColumnDescriptor origin, CatalogTableColumnDescriptor target, @@ -947,14 +836,45 @@ public class CatalogManagerImpl extends Producer<CatalogEvent, CatalogEventParam } private static void validateIndexColumns(CatalogTableDescriptor table, AbstractCreateIndexCommandParams params) { - for (String indexColumn : params.columns()) { - if (table.columnDescriptor(indexColumn) == null) { - throw new ColumnNotFoundException(indexColumn); - } - } + validateColumnsExistInTable(table, params.columns()); if (params.unique() && !params.columns().containsAll(table.colocationColumns())) { throw new IgniteException(Index.INVALID_INDEX_DEFINITION_ERR, "Unique index must include all colocation columns"); } } + + private static void ensureColumnCanBeDropped( + CatalogSchemaDescriptor schema, + CatalogTableDescriptor table, + AlterTableDropColumnParams params + ) { + validateColumnsExistInTable(table, params.columns()); + + List<String> inPrimaryKeyColumns = params.columns().stream() + .filter(table::isPrimaryKeyColumn) + .collect(toList()); + + if (!inPrimaryKeyColumns.isEmpty()) { + throw new CatalogValidationException(Table.TABLE_DEFINITION_ERR, "Can't drop primary key columns: " + inPrimaryKeyColumns); + } + + Arrays.stream(schema.indexes()) + .filter(index -> index.tableId() == table.id()) + .forEach(index -> params.columns().stream() + .filter(index::hasColumn) + .findAny() + .ifPresent(columnName -> { + throw new CatalogValidationException( + STMT_VALIDATION_ERR, + format("Can't drop indexed column: [columnName={}, indexName={}]", columnName, index.name())); + })); + } + + private static void validateColumnsExistInTable(CatalogTableDescriptor table, Collection<String> columns) { + for (String column : columns) { + if (table.columnDescriptor(column) == null) { + throw new ColumnNotFoundException(column); + } + } + } } diff --git a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogParamsValidationUtils.java b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogParamsValidationUtils.java index 4dbdba5ea9..8b5120d3d8 100644 --- a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogParamsValidationUtils.java +++ b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogParamsValidationUtils.java @@ -17,24 +17,36 @@ package org.apache.ignite.internal.catalog; +import static java.util.stream.Collectors.toList; import static org.apache.ignite.internal.catalog.commands.CatalogUtils.MAX_PARTITION_COUNT; import com.jayway.jsonpath.InvalidPathException; import com.jayway.jsonpath.JsonPath; +import java.util.Collection; import java.util.HashSet; +import java.util.List; +import java.util.Objects; import java.util.function.Predicate; import org.apache.ignite.internal.catalog.commands.AbstractCreateIndexCommandParams; import org.apache.ignite.internal.catalog.commands.AbstractIndexCommandParams; +import org.apache.ignite.internal.catalog.commands.AbstractTableCommandParams; +import org.apache.ignite.internal.catalog.commands.AlterColumnParams; +import org.apache.ignite.internal.catalog.commands.AlterTableAddColumnParams; +import org.apache.ignite.internal.catalog.commands.AlterTableDropColumnParams; import org.apache.ignite.internal.catalog.commands.AlterZoneParams; +import org.apache.ignite.internal.catalog.commands.ColumnParams; import org.apache.ignite.internal.catalog.commands.CreateHashIndexParams; import org.apache.ignite.internal.catalog.commands.CreateSortedIndexParams; +import org.apache.ignite.internal.catalog.commands.CreateTableParams; import org.apache.ignite.internal.catalog.commands.CreateZoneParams; import org.apache.ignite.internal.catalog.commands.DropIndexParams; +import org.apache.ignite.internal.catalog.commands.DropTableParams; import org.apache.ignite.internal.catalog.commands.DropZoneParams; import org.apache.ignite.internal.catalog.commands.RenameZoneParams; import org.apache.ignite.internal.util.CollectionUtils; import org.apache.ignite.lang.ErrorGroups.DistributionZones; import org.apache.ignite.lang.ErrorGroups.Index; +import org.apache.ignite.lang.ErrorGroups.Table; import org.apache.ignite.lang.util.StringUtils; import org.jetbrains.annotations.Nullable; @@ -73,9 +85,7 @@ class CatalogParamsValidationUtils { static void validateCreateSortedIndexParams(CreateSortedIndexParams params) { validateCommonCreateIndexParams(params); - if (CollectionUtils.nullOrEmpty(params.collations())) { - throw new CatalogValidationException(Index.INVALID_INDEX_DEFINITION_ERR, "Columns collations not specified"); - } + validateCollectionIsNotEmpty(params.collations(), Index.INVALID_INDEX_DEFINITION_ERR, "Columns collations not specified"); if (params.collations().size() != params.columns().size()) { throw new CatalogValidationException(Index.INVALID_INDEX_DEFINITION_ERR, "Columns collations doesn't match number of columns"); @@ -95,6 +105,85 @@ class CatalogParamsValidationUtils { validateZoneName(params.newZoneName(), "Missing new zone name"); } + static void validateDropTableParams(DropTableParams params) { + validateCommonTableParams(params); + } + + static void validateCreateTableParams(CreateTableParams params) { + validateCommonTableParams(params); + + List<String> columnNames = Objects.<List<ColumnParams>>requireNonNullElse(params.columns(), List.of()).stream() + .peek(CatalogParamsValidationUtils::validateColumnParams) + .map(ColumnParams::name) + .collect(toList()); + + validateColumns( + columnNames, + Table.TABLE_DEFINITION_ERR, + "Columns not specified", + "Duplicate columns are present: {}" + ); + + validateColumns( + params.primaryKeyColumns(), + Table.TABLE_DEFINITION_ERR, + "Primary key columns not specified", + "Duplicate primary key columns are present: {}" + ); + + validateColumnsAreContainedInAnotherColumns( + params.primaryKeyColumns(), + columnNames, + Table.TABLE_DEFINITION_ERR, + "Primary key columns missing in columns: {}" + ); + + if (params.colocationColumns() != null) { + validateColumns( + params.colocationColumns(), + Table.TABLE_DEFINITION_ERR, + "Colocation columns not specified", + "Duplicate colocation columns are present: {}" + ); + + validateColumnsAreContainedInAnotherColumns( + params.colocationColumns(), + params.primaryKeyColumns(), + Table.TABLE_DEFINITION_ERR, + "Colocation columns missing in primary key columns: {}" + ); + } + } + + static void validateDropColumnParams(AlterTableDropColumnParams params) { + validateCommonTableParams(params); + + validateCollectionIsNotEmpty(params.columns(), Table.TABLE_DEFINITION_ERR, "Columns not specified"); + } + + static void validateAddColumnParams(AlterTableAddColumnParams params) { + validateCommonTableParams(params); + + List<String> columnNames = Objects.<List<ColumnParams>>requireNonNullElse(params.columns(), List.of()).stream() + .peek(CatalogParamsValidationUtils::validateColumnParams) + .map(ColumnParams::name) + .collect(toList()); + + validateColumns( + columnNames, + Table.TABLE_DEFINITION_ERR, + "Columns not specified", + "Duplicate columns are present: {}" + ); + } + + // TODO: IGNITE-19938 Add validation column length, precision and scale + static void validateAlterColumnParams(AlterColumnParams params) { + validateCommonTableParams(params); + + validateNameField(params.columnName(), Table.TABLE_DEFINITION_ERR, "Missing column name"); + } + private static void validateUpdateZoneFieldsParameters( String zoneName, @Nullable Integer partitions, @@ -206,20 +295,12 @@ class CatalogParamsValidationUtils { validateNameField(params.tableName(), Index.INVALID_INDEX_DEFINITION_ERR, "Missing table name"); - if (CollectionUtils.nullOrEmpty(params.columns())) { - throw new CatalogValidationException(Index.INVALID_INDEX_DEFINITION_ERR, "Columns not specified"); - } - - params.columns().stream() - .filter(Predicate.not(new HashSet<>()::add)) - .findAny() - .ifPresent(columnName -> { - throw new CatalogValidationException( - Index.INVALID_INDEX_DEFINITION_ERR, - "Duplicate columns are present: {}", - params.columns() - ); - }); + validateColumns( + params.columns(), + Index.INVALID_INDEX_DEFINITION_ERR, + "Columns not specified", + "Duplicate columns are present: {}" + ); } private static void validateNameField(String name, int errorCode, String errorMessage) { @@ -227,4 +308,55 @@ class CatalogParamsValidationUtils { throw new CatalogValidationException(errorCode, errorMessage); } } + + private static void validateCollectionIsNotEmpty(Collection<?> collection, int errorCode, String errorMessage) { + if (CollectionUtils.nullOrEmpty(collection)) { + throw new CatalogValidationException(errorCode, errorMessage); + } + } + + private static void validateColumns( + List<String> columns, + int errorCode, + String emptyColumnsErrorMessage, + String duplicateColumnsErrorMessageFormat + ) { + validateCollectionIsNotEmpty(columns, errorCode, emptyColumnsErrorMessage); + + List<String> duplicates = columns.stream() + .filter(Predicate.not(new HashSet<>()::add)) + .collect(toList()); + + if (!duplicates.isEmpty()) { + throw new CatalogValidationException(errorCode, duplicateColumnsErrorMessageFormat, duplicates); + } + } + + private static void validateColumnsAreContainedInAnotherColumns( + List<String> columnsToCheck, + List<String> anotherColumns, + int errorCode, + String errorMessageFormat + ) { + List<String> notContained = columnsToCheck.stream() + .filter(Predicate.not(anotherColumns::contains)) + .collect(toList()); + + if (!notContained.isEmpty()) { + throw new CatalogValidationException(errorCode, errorMessageFormat, notContained); + } + } + + private static void validateCommonTableParams(AbstractTableCommandParams params) { + validateNameField(params.tableName(), Table.TABLE_DEFINITION_ERR, "Missing table name"); + } + + // TODO: IGNITE-19938 Add validation column length, precision and scale + private static void validateColumnParams(ColumnParams params) { + validateNameField(params.name(), Table.TABLE_DEFINITION_ERR, "Missing column name"); + + if (params.type() == null) { + throw new CatalogValidationException(Table.TABLE_DEFINITION_ERR, "Missing column type: " + params.name()); + } + } } diff --git a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/AbstractTableCommandParams.java b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/AbstractTableCommandParams.java index 67c7a588e8..0afb2fcda2 100644 --- a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/AbstractTableCommandParams.java +++ b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/AbstractTableCommandParams.java @@ -17,67 +17,59 @@ package org.apache.ignite.internal.catalog.commands; -/** - * Abstract table ddl command. - */ -public class AbstractTableCommandParams implements DdlCommandParams { +import org.jetbrains.annotations.Nullable; + +/** Abstract table ddl command. */ +public abstract class AbstractTableCommandParams implements DdlCommandParams { /** Table name. */ protected String tableName; - /** Schema name where this new table will be created. */ - protected String schema; + /** Schema name, {@code null} means to use the default schema. */ + protected @Nullable String schema; - /** - * Returns table simple name. - */ + /** Returns table name. */ public String tableName() { return tableName; } - /** - * Returns schema name. - */ - public String schemaName() { + /** Returns schema name, {@code null} means to use the default schema. */ + public @Nullable String schemaName() { return schema; } - /** - * Parameters builder. - */ - protected abstract static class AbstractBuilder<ParamT extends AbstractTableCommandParams, BuilderT> { + /** Parameters builder. */ + protected abstract static class AbstractTableBuilder<ParamT extends AbstractTableCommandParams, BuilderT> { protected ParamT params; - AbstractBuilder(ParamT params) { + AbstractTableBuilder(ParamT params) { this.params = params; } /** * Sets schema name. * - * @param schemaName Schema name. + * @param schemaName Schema name, {@code null} to use to use the default schema. * @return {@code this}. */ - public BuilderT schemaName(String schemaName) { + public BuilderT schemaName(@Nullable String schemaName) { params.schema = schemaName; + return (BuilderT) this; } /** - * Sets table simple name. + * Sets table name. * - * @param tableName Table simple name. + * @param tableName Table name. * @return {@code this}. */ public BuilderT tableName(String tableName) { params.tableName = tableName; + return (BuilderT) this; } - /** - * Builds parameters. - * - * @return Parameters. - */ + /** Builds parameters. */ public ParamT build() { ParamT params0 = params; params = null; diff --git a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/AlterColumnParams.java b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/AlterColumnParams.java index b149a3f794..d8a6204250 100644 --- a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/AlterColumnParams.java +++ b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/AlterColumnParams.java @@ -21,116 +21,151 @@ import java.util.function.Function; import org.apache.ignite.sql.ColumnType; import org.jetbrains.annotations.Nullable; -/** - * ALTER TABLE ... ALTER COLUMN statement. - */ -@SuppressWarnings("AssignmentOrReturnOfFieldWithMutableType") +/** ALTER TABLE ... ALTER COLUMN statement. */ public class AlterColumnParams extends AbstractTableCommandParams { + /** Creates parameters builder. */ + public static AlterColumnParams.Builder builder() { + return new AlterColumnParams.Builder(); + } + + private AlterColumnParams() { + // No-op. + } + private String columnName; - private ColumnType type; + private @Nullable ColumnType type; - private Integer precision; + private @Nullable Integer precision; - private Integer length; + private @Nullable Integer length; - private Integer scale; + private @Nullable Integer scale; - private Boolean notNull; + private @Nullable Boolean notNull; - private Function<ColumnType, DefaultValue> defaultResolver; + private @Nullable Function<ColumnType, DefaultValue> defaultResolver; /** Returns column name. */ public String columnName() { return columnName; } - /** Returns column type. */ + /** Returns column type, {@code null} if not set. */ public @Nullable ColumnType type() { return type; } - /** Returns column precision. */ + /** Returns column precision, {@code null} if not set. */ public @Nullable Integer precision() { return precision; } - /** Returns column length. */ + /** Returns column length, {@code null} if not set. */ public @Nullable Integer length() { return length; } - /** Returns column scale. */ + /** Returns column scale, {@code null} if not set. */ public @Nullable Integer scale() { return scale; } - /** Returns the {@code NOT NULL} constraint change flag. */ + /** Returns the {@code NOT NULL} constraint change flag, {@code null} if not set. */ public @Nullable Boolean notNull() { return notNull; } - /** Returns a default value depending on the column type. */ + /** Returns a default value depending on the column type, {@code null} if not set. */ public @Nullable DefaultValue defaultValue(ColumnType type) { return defaultResolver == null ? null : defaultResolver.apply(type); } - public static AlterColumnParams.Builder builder() { - return new AlterColumnParams.Builder(); - } - - /** - * Parameters builder. - */ - public static class Builder extends AbstractBuilder<AlterColumnParams, Builder> { + /** Parameters builder. */ + public static class Builder extends AbstractTableBuilder<AlterColumnParams, Builder> { private Builder() { super(new AlterColumnParams()); } - /** Sets column name. */ + /** + * Sets column name. + * + * @param name Colum name. + * @return {@code this}. + */ public Builder columnName(String name) { params.columnName = name; return this; } - /** Sets column type. */ + /** + * Sets column type. + * + * @param type Column type. + * @return {@code this}. + */ public Builder type(ColumnType type) { params.type = type; return this; } - /** Sets column precision. */ + /** + * Sets column precision. + * + * @param precision Column precision. + * @return {@code this}. + */ public Builder precision(int precision) { params.precision = precision; return this; } - /** Sets column length. */ + /** + * Sets column length. + * + * @param length Column length. + * @return {@code this}. + */ public Builder length(int length) { params.length = length; return this; } - /** Sets column scale. */ + /** + * Sets column scale. + * + * @param scale Column scale. + * @return {@code this}. + */ public Builder scale(int scale) { params.scale = scale; return this; } - /** Sets the {@code NOT NULL} constraint change flag. */ - public Builder notNull(@Nullable Boolean notNull) { + /** + * Sets the {@code NOT NULL} constraint change flag. + * + * @param notNull {@code NOT NULL} constraint change flag. + * @return {@code this}. + */ + public Builder notNull(Boolean notNull) { params.notNull = notNull; return this; } - /** Sets function that resolves a default value depending on the type of the column. */ - public Builder defaultValueResolver(@Nullable Function<ColumnType, DefaultValue> resolveDfltFunc) { + /** + * Sets function that resolves a default value depending on the type of the column. + * + * @param resolveDfltFunc Function that resolves a default value depending on the type of the column. + * @return {@code this}. + */ + public Builder defaultValueResolver(Function<ColumnType, DefaultValue> resolveDfltFunc) { params.defaultResolver = resolveDfltFunc; return this; diff --git a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/AlterTableAddColumnParams.java b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/AlterTableAddColumnParams.java index a2cf790a86..aead106042 100644 --- a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/AlterTableAddColumnParams.java +++ b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/AlterTableAddColumnParams.java @@ -19,29 +19,27 @@ package org.apache.ignite.internal.catalog.commands; import java.util.List; -/** - * ALTER TABLE ... ADD COLUMN statement. - */ +/** ALTER TABLE ... ADD COLUMN statement. */ public class AlterTableAddColumnParams extends AbstractTableCommandParams { /** Creates parameters builder. */ public static Builder builder() { return new Builder(); } + private AlterTableAddColumnParams() { + // No-op. + } + /** Columns. */ private List<ColumnParams> cols; - /** - * Gets columns that should be added to a table. - */ + /** Returns columns that should be added to a table. */ public List<ColumnParams> columns() { return cols; } - /** - * Parameters builder. - */ - public static class Builder extends AbstractTableCommandParams.AbstractBuilder<AlterTableAddColumnParams, Builder> { + /** Parameters builder. */ + public static class Builder extends AbstractTableBuilder<AlterTableAddColumnParams, Builder> { private Builder() { super(new AlterTableAddColumnParams()); } @@ -53,7 +51,8 @@ public class AlterTableAddColumnParams extends AbstractTableCommandParams { * @return {@code this}. */ public Builder columns(List<ColumnParams> cols) { - params.cols = cols; + params.cols = List.copyOf(cols); + return this; } } diff --git a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/AlterTableDropColumnParams.java b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/AlterTableDropColumnParams.java index 243e2b93e8..136544ed84 100644 --- a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/AlterTableDropColumnParams.java +++ b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/AlterTableDropColumnParams.java @@ -17,33 +17,29 @@ package org.apache.ignite.internal.catalog.commands; -import java.util.Collections; import java.util.Set; -/** - * ALTER TABLE ... DROP COLUMN statement. - */ -@SuppressWarnings("AssignmentOrReturnOfFieldWithMutableType") +/** ALTER TABLE ... DROP COLUMN statement. */ public class AlterTableDropColumnParams extends AbstractTableCommandParams { /** Creates parameters builder. */ public static Builder builder() { return new Builder(); } + private AlterTableDropColumnParams() { + // No-op. + } + /** Columns. */ private Set<String> cols; - /** - * Gets columns that should be dropped from a table. - */ + /** Returns columns that should be dropped from a table. */ public Set<String> columns() { - return Collections.unmodifiableSet(cols); + return cols; } - /** - * Parameters builder. - */ - public static class Builder extends AbstractTableCommandParams.AbstractBuilder<AlterTableDropColumnParams, Builder> { + /** Parameters builder. */ + public static class Builder extends AbstractTableBuilder<AlterTableDropColumnParams, Builder> { private Builder() { super(new AlterTableDropColumnParams()); } @@ -55,7 +51,8 @@ public class AlterTableDropColumnParams extends AbstractTableCommandParams { * @return {@code this}. */ public Builder columns(Set<String> cols) { - params.cols = cols; + params.cols = Set.copyOf(cols); + return this; } } diff --git a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/ColumnParams.java b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/ColumnParams.java index dd18d496a0..64241098e9 100644 --- a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/ColumnParams.java +++ b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/ColumnParams.java @@ -27,6 +27,10 @@ public class ColumnParams { return new Builder(); } + private ColumnParams() { + // No-op. + } + /** Column name. */ private String name; @@ -36,28 +40,24 @@ public class ColumnParams { /** Nullability flag. */ private boolean nullable; - /** Column length. */ - private Integer length; + /** Column length, {@code null} if not set. */ + private @Nullable Integer length; - /** Column precision. */ - private Integer precision; + /** Column precision, {@code null} if not set. */ + private @Nullable Integer precision; - /** Column scale. */ - private Integer scale; + /** Column scale, {@code null} if not set. */ + private @Nullable Integer scale; /** Column default value. */ private DefaultValue defaultValueDefinition = DefaultValue.constant(null); - /** - * Get column's name. - */ + /** Returns column name. */ public String name() { return name; } - /** - * Get column's type. - */ + /** Returns column type. */ public ColumnType type() { return type; } @@ -72,30 +72,22 @@ public class ColumnParams { return (T) defaultValueDefinition; } - /** - * Get nullable flag: {@code true} if this column accepts nulls. - */ + /** Returns nullable flag: {@code true} if this column accepts nulls. */ public boolean nullable() { return nullable; } - /** - * Get column's precision or {@code null} if not set. - */ + /** Returns column precision or {@code null} if not set. */ public @Nullable Integer precision() { return precision; } - /** - * Get column's scale or {@code null} if not set. - */ + /** Returns column scale or {@code null} if not set. */ public @Nullable Integer scale() { return scale; } - /** - * Get column's length or {@code null} if not set. - */ + /** Returns column length or {@code null} if not set. */ public @Nullable Integer length() { return length; } @@ -111,7 +103,7 @@ public class ColumnParams { /** * Set column simple name. * - * @param name Column simple name. + * @param name Column name. * @return {@code this}. */ public Builder name(String name) { @@ -135,6 +127,7 @@ public class ColumnParams { /** * Marks column as nullable. * + * @param nullable {@code true} if this column accepts nulls. * @return {@code this}. */ public Builder nullable(boolean nullable) { @@ -146,6 +139,7 @@ public class ColumnParams { /** * Sets column default value. * + * @param defaultValue Column default value. * @return {@code this}. */ public Builder defaultValue(DefaultValue defaultValue) { @@ -157,6 +151,7 @@ public class ColumnParams { /** * Sets column precision. * + * @param precision Column precision. * @return {@code this}. */ public Builder precision(Integer precision) { @@ -168,6 +163,7 @@ public class ColumnParams { /** * Sets column scale. * + * @param scale Column scale. * @return {@code this}. */ public Builder scale(Integer scale) { @@ -179,6 +175,7 @@ public class ColumnParams { /** * Sets column length. * + * @param length Column length. * @return {@code this}. */ public Builder length(Integer length) { @@ -187,11 +184,7 @@ public class ColumnParams { return this; } - /** - * Builds parameters. - * - * @return Parameters. - */ + /** Builds parameters. */ public ColumnParams build() { ColumnParams params0 = params; params = null; diff --git a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateTableParams.java b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateTableParams.java index 525219ae2a..14c96c19a4 100644 --- a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateTableParams.java +++ b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CreateTableParams.java @@ -20,67 +20,51 @@ package org.apache.ignite.internal.catalog.commands; import java.util.List; import org.jetbrains.annotations.Nullable; -/** - * CREATE TABLE statement. - */ +/** CREATE TABLE statement. */ public class CreateTableParams extends AbstractTableCommandParams { /** Creates parameters builder. */ public static Builder builder() { return new Builder(); } + private CreateTableParams() { + // No-op. + } + /** Primary key columns. */ - @Nullable private List<String> pkCols; /** Colocation columns. */ - @Nullable private List<String> colocationCols; /** Columns. */ private List<ColumnParams> cols; - /** Distribution zone name. */ - @Nullable - private String zone; - - private CreateTableParams() { - } + /** Distribution zone name, {@code null} means to use the default zone. */ + private @Nullable String zone; - /** - * Gets table columns. - */ + /** Returns table columns. */ public List<ColumnParams> columns() { return cols; } - /** - * Gets primary key columns. - */ + /** Returns primary key columns. */ public List<String> primaryKeyColumns() { return pkCols; } - /** - * Gets colocation column names. - */ - @Nullable - public List<String> colocationColumns() { + /** Returns colocation column names, {@code null} if not set. */ + public @Nullable List<String> colocationColumns() { return colocationCols; } - /** - * Gets zone name. - */ - @Nullable - public String zone() { + /** Returns zone name, {@code null} means to use the default distribution zone. */ + public @Nullable String zone() { return zone; } - /** - * Parameters builder. - */ - public static class Builder extends AbstractBuilder<CreateTableParams, Builder> { + /** Parameters builder. */ + public static class Builder extends AbstractTableBuilder<CreateTableParams, Builder> { private Builder() { super(new CreateTableParams()); } @@ -90,9 +74,10 @@ public class CreateTableParams extends AbstractTableCommandParams { * * @param cols Columns. * @return {@code this}. + * @throws NullPointerException If the columns is {@code null} or one of its elements. */ public Builder columns(List<ColumnParams> cols) { - params.cols = cols; + params.cols = List.copyOf(cols); return this; } @@ -101,9 +86,10 @@ public class CreateTableParams extends AbstractTableCommandParams { * Sets primary key columns. * * @return {@code this}. + * @throws NullPointerException If the primary key columns is {@code null} or one of its elements. */ public Builder primaryKeyColumns(List<String> pkCols) { - params.pkCols = pkCols; + params.pkCols = List.copyOf(pkCols); return this; } @@ -113,9 +99,10 @@ public class CreateTableParams extends AbstractTableCommandParams { * * @param colocationCols Colocation column names. * @return {@code this}. + * @throws NullPointerException If the colocation column names is {@code null} or one of its elements. */ - public Builder colocationColumns(@Nullable List<String> colocationCols) { - params.colocationCols = colocationCols; + public Builder colocationColumns(List<String> colocationCols) { + params.colocationCols = List.copyOf(colocationCols); return this; } @@ -123,7 +110,7 @@ public class CreateTableParams extends AbstractTableCommandParams { /** * Sets zone name. * - * @param zoneName Zone name. + * @param zoneName Zone name, {@code null} to use to use the default distribution zone. * @return {@code this}. */ public Builder zone(@Nullable String zoneName) { diff --git a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/DropTableParams.java b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/DropTableParams.java index d7253da503..9ae34f7d20 100644 --- a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/DropTableParams.java +++ b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/DropTableParams.java @@ -17,19 +17,20 @@ package org.apache.ignite.internal.catalog.commands; -/** - * DROP TABLE statement. - */ +/** DROP TABLE statement. */ public class DropTableParams extends AbstractTableCommandParams { + /** Creates parameters builder. */ public static Builder builder() { return new Builder(); } - /** - * Parameters builder. - */ - public static class Builder extends AbstractBuilder<DropTableParams, Builder> { - Builder() { + private DropTableParams() { + // No-op. + } + + /** Parameters builder. */ + public static class Builder extends AbstractTableBuilder<DropTableParams, Builder> { + private Builder() { super(new DropTableParams()); } } diff --git a/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/CatalogManagerSelfTest.java b/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/CatalogManagerSelfTest.java index 6f1a9408dc..1e00adf40b 100644 --- a/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/CatalogManagerSelfTest.java +++ b/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/CatalogManagerSelfTest.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.stream.Collectors.toList; import static org.apache.ignite.internal.catalog.CatalogService.DEFAULT_SCHEMA_NAME; import static org.apache.ignite.internal.catalog.CatalogService.DEFAULT_ZONE_NAME; import static org.apache.ignite.internal.catalog.commands.CatalogUtils.DEFAULT_DATA_REGION; @@ -29,12 +30,18 @@ import static org.apache.ignite.internal.catalog.commands.CatalogUtils.DEFAULT_S import static org.apache.ignite.internal.catalog.commands.CatalogUtils.DEFAULT_VARLEN_LENGTH; import static org.apache.ignite.internal.catalog.commands.CatalogUtils.IMMEDIATE_TIMER_VALUE; import static org.apache.ignite.internal.catalog.commands.CatalogUtils.INFINITE_TIMER_VALUE; +import static org.apache.ignite.internal.catalog.commands.DefaultValue.constant; import static org.apache.ignite.internal.catalog.descriptors.CatalogColumnCollation.ASC_NULLS_LAST; import static org.apache.ignite.internal.catalog.descriptors.CatalogColumnCollation.DESC_NULLS_FIRST; import static org.apache.ignite.internal.testframework.matchers.CompletableFutureExceptionMatcher.willThrow; import static org.apache.ignite.internal.testframework.matchers.CompletableFutureExceptionMatcher.willThrowFast; import static org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willBe; import static org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully; +import static org.apache.ignite.sql.ColumnType.DECIMAL; +import static org.apache.ignite.sql.ColumnType.INT32; +import static org.apache.ignite.sql.ColumnType.INT64; +import static org.apache.ignite.sql.ColumnType.NULL; +import static org.apache.ignite.sql.ColumnType.STRING; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.greaterThanOrEqualTo; @@ -66,15 +73,11 @@ import static org.mockito.Mockito.when; import java.util.ArrayList; import java.util.EnumSet; import java.util.List; -import java.util.Set; import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeUnit; import java.util.function.Supplier; -import java.util.stream.Collectors; import org.apache.ignite.internal.catalog.commands.AlterColumnParams; import org.apache.ignite.internal.catalog.commands.AlterColumnParams.Builder; -import org.apache.ignite.internal.catalog.commands.AlterTableAddColumnParams; -import org.apache.ignite.internal.catalog.commands.AlterTableDropColumnParams; import org.apache.ignite.internal.catalog.commands.AlterZoneParams; import org.apache.ignite.internal.catalog.commands.CatalogUtils; import org.apache.ignite.internal.catalog.commands.ColumnParams; @@ -85,7 +88,6 @@ import org.apache.ignite.internal.catalog.commands.CreateZoneParams; import org.apache.ignite.internal.catalog.commands.DataStorageParams; import org.apache.ignite.internal.catalog.commands.DefaultValue; import org.apache.ignite.internal.catalog.commands.DropIndexParams; -import org.apache.ignite.internal.catalog.commands.DropTableParams; import org.apache.ignite.internal.catalog.commands.DropZoneParams; import org.apache.ignite.internal.catalog.commands.RenameZoneParams; import org.apache.ignite.internal.catalog.descriptors.CatalogHashIndexDescriptor; @@ -175,20 +177,15 @@ public class CatalogManagerSelfTest extends BaseCatalogManagerTest { @Test public void testCreateTable() { - CreateTableParams params = 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).nullable(true).build() - )) - .primaryKeyColumns(List.of("key1", "key2")) - .colocationColumns(List.of("key2")) - .build(); - - assertThat(manager.createTable(params), willBe(nullValue())); + assertThat( + manager.createTable(createTableParams( + TABLE_NAME, + List.of(columnParams("key1", INT32), columnParams("key2", INT32), columnParams("val", INT32, true)), + List.of("key1", "key2"), + List.of("key2") + )), + willBe(nullValue()) + ); // Validate catalog version from the past. CatalogSchemaDescriptor schema = manager.schema(0); @@ -271,9 +268,7 @@ public class CatalogManagerSelfTest extends BaseCatalogManagerTest { long beforeDropTimestamp = clock.nowLong(); - DropTableParams dropTableParams = DropTableParams.builder().schemaName(SCHEMA_NAME).tableName(TABLE_NAME).build(); - - assertThat(manager.dropTable(dropTableParams), willBe(nullValue())); + assertThat(manager.dropTable(dropTableParams(TABLE_NAME)), willBe(nullValue())); // Validate catalog version from the past. CatalogSchemaDescriptor schema = manager.schema(2); @@ -322,9 +317,6 @@ public class CatalogManagerSelfTest extends BaseCatalogManagerTest { assertSame(pkIndex2, manager.index(createPkIndexName(TABLE_NAME_2), clock.nowLong())); assertSame(pkIndex2, manager.index(pkIndex2.id(), clock.nowLong())); - // Try to drop table once again. - assertThat(manager.dropTable(dropTableParams), willThrowFast(TableNotFoundException.class)); - // Validate schema wasn't changed. assertSame(schema, manager.activeSchema(clock.nowLong())); } @@ -333,21 +325,14 @@ public class CatalogManagerSelfTest extends BaseCatalogManagerTest { public void testAddColumn() { assertThat(manager.createTable(simpleTable(TABLE_NAME)), willBe(nullValue())); - AlterTableAddColumnParams params = AlterTableAddColumnParams.builder() - .schemaName(SCHEMA_NAME) - .tableName(TABLE_NAME) - .columns(List.of(ColumnParams.builder() - .name(NEW_COLUMN_NAME) - .type(ColumnType.STRING) - .nullable(true) - .defaultValue(DefaultValue.constant("Ignite!")) - .build() - )) - .build(); - long beforeAddedTimestamp = clock.nowLong(); - assertThat(manager.addColumn(params), willBe(nullValue())); + assertThat( + manager.addColumn(addColumnParams( + columnParamsBuilder(NEW_COLUMN_NAME, STRING, true).defaultValue(constant("Ignite!")).build() + )), + willBe(nullValue()) + ); // Validate catalog version from the past. CatalogSchemaDescriptor schema = manager.activeSchema(beforeAddedTimestamp); @@ -365,7 +350,7 @@ public class CatalogManagerSelfTest extends BaseCatalogManagerTest { CatalogTableColumnDescriptor column = schema.table(TABLE_NAME).column(NEW_COLUMN_NAME); assertEquals(NEW_COLUMN_NAME, column.name()); - assertEquals(ColumnType.STRING, column.type()); + assertEquals(STRING, column.type()); assertTrue(column.nullable()); assertEquals(DefaultValue.Type.CONSTANT, column.defaultValue().type()); @@ -380,16 +365,9 @@ public class CatalogManagerSelfTest extends BaseCatalogManagerTest { public void testDropColumn() { assertThat(manager.createTable(simpleTable(TABLE_NAME)), willBe(nullValue())); - // Validate dropping column - AlterTableDropColumnParams params = AlterTableDropColumnParams.builder() - .schemaName(SCHEMA_NAME) - .tableName(TABLE_NAME) - .columns(Set.of("VAL")) - .build(); - long beforeAddedTimestamp = clock.nowLong(); - assertThat(manager.dropColumn(params), willBe(nullValue())); + assertThat(manager.dropColumn(dropColumnParams("VAL")), willBe(nullValue())); // Validate catalog version from the past. CatalogSchemaDescriptor schema = manager.activeSchema(beforeAddedTimestamp); @@ -406,78 +384,15 @@ public class CatalogManagerSelfTest extends BaseCatalogManagerTest { assertNull(schema.table(TABLE_NAME).column("VAL")); } - @Test - public void testCreateDropColumnIfTableNotExists() { - assertNull(manager.table(TABLE_NAME, clock.nowLong())); - - // Try to add a new column. - AlterTableAddColumnParams addColumnParams = AlterTableAddColumnParams.builder() - .schemaName(SCHEMA_NAME) - .tableName(TABLE_NAME) - .columns(List.of(ColumnParams.builder().name(NEW_COLUMN_NAME).type(ColumnType.INT32).nullable(true).build())) - .build(); - - assertThat(manager.addColumn(addColumnParams), willThrow(TableNotFoundException.class)); - - // Try to drop column. - AlterTableDropColumnParams dropColumnParams = AlterTableDropColumnParams.builder() - .schemaName(SCHEMA_NAME) - .tableName(TABLE_NAME) - .columns(Set.of("VAL")) - .build(); - - assertThat(manager.dropColumn(dropColumnParams), willThrow(TableNotFoundException.class)); - } - - @Test - public void testDropIndexedColumn() { - assertThat(manager.createTable(simpleTable(TABLE_NAME)), willBe(nullValue())); - assertThat(manager.createIndex(simpleIndex()), willBe(nullValue())); - - // Try to drop indexed column - AlterTableDropColumnParams params = AlterTableDropColumnParams.builder() - .schemaName(SCHEMA_NAME) - .tableName(TABLE_NAME) - .columns(Set.of("VAL")) - .build(); - - assertThat(manager.dropColumn(params), willThrow(SqlException.class, - "Can't drop indexed column: [columnName=VAL, indexName=myIndex]")); - - // Try to drop PK column - params = AlterTableDropColumnParams.builder() - .schemaName(SCHEMA_NAME) - .tableName(TABLE_NAME) - .columns(Set.of("ID")) - .build(); - - assertThat(manager.dropColumn(params), willThrow(SqlException.class, "Can't drop primary key column: [name=ID]")); - - // Validate actual catalog - CatalogSchemaDescriptor schema = manager.activeSchema(clock.nowLong()); - assertNotNull(schema); - assertNotNull(schema.table(TABLE_NAME)); - assertSame(manager.schema(2), schema); - - assertNotNull(schema.table(TABLE_NAME).column("ID")); - assertNotNull(schema.table(TABLE_NAME).column("VAL")); - } - @Test public void testAddDropMultipleColumns() { assertThat(manager.createTable(simpleTable(TABLE_NAME)), willBe(nullValue())); // Add duplicate column. - AlterTableAddColumnParams addColumnParams = AlterTableAddColumnParams.builder() - .schemaName(SCHEMA_NAME) - .tableName(TABLE_NAME) - .columns(List.of( - ColumnParams.builder().name(NEW_COLUMN_NAME).type(ColumnType.INT32).nullable(true).build(), - ColumnParams.builder().name("VAL").type(ColumnType.INT32).nullable(true).build() - )) - .build(); - - assertThat(manager.addColumn(addColumnParams), willThrow(ColumnAlreadyExistsException.class)); + assertThat( + manager.addColumn(addColumnParams(columnParams(NEW_COLUMN_NAME, INT32, true), columnParams("VAL", INT32, true))), + willThrow(ColumnAlreadyExistsException.class) + ); // Validate no column added. CatalogSchemaDescriptor schema = manager.activeSchema(clock.nowLong()); @@ -485,16 +400,12 @@ public class CatalogManagerSelfTest extends BaseCatalogManagerTest { assertNull(schema.table(TABLE_NAME).column(NEW_COLUMN_NAME)); // Add multiple columns. - addColumnParams = AlterTableAddColumnParams.builder() - .schemaName(SCHEMA_NAME) - .tableName(TABLE_NAME) - .columns(List.of( - ColumnParams.builder().name(NEW_COLUMN_NAME).type(ColumnType.INT32).nullable(true).build(), - ColumnParams.builder().name(NEW_COLUMN_NAME_2).type(ColumnType.INT32).nullable(true).build() - )) - .build(); - - assertThat(manager.addColumn(addColumnParams), willBe(nullValue())); + assertThat( + manager.addColumn(addColumnParams( + columnParams(NEW_COLUMN_NAME, INT32, true), columnParams(NEW_COLUMN_NAME_2, INT32, true) + )), + willBe(nullValue()) + ); // Validate both columns added. schema = manager.activeSchema(clock.nowLong()); @@ -503,33 +414,13 @@ public class CatalogManagerSelfTest extends BaseCatalogManagerTest { assertNotNull(schema.table(TABLE_NAME).column(NEW_COLUMN_NAME_2)); // Drop multiple columns. - AlterTableDropColumnParams dropColumnParams = AlterTableDropColumnParams.builder() - .schemaName(SCHEMA_NAME) - .tableName(TABLE_NAME) - .columns(Set.of(NEW_COLUMN_NAME, NEW_COLUMN_NAME_2)) - .build(); - - assertThat(manager.dropColumn(dropColumnParams), willBe(nullValue())); + assertThat(manager.dropColumn(dropColumnParams(NEW_COLUMN_NAME, NEW_COLUMN_NAME_2)), willBe(nullValue())); // Validate both columns dropped. schema = manager.activeSchema(clock.nowLong()); assertNull(schema.table(TABLE_NAME).column(NEW_COLUMN_NAME)); assertNull(schema.table(TABLE_NAME).column(NEW_COLUMN_NAME_2)); - - // Check dropping of non-existing column - dropColumnParams = AlterTableDropColumnParams.builder() - .schemaName(SCHEMA_NAME) - .tableName(TABLE_NAME) - .columns(Set.of(NEW_COLUMN_NAME, "VAL")) - .build(); - - assertThat(manager.dropColumn(dropColumnParams), willThrow(ColumnNotFoundException.class)); - - // Validate no column dropped. - schema = manager.activeSchema(clock.nowLong()); - - assertNotNull(schema.table(TABLE_NAME).column("VAL")); } /** @@ -546,27 +437,27 @@ public class CatalogManagerSelfTest extends BaseCatalogManagerTest { assertNull(manager.schema(schemaVer + 1)); // NULL-> NULL : No-op. - assertThat(changeColumn(TABLE_NAME, "VAL", null, null, () -> DefaultValue.constant(null)), + assertThat(changeColumn(TABLE_NAME, "VAL", null, null, () -> constant(null)), willBe(nullValue())); assertNull(manager.schema(schemaVer + 1)); // NULL -> 1 : Ok. - assertThat(changeColumn(TABLE_NAME, "VAL", null, null, () -> DefaultValue.constant(1)), + assertThat(changeColumn(TABLE_NAME, "VAL", null, null, () -> constant(1)), willBe(nullValue())); assertNotNull(manager.schema(++schemaVer)); // 1 -> 1 : No-op. - assertThat(changeColumn(TABLE_NAME, "VAL", null, null, () -> DefaultValue.constant(1)), + assertThat(changeColumn(TABLE_NAME, "VAL", null, null, () -> constant(1)), willBe(nullValue())); assertNull(manager.schema(schemaVer + 1)); // 1 -> 2 : Ok. - assertThat(changeColumn(TABLE_NAME, "VAL", null, null, () -> DefaultValue.constant(2)), + assertThat(changeColumn(TABLE_NAME, "VAL", null, null, () -> constant(2)), willBe(nullValue())); assertNotNull(manager.schema(++schemaVer)); // 2 -> NULL : Ok. - assertThat(changeColumn(TABLE_NAME, "VAL", null, null, () -> DefaultValue.constant(null)), + assertThat(changeColumn(TABLE_NAME, "VAL", null, null, () -> constant(null)), willBe(nullValue())); assertNotNull(manager.schema(++schemaVer)); } @@ -620,8 +511,8 @@ public class CatalogManagerSelfTest extends BaseCatalogManagerTest { */ @Test public void testAlterColumnTypePrecision() { - ColumnParams pkCol = ColumnParams.builder().name("ID").type(ColumnType.INT32).build(); - ColumnParams col = ColumnParams.builder().name("COL_DECIMAL").type(ColumnType.DECIMAL).precision(10).build(); + ColumnParams pkCol = columnParams("ID", INT32); + ColumnParams col = columnParamsBuilder("COL_DECIMAL", DECIMAL).precision(10).build(); assertThat(manager.createTable(simpleTable(TABLE_NAME, List.of(pkCol, col))), willBe(nullValue())); @@ -657,9 +548,9 @@ public class CatalogManagerSelfTest extends BaseCatalogManagerTest { @ParameterizedTest @EnumSource(value = ColumnType.class, names = {"NULL", "DECIMAL"}, mode = Mode.EXCLUDE) public void testAlterColumnTypeAnyPrecisionChangeIsRejected(ColumnType type) { - ColumnParams pkCol = ColumnParams.builder().name("ID").type(ColumnType.INT32).build(); - ColumnParams col = ColumnParams.builder().name("COL").type(type).build(); - ColumnParams colWithPrecision = ColumnParams.builder().name("COL_PRECISION").type(type).precision(3).build(); + ColumnParams pkCol = columnParams("ID", INT32); + ColumnParams col = columnParams("COL", type); + ColumnParams colWithPrecision = columnParamsBuilder("COL_PRECISION", type).precision(3).build(); assertThat(manager.createTable(simpleTable(TABLE_NAME, List.of(pkCol, col, colWithPrecision))), willBe(nullValue())); @@ -693,8 +584,8 @@ public class CatalogManagerSelfTest extends BaseCatalogManagerTest { @ParameterizedTest @EnumSource(value = ColumnType.class, names = {"STRING", "BYTE_ARRAY"}, mode = Mode.INCLUDE) public void testAlterColumnTypeLength(ColumnType type) { - ColumnParams pkCol = ColumnParams.builder().name("ID").type(ColumnType.INT32).build(); - ColumnParams col = ColumnParams.builder().name("COL_" + type).length(10).type(type).build(); + ColumnParams pkCol = columnParams("ID", INT32); + ColumnParams col = columnParamsBuilder("COL_" + type, type).length(10).build(); assertThat(manager.createTable(simpleTable(TABLE_NAME, List.of(pkCol, col))), willBe(nullValue())); @@ -744,9 +635,9 @@ public class CatalogManagerSelfTest extends BaseCatalogManagerTest { @ParameterizedTest @EnumSource(value = ColumnType.class, names = {"NULL", "STRING", "BYTE_ARRAY"}, mode = Mode.EXCLUDE) public void testAlterColumnTypeAnyLengthChangeIsRejected(ColumnType type) { - ColumnParams pkCol = ColumnParams.builder().name("ID").type(ColumnType.INT32).build(); - ColumnParams col = ColumnParams.builder().name("COL").type(type).build(); - ColumnParams colWithLength = ColumnParams.builder().name("COL_PRECISION").type(type).length(10).build(); + ColumnParams pkCol = columnParams("ID", INT32); + ColumnParams col = columnParams("COL", type); + ColumnParams colWithLength = columnParamsBuilder("COL_PRECISION", type).length(10).build(); assertThat(manager.createTable(simpleTable(TABLE_NAME, List.of(pkCol, col, colWithLength))), willBe(nullValue())); @@ -775,8 +666,8 @@ public class CatalogManagerSelfTest extends BaseCatalogManagerTest { @ParameterizedTest @EnumSource(value = ColumnType.class, names = "NULL", mode = Mode.EXCLUDE) public void testAlterColumnTypeScaleIsRejected(ColumnType type) { - ColumnParams pkCol = ColumnParams.builder().name("ID").type(ColumnType.INT32).build(); - ColumnParams col = ColumnParams.builder().name("COL_" + type).type(type).scale(3).build(); + ColumnParams pkCol = columnParams("ID", INT32); + ColumnParams col = columnParamsBuilder("COL_" + type, type).scale(3).build(); assertThat(manager.createTable(simpleTable(TABLE_NAME, List.of(pkCol, col))), willBe(nullValue())); int schemaVer = 1; @@ -818,18 +709,16 @@ public class CatalogManagerSelfTest extends BaseCatalogManagerTest { @EnumSource(value = ColumnType.class, names = "NULL", mode = Mode.EXCLUDE) public void testAlterColumnType(ColumnType target) { EnumSet<ColumnType> types = EnumSet.allOf(ColumnType.class); - types.remove(ColumnType.NULL); + types.remove(NULL); List<ColumnParams> testColumns = types.stream() - .map(t -> ColumnParams.builder().name("COL_" + t).type(t).build()) - .collect(Collectors.toList()); + .map(t -> columnParams("COL_" + t, t)) + .collect(toList()); - List<ColumnParams> tableColumns = new ArrayList<>(List.of(ColumnParams.builder().name("ID").type(ColumnType.INT32).build())); + List<ColumnParams> tableColumns = new ArrayList<>(List.of(columnParams("ID", INT32))); tableColumns.addAll(testColumns); - CreateTableParams createTableParams = simpleTable(TABLE_NAME, tableColumns); - - assertThat(manager.createTable(createTableParams), willBe(nullValue())); + assertThat(manager.createTable(simpleTable(TABLE_NAME, tableColumns)), willBe(nullValue())); int schemaVer = 1; assertNotNull(manager.schema(schemaVer)); @@ -859,7 +748,7 @@ public class CatalogManagerSelfTest extends BaseCatalogManagerTest { public void testAlterColumnTypeRejectedForPrimaryKey() { assertThat(manager.createTable(simpleTable(TABLE_NAME)), willBe(nullValue())); - assertThat(changeColumn(TABLE_NAME, "ID", new TestColumnTypeParams(ColumnType.INT64), null, null), + assertThat(changeColumn(TABLE_NAME, "ID", new TestColumnTypeParams(INT64), null, null), willThrowFast(SqlException.class, "Cannot change data type for primary key column 'ID'.")); } @@ -875,9 +764,9 @@ public class CatalogManagerSelfTest extends BaseCatalogManagerTest { assertNotNull(manager.schema(schemaVer)); assertNull(manager.schema(schemaVer + 1)); - Supplier<DefaultValue> dflt = () -> DefaultValue.constant(null); + Supplier<DefaultValue> dflt = () -> constant(null); boolean notNull = false; - TestColumnTypeParams typeParams = new TestColumnTypeParams(ColumnType.INT64); + TestColumnTypeParams typeParams = new TestColumnTypeParams(INT64); // Ensures that 3 different actions applied. assertThat(changeColumn(TABLE_NAME, "VAL_NOT_NULL", typeParams, notNull, dflt), willBe(nullValue())); @@ -886,17 +775,17 @@ public class CatalogManagerSelfTest extends BaseCatalogManagerTest { assertNotNull(schema); CatalogTableColumnDescriptor desc = schema.table(TABLE_NAME).column("VAL_NOT_NULL"); - assertEquals(DefaultValue.constant(null), desc.defaultValue()); + assertEquals(constant(null), desc.defaultValue()); assertTrue(desc.nullable()); - assertEquals(ColumnType.INT64, desc.type()); + assertEquals(INT64, desc.type()); // Ensures that only one of three actions applied. - dflt = () -> DefaultValue.constant(2); + dflt = () -> constant(2); assertThat(changeColumn(TABLE_NAME, "VAL_NOT_NULL", typeParams, notNull, dflt), willBe(nullValue())); schema = manager.schema(++schemaVer); assertNotNull(schema); - assertEquals(DefaultValue.constant(2), schema.table(TABLE_NAME).column("VAL_NOT_NULL").defaultValue()); + assertEquals(constant(2), schema.table(TABLE_NAME).column("VAL_NOT_NULL").defaultValue()); // Ensures that no action will be applied. assertThat(changeColumn(TABLE_NAME, "VAL_NOT_NULL", typeParams, notNull, dflt), willBe(nullValue())); @@ -921,9 +810,7 @@ public class CatalogManagerSelfTest extends BaseCatalogManagerTest { long beforeDropTimestamp = clock.nowLong(); - DropTableParams dropTableParams = DropTableParams.builder().schemaName(SCHEMA_NAME).tableName(TABLE_NAME).build(); - - assertThat(manager.dropTable(dropTableParams), willBe(nullValue())); + assertThat(manager.dropTable(dropTableParams(TABLE_NAME)), willBe(nullValue())); // Validate catalog version from the past. CatalogSchemaDescriptor schema = manager.schema(2); @@ -1067,24 +954,16 @@ public class CatalogManagerSelfTest extends BaseCatalogManagerTest { @Test public void catalogActivationTime() throws Exception { - final long delayDuration = TimeUnit.DAYS.toMillis(365); + long delayDuration = TimeUnit.DAYS.toMillis(365); CatalogManagerImpl manager = new CatalogManagerImpl(updateLog, clockWaiter, delayDuration); manager.start(); try { - CreateTableParams params = CreateTableParams.builder() - .schemaName(SCHEMA_NAME) - .tableName(TABLE_NAME) - .columns(List.of( - ColumnParams.builder().name("key").type(ColumnType.INT32).build(), - ColumnParams.builder().name("val").type(ColumnType.INT32).nullable(true).build() - )) - .primaryKeyColumns(List.of("key")) - .build(); - - manager.createTable(params); + CompletableFuture<Void> createTableFuture = manager.createTable(simpleTable(TABLE_NAME)); + + assertFalse(createTableFuture.isDone()); verify(updateLog).append(any()); // TODO IGNITE-19400: recheck createTable future completion guarantees @@ -1121,31 +1000,16 @@ public class CatalogManagerSelfTest extends BaseCatalogManagerTest { @Test public void testTableEvents() { - CreateTableParams createTableParams = 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).nullable(true).build() - )) - .primaryKeyColumns(List.of("key1", "key2")) - .colocationColumns(List.of("key2")) - .build(); - - DropTableParams dropTableparams = DropTableParams.builder().tableName(TABLE_NAME).build(); - EventListener<CatalogEventParameters> eventListener = mock(EventListener.class); when(eventListener.notify(any(), any())).thenReturn(completedFuture(false)); manager.listen(CatalogEvent.TABLE_CREATE, eventListener); manager.listen(CatalogEvent.TABLE_DROP, eventListener); - assertThat(manager.createTable(createTableParams), willBe(nullValue())); + assertThat(manager.createTable(simpleTable(TABLE_NAME)), willBe(nullValue())); verify(eventListener).notify(any(CreateTableEventParameters.class), isNull()); - assertThat(manager.dropTable(dropTableparams), willBe(nullValue())); + assertThat(manager.dropTable(dropTableParams(TABLE_NAME)), willBe(nullValue())); verify(eventListener).notify(any(DropTableEventParameters.class), isNull()); verifyNoMoreInteractions(eventListener); @@ -1153,22 +1017,7 @@ public class CatalogManagerSelfTest extends BaseCatalogManagerTest { @Test public void testCreateIndexEvents() { - CreateTableParams createTableParams = 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).nullable(true).build() - )) - .primaryKeyColumns(List.of("key1", "key2")) - .colocationColumns(List.of("key2")) - .build(); - - DropTableParams dropTableparams = DropTableParams.builder().tableName(TABLE_NAME).build(); - - CreateHashIndexParams createIndexParams = createHashIndexParams(INDEX_NAME, List.of("key2")); + CreateHashIndexParams createIndexParams = createHashIndexParams(INDEX_NAME, List.of("ID")); DropIndexParams dropIndexParams = DropIndexParams.builder().indexName(INDEX_NAME).build(); @@ -1183,7 +1032,7 @@ public class CatalogManagerSelfTest extends BaseCatalogManagerTest { verifyNoInteractions(eventListener); // Create table with PK index. - assertThat(manager.createTable(createTableParams), willCompleteSuccessfully()); + assertThat(manager.createTable(simpleTable(TABLE_NAME)), willCompleteSuccessfully()); verify(eventListener).notify(any(CreateIndexEventParameters.class), isNull()); clearInvocations(eventListener); @@ -1201,7 +1050,7 @@ public class CatalogManagerSelfTest extends BaseCatalogManagerTest { clearInvocations(eventListener); // Drop table with pk index. - assertThat(manager.dropTable(dropTableparams), willBe(nullValue())); + assertThat(manager.dropTable(dropTableParams(TABLE_NAME)), willBe(nullValue())); // Try drop index once again. assertThat(manager.dropIndex(dropIndexParams), willThrow(IndexNotFoundException.class)); @@ -1473,53 +1322,32 @@ public class CatalogManagerSelfTest extends BaseCatalogManagerTest { @Test public void testColumnEvents() { - AlterTableAddColumnParams addColumnParams = AlterTableAddColumnParams.builder() - .schemaName(SCHEMA_NAME) - .tableName(TABLE_NAME) - .columns(List.of(ColumnParams.builder() - .name(NEW_COLUMN_NAME) - .type(ColumnType.INT32) - .defaultValue(DefaultValue.constant(42)) - .nullable(true) - .build() - )) - .build(); - - AlterTableDropColumnParams dropColumnParams = AlterTableDropColumnParams.builder() - .schemaName(SCHEMA_NAME) - .tableName(TABLE_NAME) - .columns(Set.of(NEW_COLUMN_NAME)) - .build(); - EventListener<CatalogEventParameters> eventListener = mock(EventListener.class); when(eventListener.notify(any(), any())).thenReturn(completedFuture(false)); manager.listen(CatalogEvent.TABLE_ALTER, eventListener); // Try to add column without table. - assertThat(manager.addColumn(addColumnParams), willThrow(TableNotFoundException.class)); + assertThat(manager.addColumn(addColumnParams(columnParams(NEW_COLUMN_NAME, INT32))), willThrow(TableNotFoundException.class)); verifyNoInteractions(eventListener); // Create table. assertThat(manager.createTable(simpleTable(TABLE_NAME)), willBe(nullValue())); // Add column. - assertThat(manager.addColumn(addColumnParams), willBe(nullValue())); + assertThat(manager.addColumn(addColumnParams(columnParams(NEW_COLUMN_NAME, INT32))), willBe(nullValue())); verify(eventListener).notify(any(AddColumnEventParameters.class), isNull()); // Drop column. - assertThat(manager.dropColumn(dropColumnParams), willBe(nullValue())); + assertThat(manager.dropColumn(dropColumnParams(NEW_COLUMN_NAME)), willBe(nullValue())); verify(eventListener).notify(any(DropColumnEventParameters.class), isNull()); - // Try drop column once again. - assertThat(manager.dropColumn(dropColumnParams), willThrow(ColumnNotFoundException.class)); - verifyNoMoreInteractions(eventListener); } @Test public void userFutureCompletesAfterClusterWideActivationHappens() throws Exception { - final long delayDuration = TimeUnit.DAYS.toMillis(365); + long delayDuration = TimeUnit.DAYS.toMillis(365); HybridTimestamp startTs = clock.now(); @@ -1528,19 +1356,9 @@ public class CatalogManagerSelfTest extends BaseCatalogManagerTest { manager.start(); try { - CreateTableParams params = CreateTableParams.builder() - .schemaName(SCHEMA_NAME) - .tableName(TABLE_NAME) - .columns(List.of( - ColumnParams.builder().name("key").type(ColumnType.INT32).build(), - ColumnParams.builder().name("val").type(ColumnType.INT32).nullable(true).build() - )) - .primaryKeyColumns(List.of("key")) - .build(); - - CompletableFuture<Void> future = manager.createTable(params); + CompletableFuture<Void> createTableFuture = manager.createTable(simpleTable(TABLE_NAME)); - assertThat(future.isDone(), is(false)); + assertFalse(createTableFuture.isDone()); ArgumentCaptor<HybridTimestamp> tsCaptor = ArgumentCaptor.forClass(HybridTimestamp.class); @@ -1619,10 +1437,7 @@ public class CatalogManagerSelfTest extends BaseCatalogManagerTest { assertEquals(tableId, eventParameters.tableId()); // Let's delete the table. - assertThat( - manager.dropTable(DropTableParams.builder().schemaName(SCHEMA_NAME).tableName(TABLE_NAME).build()), - willBe(nullValue()) - ); + assertThat(manager.dropTable(dropTableParams(TABLE_NAME)), willBe(nullValue())); // Let's make sure that the PK index has been deleted. eventParameters = captor.getValue(); @@ -1631,123 +1446,6 @@ public class CatalogManagerSelfTest extends BaseCatalogManagerTest { assertEquals(tableId, eventParameters.tableId()); } - @Test - void testCreateTableErrors() { - // Table must have at least one column. - assertThat( - manager.createTable( - CreateTableParams.builder() - .schemaName(SCHEMA_NAME) - .zone(ZONE_NAME) - .tableName(TABLE_NAME) - .columns(List.of()) - .primaryKeyColumns(List.of()) - .build() - ), - willThrowFast(IgniteInternalException.class, "Table must include at least one column.") - ); - - // Table must have PK columns. - assertThat( - manager.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( - manager.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( - manager.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( - manager.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( - manager.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( - manager.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]")); - } - @Test void testLatestCatalogVersion() { assertEquals(0, manager.latestCatalogVersion()); @@ -1792,14 +1490,7 @@ public class CatalogManagerSelfTest extends BaseCatalogManagerTest { public void addColumnIncrementsTableVersion() { createSomeTable(TABLE_NAME); - CompletableFuture<Void> future = manager.addColumn( - AlterTableAddColumnParams.builder() - .schemaName(SCHEMA_NAME) - .tableName(TABLE_NAME) - .columns(List.of(ColumnParams.builder().name("val2").type(ColumnType.INT32).build())) - .build() - ); - assertThat(future, willCompleteSuccessfully()); + assertThat(manager.addColumn(addColumnParams(columnParams("val2", INT32))), willCompleteSuccessfully()); CatalogTableDescriptor table = manager.table(TABLE_NAME, Long.MAX_VALUE); @@ -1810,14 +1501,7 @@ public class CatalogManagerSelfTest extends BaseCatalogManagerTest { public void dropColumnIncrementsTableVersion() { createSomeTable(TABLE_NAME); - CompletableFuture<Void> future = manager.dropColumn( - AlterTableDropColumnParams.builder() - .schemaName(SCHEMA_NAME) - .tableName(TABLE_NAME) - .columns(Set.of("val1")) - .build() - ); - assertThat(future, willCompleteSuccessfully()); + assertThat(manager.dropColumn(dropColumnParams("val1")), willCompleteSuccessfully()); CatalogTableDescriptor table = manager.table(TABLE_NAME, Long.MAX_VALUE); @@ -1833,7 +1517,7 @@ public class CatalogManagerSelfTest extends BaseCatalogManagerTest { .schemaName(SCHEMA_NAME) .tableName(TABLE_NAME) .columnName("val1") - .type(ColumnType.INT64) + .type(INT64) .build() ); assertThat(future, willCompleteSuccessfully()); @@ -1938,19 +1622,84 @@ public class CatalogManagerSelfTest extends BaseCatalogManagerTest { assertThat(manager.dropIndex(DropIndexParams.builder().indexName(INDEX_NAME).build()), willThrowFast(IndexNotFoundException.class)); } - private void createSomeTable(String tableName) { - CreateTableParams params = CreateTableParams.builder() - .schemaName(SCHEMA_NAME) - .tableName(tableName) - .zone(ZONE_NAME) - .columns(List.of( - ColumnParams.builder().name("key1").type(ColumnType.INT32).build(), - ColumnParams.builder().name("val1").type(ColumnType.INT32).build() - )) - .primaryKeyColumns(List.of("key1")) - .build(); + @Test + void testDropNotExistingTable() { + assertThat(manager.dropTable(dropTableParams(TABLE_NAME)), willThrowFast(TableNotFoundException.class)); + } + + @Test + void testCreateTableWithAlreadyExistingName() { + assertThat(manager.createTable(simpleTable(TABLE_NAME)), willCompleteSuccessfully()); - assertThat(manager.createTable(params), willCompleteSuccessfully()); + assertThat(manager.createTable(simpleTable(TABLE_NAME)), willThrowFast(TableAlreadyExistsException.class)); + } + + @Test + void testCreateTableWithSameNameAsExistingIndex() { + assertThat(manager.createTable(simpleTable(TABLE_NAME)), willCompleteSuccessfully()); + assertThat(manager.createIndex(simpleIndex()), willCompleteSuccessfully()); + + assertThat(manager.createTable(simpleTable(INDEX_NAME)), willThrowFast(IndexAlreadyExistsException.class)); + } + + @Test + void testDropColumnWithNotExistingTable() { + assertThat(manager.dropColumn(dropColumnParams("key")), willThrowFast(TableNotFoundException.class)); + } + + @Test + void testDropColumnWithMissingTableColumns() { + assertThat(manager.createTable(simpleTable(TABLE_NAME)), willCompleteSuccessfully()); + + assertThat(manager.dropColumn(dropColumnParams("fake")), willThrowFast(ColumnNotFoundException.class)); + } + + @Test + void testDropColumnWithPrimaryKeyColumns() { + assertThat(manager.createTable(simpleTable(TABLE_NAME)), willCompleteSuccessfully()); + + assertThat( + manager.dropColumn(dropColumnParams("ID")), + willThrowFast(CatalogValidationException.class, "Can't drop primary key columns: [ID]") + ); + } + + @Test + void testDropColumnWithIndexColumns() { + assertThat(manager.createTable(simpleTable(TABLE_NAME)), willCompleteSuccessfully()); + assertThat(manager.createIndex(simpleIndex()), willCompleteSuccessfully()); + + assertThat( + manager.dropColumn(dropColumnParams("VAL")), + willThrowFast( + CatalogValidationException.class, + String.format("Can't drop indexed column: [columnName=VAL, indexName=%s]", INDEX_NAME) + ) + ); + } + + @Test + void testAddColumnWithNotExistingTable() { + assertThat(manager.addColumn(addColumnParams(columnParams("key", INT32))), willThrowFast(TableNotFoundException.class)); + } + + @Test + void testAddColumnWithExistingName() { + assertThat(manager.createTable(simpleTable(TABLE_NAME)), willCompleteSuccessfully()); + + assertThat(manager.addColumn(addColumnParams(columnParams("ID", INT32))), willThrowFast(ColumnAlreadyExistsException.class)); + } + + private void createSomeTable(String tableName) { + assertThat( + manager.createTable(createTableParams( + tableName, + List.of(columnParams("key1", INT32), columnParams("val1", INT32)), + List.of("key1"), + List.of("key1") + )), + willCompleteSuccessfully() + ); } private CompletableFuture<Void> changeColumn( @@ -1990,26 +1739,19 @@ public class CatalogManagerSelfTest extends BaseCatalogManagerTest { private static CreateTableParams simpleTable(String name) { List<ColumnParams> cols = List.of( - ColumnParams.builder().name("ID").type(ColumnType.INT32).build(), - ColumnParams.builder().name("VAL").type(ColumnType.INT32).nullable(true).defaultValue(DefaultValue.constant(null)).build(), - ColumnParams.builder().name("VAL_NOT_NULL").type(ColumnType.INT32).defaultValue(DefaultValue.constant(1)).build(), - ColumnParams.builder().name("DEC").type(ColumnType.DECIMAL).nullable(true).build(), - ColumnParams.builder().name("STR").type(ColumnType.STRING).nullable(true).build(), - ColumnParams.builder().name("DEC_SCALE").type(ColumnType.DECIMAL).scale(3).build() + columnParams("ID", INT32), + columnParamsBuilder("VAL", INT32, true).defaultValue(constant(null)).build(), + columnParamsBuilder("VAL_NOT_NULL", INT32).defaultValue(constant(1)).build(), + columnParams("DEC", DECIMAL, true), + columnParams("STR", STRING, true), + columnParamsBuilder("DEC_SCALE", DECIMAL).scale(3).build() ); return simpleTable(name, cols); } - private static CreateTableParams simpleTable(String name, List<ColumnParams> cols) { - return CreateTableParams.builder() - .schemaName(SCHEMA_NAME) - .tableName(name) - .zone(ZONE_NAME) - .columns(cols) - .primaryKeyColumns(List.of(cols.get(0).name())) - .colocationColumns(List.of(cols.get(0).name())) - .build(); + private static CreateTableParams simpleTable(String tableName, List<ColumnParams> cols) { + return createTableParams(tableName, cols, List.of(cols.get(0).name()), List.of(cols.get(0).name())); } private static CreateSortedIndexParams simpleIndex() { diff --git a/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/CatalogManagerValidationTest.java b/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/CatalogManagerValidationTest.java index 2a9b680a47..314ff3e29b 100644 --- a/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/CatalogManagerValidationTest.java +++ b/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/CatalogManagerValidationTest.java @@ -27,16 +27,25 @@ import static org.apache.ignite.internal.catalog.commands.CatalogUtils.MAX_PARTI import static org.apache.ignite.internal.catalog.descriptors.CatalogColumnCollation.ASC_NULLS_FIRST; import static org.apache.ignite.internal.testframework.matchers.CompletableFutureExceptionMatcher.willThrowFast; import static org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willBe; +import static org.apache.ignite.sql.ColumnType.INT32; +import static org.apache.ignite.sql.ColumnType.INT64; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.nullValue; import java.util.Arrays; import java.util.List; +import java.util.Set; +import org.apache.ignite.internal.catalog.commands.AlterColumnParams; +import org.apache.ignite.internal.catalog.commands.AlterTableAddColumnParams; +import org.apache.ignite.internal.catalog.commands.AlterTableDropColumnParams; import org.apache.ignite.internal.catalog.commands.AlterZoneParams; +import org.apache.ignite.internal.catalog.commands.ColumnParams; import org.apache.ignite.internal.catalog.commands.CreateHashIndexParams; import org.apache.ignite.internal.catalog.commands.CreateSortedIndexParams; +import org.apache.ignite.internal.catalog.commands.CreateTableParams; import org.apache.ignite.internal.catalog.commands.CreateZoneParams; import org.apache.ignite.internal.catalog.commands.DropIndexParams; +import org.apache.ignite.internal.catalog.commands.DropTableParams; import org.apache.ignite.internal.catalog.commands.DropZoneParams; import org.apache.ignite.internal.catalog.commands.RenameZoneParams; import org.jetbrains.annotations.Nullable; @@ -653,6 +662,166 @@ public class CatalogManagerValidationTest extends BaseCatalogManagerTest { ); } + @Test + void testValidateTableNameOnTableDrop() { + assertThat( + manager.dropTable(DropTableParams.builder().build()), + willThrowFast(CatalogValidationException.class, "Missing table name") + ); + } + + @Test + void testValidateTableNameOnTableCreation() { + assertThat( + manager.createTable(CreateTableParams.builder().build()), + willThrowFast(CatalogValidationException.class, "Missing table name") + ); + } + + @Test + void testValidateColumnsOnTableCreation() { + assertThat( + manager.createTable(createTableParams(TABLE_NAME, null, null, null)), + willThrowFast(CatalogValidationException.class, "Columns not specified") + ); + + assertThat( + manager.createTable(createTableParams(TABLE_NAME, List.of(), null, null)), + willThrowFast(CatalogValidationException.class, "Columns not specified") + ); + + assertThat( + manager.createTable(createTableParams(TABLE_NAME, List.of(ColumnParams.builder().build()), null, null)), + willThrowFast(CatalogValidationException.class, "Missing column name") + ); + + assertThat( + manager.createTable(createTableParams(TABLE_NAME, List.of(ColumnParams.builder().name("key").build()), null, null)), + willThrowFast(CatalogValidationException.class, "Missing column type: key") + ); + + assertThat( + manager.createTable( + createTableParams(TABLE_NAME, List.of(columnParams("key", INT32), columnParams("key", INT64)), null, null) + ), + willThrowFast(CatalogValidationException.class, "Duplicate columns are present: [key]") + ); + } + + @Test + void testValidatePrimaryKeyColumnsOnTableCreation() { + assertThat( + manager.createTable(simpleTableParamsWithPrimaryKeys(null)), + willThrowFast(CatalogValidationException.class, "Primary key columns not specified") + ); + + assertThat( + manager.createTable(simpleTableParamsWithPrimaryKeys(List.of())), + willThrowFast(CatalogValidationException.class, "Primary key columns not specified") + ); + + assertThat( + manager.createTable(simpleTableParamsWithPrimaryKeys(List.of("key", "key"))), + willThrowFast(CatalogValidationException.class, "Duplicate primary key columns are present: [key]") + ); + + assertThat( + manager.createTable(simpleTableParamsWithPrimaryKeys(List.of("foo", "bar"))), + willThrowFast(CatalogValidationException.class, "Primary key columns missing in columns: [foo, bar]") + ); + } + + @Test + void testValidatePrimaryColocationColumnsOnTableCreation() { + assertThat( + manager.createTable(simpleTableParamsWithColocationColumns(List.of())), + willThrowFast(CatalogValidationException.class, "Colocation columns not specified") + ); + + assertThat( + manager.createTable(simpleTableParamsWithColocationColumns(List.of("key", "key"))), + willThrowFast(CatalogValidationException.class, "Duplicate colocation columns are present: [key]") + ); + + assertThat( + manager.createTable(simpleTableParamsWithColocationColumns(List.of("foo", "bar"))), + willThrowFast(CatalogValidationException.class, "Colocation columns missing in primary key columns: [foo, bar]") + ); + } + + @Test + void testValidateTableNameOnDropColumn() { + assertThat( + manager.dropColumn(AlterTableDropColumnParams.builder().build()), + willThrowFast(CatalogValidationException.class, "Missing table name") + ); + } + + @Test + void testValidateColumnsOnDropColumn() { + assertThat( + manager.dropColumn(AlterTableDropColumnParams.builder().tableName(TABLE_NAME).build()), + willThrowFast(CatalogValidationException.class, "Columns not specified") + ); + + assertThat( + manager.dropColumn(AlterTableDropColumnParams.builder().tableName(TABLE_NAME).columns(Set.of()).build()), + willThrowFast(CatalogValidationException.class, "Columns not specified") + ); + } + + @Test + void testValidateTableNameOnAddColumn() { + assertThat( + manager.addColumn(AlterTableAddColumnParams.builder().build()), + willThrowFast(CatalogValidationException.class, "Missing table name") + ); + } + + @Test + void testValidateColumnsOnAddColumn() { + assertThat( + manager.addColumn(AlterTableAddColumnParams.builder().tableName(TABLE_NAME).build()), + willThrowFast(CatalogValidationException.class, "Columns not specified") + ); + + assertThat( + manager.addColumn(AlterTableAddColumnParams.builder().tableName(TABLE_NAME).columns(List.of()).build()), + willThrowFast(CatalogValidationException.class, "Columns not specified") + ); + + assertThat( + manager.addColumn(addColumnParams(ColumnParams.builder().build())), + willThrowFast(CatalogValidationException.class, "Missing column name") + ); + + assertThat( + manager.addColumn(addColumnParams(ColumnParams.builder().name("key").build())), + willThrowFast(CatalogValidationException.class, "Missing column type: key") + ); + + assertThat( + manager.addColumn(addColumnParams(columnParams("key", INT32), columnParams("key", INT64))), + willThrowFast(CatalogValidationException.class, "Duplicate columns are present: [key]") + ); + } + + @Test + void testValidateTableNameOnAlterColumn() { + assertThat( + manager.alterColumn(AlterColumnParams.builder().build()), + willThrowFast(CatalogValidationException.class, "Missing table name") + ); + } + + @Test + void testValidateColumnNameOnAlterColumn() { + assertThat( + manager.alterColumn(AlterColumnParams.builder().tableName(TABLE_NAME).build()), + willThrowFast(CatalogValidationException.class, "Missing column name") + ); + } + private static CreateZoneParams.Builder createZoneBuilder(String zoneName) { return CreateZoneParams.builder().zoneName(zoneName); } @@ -685,4 +854,17 @@ public class CatalogManagerValidationTest extends BaseCatalogManagerTest { .dataNodesAutoAdjustScaleDown(scaleDown) .build(); } + + private CreateTableParams simpleTableParamsWithPrimaryKeys(@Nullable List<String> primaryKeys) { + return createTableParams(TABLE_NAME, List.of(columnParams("key", INT32), columnParams("val", INT64)), primaryKeys, null); + } + + private CreateTableParams simpleTableParamsWithColocationColumns(@Nullable List<String> colocationColumns) { + return createTableParams( + TABLE_NAME, + List.of(columnParams("key", INT32), columnParams("val", INT64)), + List.of("key"), + colocationColumns + ); + } } diff --git a/modules/catalog/src/testFixtures/java/org/apache/ignite/internal/catalog/BaseCatalogManagerTest.java b/modules/catalog/src/testFixtures/java/org/apache/ignite/internal/catalog/BaseCatalogManagerTest.java index c138c767e9..e8b46f2bcc 100644 --- a/modules/catalog/src/testFixtures/java/org/apache/ignite/internal/catalog/BaseCatalogManagerTest.java +++ b/modules/catalog/src/testFixtures/java/org/apache/ignite/internal/catalog/BaseCatalogManagerTest.java @@ -18,15 +18,23 @@ package org.apache.ignite.internal.catalog; import static org.apache.ignite.internal.catalog.CatalogService.DEFAULT_SCHEMA_NAME; +import static org.apache.ignite.internal.catalog.CatalogService.DEFAULT_ZONE_NAME; import static org.apache.ignite.internal.testframework.matchers.CompletableFutureMatcher.willCompleteSuccessfully; import static org.hamcrest.MatcherAssert.assertThat; import static org.mockito.Mockito.spy; import java.util.List; import java.util.Objects; +import java.util.Set; import java.util.stream.Stream; +import org.apache.ignite.internal.catalog.commands.AlterTableAddColumnParams; +import org.apache.ignite.internal.catalog.commands.AlterTableDropColumnParams; +import org.apache.ignite.internal.catalog.commands.ColumnParams; import org.apache.ignite.internal.catalog.commands.CreateHashIndexParams; import org.apache.ignite.internal.catalog.commands.CreateSortedIndexParams; +import org.apache.ignite.internal.catalog.commands.CreateTableParams; +import org.apache.ignite.internal.catalog.commands.CreateTableParams.Builder; +import org.apache.ignite.internal.catalog.commands.DropTableParams; import org.apache.ignite.internal.catalog.descriptors.CatalogColumnCollation; import org.apache.ignite.internal.catalog.storage.UpdateLog; import org.apache.ignite.internal.catalog.storage.UpdateLogImpl; @@ -39,6 +47,7 @@ import org.apache.ignite.internal.testframework.BaseIgniteAbstractTest; import org.apache.ignite.internal.util.IgniteUtils; import org.apache.ignite.internal.vault.VaultManager; import org.apache.ignite.internal.vault.inmemory.InMemoryVaultService; +import org.apache.ignite.sql.ColumnType; import org.jetbrains.annotations.Nullable; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -149,4 +158,58 @@ public abstract class BaseCatalogManagerTest extends BaseIgniteAbstractTest { ) { return createSortedIndexParams(indexName, false, indexColumns, columnsCollations); } + + protected static CreateTableParams createTableParams( + String tableName, + @Nullable List<ColumnParams> columns, + @Nullable List<String> primaryKeys, + @Nullable List<String> colocationColumns + ) { + Builder builder = CreateTableParams.builder() + .schemaName(DEFAULT_SCHEMA_NAME) + .zone(DEFAULT_ZONE_NAME) + .tableName(tableName); + + if (columns != null) { + builder.columns(columns); + } + + if (primaryKeys != null) { + builder.primaryKeyColumns(primaryKeys); + } + + if (colocationColumns != null) { + builder.colocationColumns(colocationColumns); + } + + return builder.build(); + } + + protected static ColumnParams columnParams(String name, ColumnType type) { + return columnParams(name, type, false); + } + + protected static ColumnParams columnParams(String name, ColumnType type, boolean nullable) { + return columnParamsBuilder(name, type, nullable).build(); + } + + protected static ColumnParams.Builder columnParamsBuilder(String name, ColumnType type) { + return columnParamsBuilder(name, type, false); + } + + protected static ColumnParams.Builder columnParamsBuilder(String name, ColumnType type, boolean nullable) { + return ColumnParams.builder().name(name).nullable(nullable).type(type); + } + + protected static DropTableParams dropTableParams(String tableName) { + return DropTableParams.builder().schemaName(DEFAULT_SCHEMA_NAME).tableName(tableName).build(); + } + + protected static AlterTableDropColumnParams dropColumnParams(String... columns) { + return AlterTableDropColumnParams.builder().schemaName(DEFAULT_SCHEMA_NAME).tableName(TABLE_NAME).columns(Set.of(columns)).build(); + } + + protected static AlterTableAddColumnParams addColumnParams(ColumnParams... columns) { + return AlterTableAddColumnParams.builder().schemaName(DEFAULT_SCHEMA_NAME).tableName(TABLE_NAME).columns(List.of(columns)).build(); + } } diff --git a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ddl/DdlToCatalogCommandConverter.java b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ddl/DdlToCatalogCommandConverter.java index 45ccfdd95c..e9e4da9f5f 100644 --- a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ddl/DdlToCatalogCommandConverter.java +++ b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ddl/DdlToCatalogCommandConverter.java @@ -34,6 +34,7 @@ import org.apache.ignite.internal.catalog.commands.ColumnParams; import org.apache.ignite.internal.catalog.commands.CreateHashIndexParams; import org.apache.ignite.internal.catalog.commands.CreateSortedIndexParams; import org.apache.ignite.internal.catalog.commands.CreateTableParams; +import org.apache.ignite.internal.catalog.commands.CreateTableParams.Builder; import org.apache.ignite.internal.catalog.commands.CreateZoneParams; import org.apache.ignite.internal.catalog.commands.DataStorageParams; import org.apache.ignite.internal.catalog.commands.DefaultValue; @@ -65,17 +66,20 @@ class DdlToCatalogCommandConverter { static CreateTableParams convert(CreateTableCommand cmd) { List<ColumnParams> columns = cmd.columns().stream().map(DdlToCatalogCommandConverter::convert).collect(Collectors.toList()); - return CreateTableParams.builder() + Builder builder = CreateTableParams.builder() .schemaName(cmd.schemaName()) .tableName(cmd.tableName()) .columns(columns) - .colocationColumns(cmd.colocationColumns()) .primaryKeyColumns(cmd.primaryKeyColumns()) - .zone(cmd.zone()) + .zone(cmd.zone()); - .build(); + if (cmd.colocationColumns() != null) { + builder.colocationColumns(cmd.colocationColumns()); + } + + return builder.build(); } static DropTableParams convert(DropTableCommand cmd) {