This is an automated email from the ASF dual-hosted git repository. amashenkov pushed a commit to branch ignite-19592 in repository https://gitbox.apache.org/repos/asf/ignite-3.git
commit 45b86ef28402a01a35456e8f84fd337a3216702e Author: amashenkov <[email protected]> AuthorDate: Thu May 18 16:48:03 2023 +0300 Add create/drop columns commands. --- .../internal/catalog/CatalogServiceImpl.java | 139 +++++++++++++- .../commands/AlterTableAddColumnParams.java | 4 + .../commands/AlterTableDropColumnParams.java | 8 + .../internal/catalog/commands/CatalogUtils.java | 8 +- .../internal/catalog/commands/ColumnParams.java | 88 ++++++++- .../catalog/descriptors/HashIndexDescriptor.java | 19 +- .../catalog/descriptors/IndexDescriptor.java | 20 +- .../catalog/descriptors/SortedIndexDescriptor.java | 22 ++- .../catalog/descriptors/TableDescriptor.java | 44 +++-- .../DropColumnsEntry.java} | 40 ++-- .../NewColumnsEntry.java} | 41 ++--- .../internal/catalog/CatalogServiceSelfTest.java | 205 ++++++++++++++++++++- .../runner/app/ItSchemaChangeTableViewTest.java | 3 + .../engine/exec/ddl/DdlCommandHandlerWrapper.java | 20 ++ .../exec/ddl/DdlToCatalogCommandConverter.java | 36 +++- .../distributed/schema/FullTableSchemaTest.java | 2 +- 16 files changed, 605 insertions(+), 94 deletions(-) diff --git a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogServiceImpl.java b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogServiceImpl.java index 027a74484c..b00da5d9fa 100644 --- a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogServiceImpl.java +++ b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/CatalogServiceImpl.java @@ -27,21 +27,27 @@ 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.stream.Collectors; import org.apache.ignite.internal.catalog.commands.AlterTableAddColumnParams; import org.apache.ignite.internal.catalog.commands.AlterTableDropColumnParams; import org.apache.ignite.internal.catalog.commands.CatalogUtils; +import org.apache.ignite.internal.catalog.commands.ColumnParams; import org.apache.ignite.internal.catalog.commands.CreateTableParams; import org.apache.ignite.internal.catalog.commands.DropTableParams; import org.apache.ignite.internal.catalog.descriptors.IndexDescriptor; import org.apache.ignite.internal.catalog.descriptors.SchemaDescriptor; +import org.apache.ignite.internal.catalog.descriptors.TableColumnDescriptor; import org.apache.ignite.internal.catalog.descriptors.TableDescriptor; import org.apache.ignite.internal.catalog.events.CatalogEvent; import org.apache.ignite.internal.catalog.events.CatalogEventParameters; import org.apache.ignite.internal.catalog.events.CreateTableEventParameters; import org.apache.ignite.internal.catalog.events.DropTableEventParameters; +import org.apache.ignite.internal.catalog.storage.DropColumnsEntry; import org.apache.ignite.internal.catalog.storage.DropTableEntry; +import org.apache.ignite.internal.catalog.storage.NewColumnsEntry; import org.apache.ignite.internal.catalog.storage.NewTableEntry; import org.apache.ignite.internal.catalog.storage.ObjectIdGenUpdateEntry; import org.apache.ignite.internal.catalog.storage.UpdateEntry; @@ -52,7 +58,10 @@ import org.apache.ignite.internal.logger.IgniteLogger; import org.apache.ignite.internal.logger.Loggers; import org.apache.ignite.internal.manager.Producer; import org.apache.ignite.internal.util.ArrayUtils; +import org.apache.ignite.internal.util.CollectionUtils; 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.Common; import org.apache.ignite.lang.IgniteInternalException; import org.apache.ignite.lang.TableAlreadyExistsException; @@ -61,7 +70,6 @@ import org.jetbrains.annotations.Nullable; /** * Catalog service implementation. - * TODO: IGNITE-19081 Introduce catalog events and make CatalogServiceImpl extends Producer. */ public class CatalogServiceImpl extends Producer<CatalogEvent, CatalogEventParameters> implements CatalogManager { private static final int MAX_RETRY_COUNT = 10; @@ -205,13 +213,86 @@ public class CatalogServiceImpl extends Producer<CatalogEvent, CatalogEventParam /** {@inheritDoc} */ @Override public CompletableFuture<Void> addColumn(AlterTableAddColumnParams params) { - return failedFuture(new UnsupportedOperationException("Not implemented yet.")); + if (params.columns().isEmpty()) { + return completedFuture(null); + } else if (params.columns().size() > 1 && params.ifColumnNotExists()) { + return failedFuture(new UnsupportedOperationException("Clause 'IF NOT EXISTS' is not supported when adding multiple columns.")); + } + + return saveUpdate(catalog -> { + String schemaName = Objects.requireNonNullElse(params.schemaName(), CatalogService.PUBLIC); + + SchemaDescriptor schema = Objects.requireNonNull(catalog.schema(schemaName), "No schema found: " + schemaName); + + TableDescriptor table = schema.table(params.tableName()); + + if (table == null) { + throw new TableNotFoundException(schemaName, params.tableName()); + } + + List<TableColumnDescriptor> columnDescriptors = new ArrayList<>(); + + for (ColumnParams col : params.columns()) { + if (table.column(col.name()) != null) { + throw new ColumnAlreadyExistsException(col.name()); + } + + columnDescriptors.add(CatalogUtils.fromParams(col)); + } + + return List.of( + new NewColumnsEntry(table.id(), columnDescriptors) + ); + }); } /** {@inheritDoc} */ @Override public CompletableFuture<Void> dropColumn(AlterTableDropColumnParams params) { - return failedFuture(new UnsupportedOperationException("Not implemented yet.")); + if (params.columns().isEmpty()) { + return completedFuture(null); + } else if (params.columns().size() > 1 && params.ifColumnExists()) { + return failedFuture(new UnsupportedOperationException("Clause 'IF NOT EXISTS' is not supported when adding multiple columns.")); + } + + return saveUpdate(catalog -> { + String schemaName = Objects.requireNonNullElse(params.schemaName(), CatalogService.PUBLIC); + + SchemaDescriptor schema = Objects.requireNonNull(catalog.schema(schemaName), "No schema found: " + schemaName); + + TableDescriptor table = schema.table(params.tableName()); + + if (table == null) { + throw new TableNotFoundException(schemaName, params.tableName()); + } + + Arrays.stream(schema.indexes()) + .filter(index -> index.tableId() == table.id()) + .flatMap(index -> index.columns().stream()) + .filter(col -> params.columns().contains(col)) + .findAny() + .ifPresent(columnName -> { + throw new IllegalArgumentException("Can't drop indexed column: column=" + columnName); + }); + + for (String columnName : params.columns()) { + if (table.column(columnName) == null) { + throw new ColumnNotFoundException(columnName); + } + if (table.isPrimaryKeyColumn(columnName)) { + throw new IllegalArgumentException("Can't drop primary key column: column=" + columnName); + } + } + + Set<String> columnsToDrop = table.columns().stream() + .map(TableColumnDescriptor::name) + .filter(col -> params.columns().contains(col)) + .collect(Collectors.toSet()); + + return List.of( + new DropColumnsEntry(table.id(), columnsToDrop) + ); + }); } private void registerCatalog(Catalog newCatalog) { @@ -307,7 +388,59 @@ public class CatalogServiceImpl extends Producer<CatalogEvent, CatalogEventParam CatalogEvent.TABLE_DROP, new DropTableEventParameters(version, tableId) )); + } else if (entry instanceof NewColumnsEntry) { + int tableId = ((NewColumnsEntry) entry).tableId(); + List<TableColumnDescriptor> columnDescriptors = ((NewColumnsEntry) entry).descriptors(); + + catalog = new Catalog( + version, + System.currentTimeMillis(), + catalog.objectIdGenState(), + new SchemaDescriptor( + schema.id(), + schema.name(), + version, + Arrays.stream(schema.tables()) + .map(table -> table.id() != tableId + ? table + : new TableDescriptor( + table.id(), + table.name(), + CollectionUtils.concat(table.columns(), columnDescriptors), + table.primaryKeyColumns(), + table.colocationColumns()) + ) + .toArray(TableDescriptor[]::new), + schema.indexes() + ) + ); + } else if (entry instanceof DropColumnsEntry) { + int tableId = ((DropColumnsEntry) entry).tableId(); + Set<String> columns = ((DropColumnsEntry) entry).columns(); + catalog = new Catalog( + version, + System.currentTimeMillis(), + catalog.objectIdGenState(), + new SchemaDescriptor( + schema.id(), + schema.name(), + version, + Arrays.stream(schema.tables()) + .map(table -> table.id() != tableId + ? table + : new TableDescriptor( + table.id(), + table.name(), + table.columns().stream().filter(col -> !columns.contains(col.name())) + .collect(Collectors.toList()), + table.primaryKeyColumns(), + table.colocationColumns()) + ) + .toArray(TableDescriptor[]::new), + schema.indexes() + ) + ); } else if (entry instanceof ObjectIdGenUpdateEntry) { catalog = new Catalog( version, 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 61a7c9a8e3..8bc80afe68 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 @@ -23,6 +23,7 @@ import java.util.List; * ALTER TABLE ... ADD COLUMN statement. */ public class AlterTableAddColumnParams extends AbstractTableCommandParams { + /** Creates parameters builder. */ public static Builder builder() { return new Builder(); } @@ -33,6 +34,9 @@ public class AlterTableAddColumnParams extends AbstractTableCommandParams { /** Columns. */ private List<ColumnParams> cols; + /** + * Gets columns that should be added to a table. + */ public List<ColumnParams> columns() { return cols; } 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 379c54d215..20802a9eef 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 @@ -25,12 +25,20 @@ import java.util.Set; */ @SuppressWarnings("AssignmentOrReturnOfFieldWithMutableType") public class AlterTableDropColumnParams extends AbstractTableCommandParams { + /** Creates parameters builder. */ + public static Builder builder() { + return new Builder(); + } + /** Quietly ignore this command if column is not exist. */ private boolean ifColumnExists; /** Columns. */ private Set<String> cols; + /** + * Gets columns that should be dropped from a table. + */ public Set<String> columns() { return Collections.unmodifiableSet(cols); } diff --git a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CatalogUtils.java b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CatalogUtils.java index 1ead944291..d462be6137 100644 --- a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CatalogUtils.java +++ b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/commands/CatalogUtils.java @@ -41,7 +41,13 @@ public class CatalogUtils { ); } - private static TableColumnDescriptor fromParams(ColumnParams params) { + /** + * Converts AlterTableAdd command columns parameters to column descriptor. + * + * @param params Parameters. + * @return Column descriptor. + */ + public static TableColumnDescriptor fromParams(ColumnParams params) { return new TableColumnDescriptor(params.name(), params.type(), params.nullable(), params.defaultValueDefinition()); } } 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 d8cf657e35..2b108c2220 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 @@ -17,21 +17,29 @@ package org.apache.ignite.internal.catalog.commands; -import java.io.Serializable; import java.util.Objects; import org.apache.ignite.sql.ColumnType; /** Defines a particular column within table. */ -public class ColumnParams implements Serializable { - private static final long serialVersionUID = 5602599481844743521L; +public class ColumnParams { + /** Creates parameters builder. */ + public static Builder builder() { + return new Builder(); + } + + /** Column name. */ + private String name; - private final String name; + /** Column type. */ + private ColumnType type; - private final ColumnType type; + /** Nullability flag. */ + private boolean nullable; - private final boolean nullable; + /** Column default value. */ + private DefaultValue defaultValueDefinition; - private final DefaultValue defaultValueDefinition; + private ColumnParams() {} /** Creates a column definition. */ public ColumnParams(String name, ColumnType type, DefaultValue defaultValueDefinition, boolean nullable) { @@ -86,4 +94,70 @@ public class ColumnParams implements Serializable { public Integer scale() { return null; } + + /** Parameters builder. */ + public static class Builder { + private ColumnParams params; + + private Builder() { + params = new ColumnParams(); + } + + /** + * Set column simple name. + * + * @param name Column simple name. + * @return {@code this}. + */ + public Builder name(String name) { + params.name = name; + + return this; + } + + /** + * Set column type. + * + * @param type Column type. + * @return {@code this}. + */ + public Builder type(ColumnType type) { + params.type = type; + + return this; + } + + /** + * Marks column as nullable. + * + * @return {@code this}. + */ + public Builder nullable(boolean nullable) { + params.nullable = nullable; + + return this; + } + + /** + * Sets column default value. + * + * @return {@code this}. + */ + public Builder defaultValue(DefaultValue defaultValue) { + params.defaultValueDefinition = defaultValue; + + return this; + } + + /** + * Builds parameters. + * + * @return Parameters. + */ + public ColumnParams build() { + ColumnParams params0 = params; + params = null; + return params0; + } + } } diff --git a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/descriptors/HashIndexDescriptor.java b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/descriptors/HashIndexDescriptor.java index 8e2d96c3da..ac94378223 100644 --- a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/descriptors/HashIndexDescriptor.java +++ b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/descriptors/HashIndexDescriptor.java @@ -17,9 +17,7 @@ package org.apache.ignite.internal.catalog.descriptors; -import java.util.HashSet; import java.util.List; -import java.util.Objects; import org.apache.ignite.internal.tostring.S; /** @@ -28,8 +26,6 @@ import org.apache.ignite.internal.tostring.S; public class HashIndexDescriptor extends IndexDescriptor { private static final long serialVersionUID = -6784028115063219759L; - private final List<String> columns; - /** * Constructs a hash index descriptor. * @@ -37,22 +33,13 @@ public class HashIndexDescriptor extends IndexDescriptor { * @param name Name of the index. * @param tableId Id of the table index belongs to. * @param columns A list of indexed columns. Must not contains duplicates. + * @param unique Unique flag. * @throws IllegalArgumentException If columns list contains duplicates. */ - public HashIndexDescriptor(int id, String name, int tableId, List<String> columns) { - super(id, name, tableId, true); - - this.columns = List.copyOf(Objects.requireNonNull(columns, "columns")); - - if (new HashSet<>(columns).size() != columns.size()) { - throw new IllegalArgumentException("Indexed columns should be unique"); - } + public HashIndexDescriptor(int id, String name, int tableId, List<String> columns, boolean unique) { + super(id, name, tableId, columns, unique); } - /** Returns indexed columns. */ - public List<String> columns() { - return columns; - } /** {@inheritDoc} */ @Override diff --git a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/descriptors/IndexDescriptor.java b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/descriptors/IndexDescriptor.java index 99feaf15e3..c365a6446f 100644 --- a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/descriptors/IndexDescriptor.java +++ b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/descriptors/IndexDescriptor.java @@ -17,6 +17,9 @@ package org.apache.ignite.internal.catalog.descriptors; +import java.util.List; +import java.util.Objects; +import java.util.Set; import org.apache.ignite.internal.tostring.S; /** @@ -29,21 +32,34 @@ public abstract class IndexDescriptor extends ObjectDescriptor { private final int tableId; /** Unique constraint flag. */ - private boolean unique; + private final boolean unique; + + /** Index columns. */ + private final List<String> columns; /** Write only flag. {@code True} when index is building. */ private boolean writeOnly; - IndexDescriptor(int id, String name, int tableId, boolean unique) { + IndexDescriptor(int id, String name, int tableId, List<String> columns, boolean unique) { super(id, Type.INDEX, name); this.tableId = tableId; this.unique = unique; + this.columns = Objects.requireNonNull(columns, "columns"); + + if (Set.copyOf(this.columns).size() != this.columns.size()) { + throw new IllegalArgumentException("Indexed columns should be unique"); + } } public int tableId() { return tableId; } + /** Returns indexed columns. */ + public List<String> columns() { + return columns; + } + public boolean unique() { return unique; } diff --git a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/descriptors/SortedIndexDescriptor.java b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/descriptors/SortedIndexDescriptor.java index cfc59ee4c4..858cbea637 100644 --- a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/descriptors/SortedIndexDescriptor.java +++ b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/descriptors/SortedIndexDescriptor.java @@ -20,6 +20,8 @@ package org.apache.ignite.internal.catalog.descriptors; import java.util.List; import java.util.Objects; +import java.util.stream.Collectors; +import java.util.stream.IntStream; import org.apache.ignite.internal.tostring.S; /** @@ -28,7 +30,7 @@ import org.apache.ignite.internal.tostring.S; public class SortedIndexDescriptor extends IndexDescriptor { private static final long serialVersionUID = 2085714310150728611L; - private final List<IndexColumnDescriptor> columns; + private final List<IndexColumnDescriptor> columnDescriptors; /** * Constructs a sorted description. @@ -36,17 +38,23 @@ public class SortedIndexDescriptor extends IndexDescriptor { * @param id Id of the index. * @param name Name of the index. * @param tableId Id of the table index belongs to. - * @param columns A list of columns descriptors. + * @param unique Unique flag. + * @param columns A list of columns names. + * @param collations A list of columns collations. * @throws IllegalArgumentException If columns list contains duplicates or columns size doesn't match the collations size. */ - public SortedIndexDescriptor(int id, String name, int tableId, List<IndexColumnDescriptor> columns) { - super(id, name, tableId, false); + public SortedIndexDescriptor(int id, String name, int tableId, boolean unique, List<String> columns, List<ColumnCollation> collations) { + super(id, name, tableId, columns, unique); - this.columns = Objects.requireNonNull(columns, "columns"); + assert collations.size() == columns.size(); + + this.columnDescriptors = IntStream.range(0, Objects.requireNonNull(collations, "collations").size()) + .mapToObj(i -> new IndexColumnDescriptor(columns.get(i), collations.get(i))) + .collect(Collectors.toList()); } - public List<IndexColumnDescriptor> columns() { - return columns; + public List<IndexColumnDescriptor> columnsDecsriptors() { + return columnDescriptors; } /** {@inheritDoc} */ diff --git a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/descriptors/TableDescriptor.java b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/descriptors/TableDescriptor.java index a9bfd56168..803397c07c 100644 --- a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/descriptors/TableDescriptor.java +++ b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/descriptors/TableDescriptor.java @@ -19,7 +19,6 @@ package org.apache.ignite.internal.catalog.descriptors; import java.io.IOException; import java.io.ObjectInputStream; -import java.util.Arrays; import java.util.List; import java.util.Map; import java.util.Objects; @@ -39,9 +38,9 @@ public class TableDescriptor extends ObjectDescriptor { private final int zoneId = 0; private final int engineId = 0; - private final TableColumnDescriptor[] columns; - private final String[] primaryKeyColumns; - private final String[] colocationColumns; + private final List<TableColumnDescriptor> columns; + private final List<String> primaryKeyColumns; + private final List<String> colocationColumns; @IgniteToStringExclude private transient Map<String, TableColumnDescriptor> columnsMap; @@ -64,20 +63,19 @@ public class TableDescriptor extends ObjectDescriptor { ) { super(id, Type.TABLE, name); - this.columns = Objects.requireNonNull(columns, "No columns defined.").toArray(TableColumnDescriptor[]::new); - primaryKeyColumns = Objects.requireNonNull(pkCols, "No primary key columns.").toArray(String[]::new); - colocationColumns = colocationCols == null ? primaryKeyColumns : colocationCols.toArray(String[]::new); + this.columns = Objects.requireNonNull(columns, "No columns defined."); + primaryKeyColumns = Objects.requireNonNull(pkCols, "No primary key columns."); + colocationColumns = colocationCols == null ? pkCols : colocationCols; this.columnsMap = columns.stream().collect(Collectors.toMap(TableColumnDescriptor::name, Function.identity())); // TODO: IGNITE-19082 Throw proper exceptions. assert !columnsMap.isEmpty() : "No columns."; - assert primaryKeyColumns.length > 0 : "No primary key columns."; - assert colocationColumns.length > 0 : "No colocation columns."; + assert !primaryKeyColumns.isEmpty() : "No primary key columns."; + assert !colocationColumns.isEmpty() : "No colocation columns."; - assert Arrays.stream(primaryKeyColumns).noneMatch(c -> Objects.requireNonNull(columnsMap.get(c), c).nullable()); - //noinspection ArrayEquality - assert primaryKeyColumns == colocationColumns || Set.of(primaryKeyColumns).containsAll(List.of(colocationColumns)); + assert primaryKeyColumns.stream().noneMatch(c -> Objects.requireNonNull(columnsMap.get(c), c).nullable()); + assert Set.copyOf(primaryKeyColumns).containsAll(colocationColumns); } public int zoneId() { @@ -88,9 +86,29 @@ public class TableDescriptor extends ObjectDescriptor { return engineId; } + public List<String> primaryKeyColumns() { + return primaryKeyColumns; + } + + public List<String> colocationColumns() { + return colocationColumns; + } + + public List<TableColumnDescriptor> columns() { + return columns; + } + + public TableColumnDescriptor column(String name) { + return columnsMap.get(name); + } + + public boolean isPrimaryKeyColumn(String name) { + return primaryKeyColumns.contains(name); + } + private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException { in.defaultReadObject(); - this.columnsMap = Arrays.stream(columns).collect(Collectors.toMap(TableColumnDescriptor::name, Function.identity())); + this.columnsMap = columns.stream().collect(Collectors.toMap(TableColumnDescriptor::name, Function.identity())); } /** {@inheritDoc} */ diff --git a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/descriptors/IndexDescriptor.java b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/storage/DropColumnsEntry.java similarity index 62% copy from modules/catalog/src/main/java/org/apache/ignite/internal/catalog/descriptors/IndexDescriptor.java copy to modules/catalog/src/main/java/org/apache/ignite/internal/catalog/storage/DropColumnsEntry.java index 99feaf15e3..ab77a41205 100644 --- a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/descriptors/IndexDescriptor.java +++ b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/storage/DropColumnsEntry.java @@ -15,41 +15,39 @@ * limitations under the License. */ -package org.apache.ignite.internal.catalog.descriptors; +package org.apache.ignite.internal.catalog.storage; +import java.util.Set; import org.apache.ignite.internal.tostring.S; /** - * Index descriptor base class. + * Describes addition of a new columns. */ -public abstract class IndexDescriptor extends ObjectDescriptor { - private static final long serialVersionUID = -8045949593661301287L; +public class DropColumnsEntry implements UpdateEntry { + private static final long serialVersionUID = 2970125889493580121L; - /** Table id. */ private final int tableId; - - /** Unique constraint flag. */ - private boolean unique; - - /** Write only flag. {@code True} when index is building. */ - private boolean writeOnly; - - IndexDescriptor(int id, String name, int tableId, boolean unique) { - super(id, Type.INDEX, name); + private final Set<String> columns; + + /** + * Constructs the object. + * + * @param tableId Table id. + * @param columns A names of columns to drop. + */ + public DropColumnsEntry(int tableId, Set<String> columns) { this.tableId = tableId; - this.unique = unique; + this.columns = columns; } + /** Returns table id. */ public int tableId() { return tableId; } - public boolean unique() { - return unique; - } - - public boolean writeOnly() { - return writeOnly; + /** Returns name of columns to drop. */ + public Set<String> columns() { + return columns; } /** {@inheritDoc} */ diff --git a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/descriptors/IndexDescriptor.java b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/storage/NewColumnsEntry.java similarity index 57% copy from modules/catalog/src/main/java/org/apache/ignite/internal/catalog/descriptors/IndexDescriptor.java copy to modules/catalog/src/main/java/org/apache/ignite/internal/catalog/storage/NewColumnsEntry.java index 99feaf15e3..04c352c4e9 100644 --- a/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/descriptors/IndexDescriptor.java +++ b/modules/catalog/src/main/java/org/apache/ignite/internal/catalog/storage/NewColumnsEntry.java @@ -15,41 +15,40 @@ * limitations under the License. */ -package org.apache.ignite.internal.catalog.descriptors; +package org.apache.ignite.internal.catalog.storage; +import java.util.List; +import org.apache.ignite.internal.catalog.descriptors.TableColumnDescriptor; import org.apache.ignite.internal.tostring.S; /** - * Index descriptor base class. + * Describes addition of a new columns. */ -public abstract class IndexDescriptor extends ObjectDescriptor { - private static final long serialVersionUID = -8045949593661301287L; +public class NewColumnsEntry implements UpdateEntry { + private static final long serialVersionUID = 2970125889493580121L; - /** Table id. */ private final int tableId; - - /** Unique constraint flag. */ - private boolean unique; - - /** Write only flag. {@code True} when index is building. */ - private boolean writeOnly; - - IndexDescriptor(int id, String name, int tableId, boolean unique) { - super(id, Type.INDEX, name); + private final List<TableColumnDescriptor> descriptors; + + /** + * Constructs the object. + * + * @param tableId Table id. + * @param descriptors A descriptors of columns to add. + */ + public NewColumnsEntry(int tableId, List<TableColumnDescriptor> descriptors) { this.tableId = tableId; - this.unique = unique; + this.descriptors = descriptors; } + /** Returns table id. */ public int tableId() { return tableId; } - public boolean unique() { - return unique; - } - - public boolean writeOnly() { - return writeOnly; + /** Returns descriptors of columns to add. */ + public List<TableColumnDescriptor> descriptors() { + return descriptors; } /** {@inheritDoc} */ diff --git a/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/CatalogServiceSelfTest.java b/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/CatalogServiceSelfTest.java index 003ab864f1..6b744176ef 100644 --- a/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/CatalogServiceSelfTest.java +++ b/modules/catalog/src/test/java/org/apache/ignite/internal/catalog/CatalogServiceSelfTest.java @@ -28,6 +28,7 @@ import static org.junit.jupiter.api.Assertions.assertNotSame; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.times; @@ -36,12 +37,16 @@ import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; import java.util.List; +import java.util.Set; import java.util.concurrent.CompletableFuture; +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.CreateTableParams; import org.apache.ignite.internal.catalog.commands.DefaultValue; import org.apache.ignite.internal.catalog.commands.DropTableParams; import org.apache.ignite.internal.catalog.descriptors.SchemaDescriptor; +import org.apache.ignite.internal.catalog.descriptors.TableColumnDescriptor; import org.apache.ignite.internal.catalog.descriptors.TableDescriptor; import org.apache.ignite.internal.catalog.events.CatalogEvent; import org.apache.ignite.internal.catalog.events.CatalogEventParameters; @@ -58,6 +63,8 @@ import org.apache.ignite.internal.metastorage.impl.StandaloneMetaStorageManager; import org.apache.ignite.internal.metastorage.server.SimpleInMemoryKeyValueStorage; import org.apache.ignite.internal.vault.VaultManager; import org.apache.ignite.internal.vault.inmemory.InMemoryVaultService; +import org.apache.ignite.lang.ColumnAlreadyExistsException; +import org.apache.ignite.lang.ColumnNotFoundException; import org.apache.ignite.lang.IgniteInternalException; import org.apache.ignite.lang.NodeStoppingException; import org.apache.ignite.lang.TableAlreadyExistsException; @@ -76,6 +83,9 @@ import org.mockito.Mockito; public class CatalogServiceSelfTest { private static final String TABLE_NAME = "myTable"; private static final String TABLE_NAME_2 = "myTable2"; + public static final String COLUMN_NAME = "VAL"; + private static final String NEW_COLUMN_NAME = "NEWCOL"; + private static final String NEW_COLUMN_NAME_2 = "NEWCOL2"; private MetaStorageManager metastore; @@ -234,12 +244,14 @@ public class CatalogServiceSelfTest { } @Test - public void testDropTable() { + public void testDropTable() throws InterruptedException { assertThat(service.createTable(simpleTable(TABLE_NAME)), willBe((Object) null)); assertThat(service.createTable(simpleTable(TABLE_NAME_2)), willBe((Object) null)); long beforeDropTimestamp = System.currentTimeMillis(); + Thread.sleep(5); + DropTableParams dropTableParams = DropTableParams.builder().schemaName("PUBLIC").tableName(TABLE_NAME).build(); assertThat(service.dropTable(dropTableParams), willBe((Object) null)); @@ -305,6 +317,197 @@ public class CatalogServiceSelfTest { assertThat(service.dropTable(params), willThrowFast(TableNotFoundException.class)); } + @Test + public void testAddColumn() throws InterruptedException { + assertThat(service.createTable(simpleTable(TABLE_NAME)), willBe((Object) null)); + + AlterTableAddColumnParams params = AlterTableAddColumnParams.builder() + .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 = System.currentTimeMillis(); + + Thread.sleep(5); + + assertThat(service.addColumn(params), willBe((Object) null)); + + // Validate catalog version from the past. + SchemaDescriptor schema = service.activeSchema(beforeAddedTimestamp); + assertNotNull(schema); + assertNotNull(schema.table(TABLE_NAME)); + + assertNull(schema.table(TABLE_NAME).column(NEW_COLUMN_NAME)); + + // Validate actual catalog + schema = service.activeSchema(System.currentTimeMillis()); + assertNotNull(schema); + assertNotNull(schema.table(TABLE_NAME)); + + // Validate column descriptor. + TableColumnDescriptor column = schema.table(TABLE_NAME).column(NEW_COLUMN_NAME); + + assertEquals(NEW_COLUMN_NAME, column.name()); + assertEquals(ColumnType.STRING, column.type()); + assertTrue(column.nullable()); + + assertEquals(DefaultValue.Type.CONSTANT, column.defaultValue().type()); + assertEquals("Ignite!", ((DefaultValue.ConstantValue) column.defaultValue()).value()); + + assertEquals(0, column.length()); + assertEquals(0, column.precision()); + assertEquals(0, column.scale()); + } + + @Test + public void testDropColumn() throws InterruptedException { + assertThat(service.createTable(simpleTable(TABLE_NAME)), willBe((Object) null)); + + // Validate dropping column + AlterTableDropColumnParams params = AlterTableDropColumnParams.builder() + .tableName(TABLE_NAME) + .columns(Set.of(COLUMN_NAME)) + .build(); + + long beforeAddedTimestamp = System.currentTimeMillis(); + + Thread.sleep(5); + + assertThat(service.dropColumn(params), willBe((Object) null)); + + // Validate catalog version from the past. + SchemaDescriptor schema = service.activeSchema(beforeAddedTimestamp); + assertNotNull(schema); + assertNotNull(schema.table(TABLE_NAME)); + + assertNotNull(schema.table(TABLE_NAME).column(COLUMN_NAME)); + + // Validate actual catalog + schema = service.activeSchema(System.currentTimeMillis()); + assertNotNull(schema); + assertNotNull(schema.table(TABLE_NAME)); + + assertNull(schema.table(TABLE_NAME).column(COLUMN_NAME)); + } + + @Test + public void testAddColumnIfExists() { + assertThat(service.createTable(simpleTable(TABLE_NAME)), willBe((Object) null)); + + AlterTableAddColumnParams params = AlterTableAddColumnParams.builder() + .tableName(TABLE_NAME) + .columns(List.of(ColumnParams.builder().name(NEW_COLUMN_NAME).type(ColumnType.INT32).nullable(true).build())) + .ifColumnNotExists(false) + .build(); + + assertThat(service.addColumn(params), willBe((Object) null)); + assertThat(service.addColumn(params), willThrow(ColumnAlreadyExistsException.class)); + + params = AlterTableAddColumnParams.builder() + .tableName(TABLE_NAME) + .columns(List.of(ColumnParams.builder().name(NEW_COLUMN_NAME).type(ColumnType.INT32).nullable(true).build())) + .ifColumnNotExists(true) + .build(); + + assertThat(service.addColumn(params), willThrow(ColumnAlreadyExistsException.class)); + } + + @Test + public void testAddDropMultipleColumns() { + assertThat(service.createTable(simpleTable(TABLE_NAME)), willBe((Object) null)); + + // Try to add multiple columns with 'IF NOT EXISTS' clause + AlterTableAddColumnParams addColumnParams = AlterTableAddColumnParams.builder() + .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() + )) + .ifColumnNotExists(true) + .build(); + + assertThat(service.addColumn(addColumnParams), willThrow(UnsupportedOperationException.class)); + + // Validate no column added. + SchemaDescriptor schema = service.activeSchema(System.currentTimeMillis()); + + assertNull(schema.table(TABLE_NAME).column(NEW_COLUMN_NAME)); + assertNull(schema.table(TABLE_NAME).column(NEW_COLUMN_NAME_2)); + + // Add multiple columns. + addColumnParams = AlterTableAddColumnParams.builder() + .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() + )) + .ifColumnNotExists(false) + .build(); + + assertThat(service.addColumn(addColumnParams), willBe((Object) null)); + + // Validate both columns added. + schema = service.activeSchema(System.currentTimeMillis()); + + assertNotNull(schema.table(TABLE_NAME).column(NEW_COLUMN_NAME)); + assertNotNull(schema.table(TABLE_NAME).column(NEW_COLUMN_NAME_2)); + + // Try to drop multiple columns with 'IF NOT EXISTS' clause + AlterTableDropColumnParams dropColumnParams = AlterTableDropColumnParams.builder() + .tableName(TABLE_NAME) + .columns(Set.of(NEW_COLUMN_NAME, NEW_COLUMN_NAME_2)) + .ifColumnExists(true) + .build(); + + assertThat(service.dropColumn(dropColumnParams), willThrow(UnsupportedOperationException.class)); + + // Validate no column dropped. + schema = service.activeSchema(System.currentTimeMillis()); + + assertNotNull(schema.table(TABLE_NAME).column(NEW_COLUMN_NAME)); + assertNotNull(schema.table(TABLE_NAME).column(NEW_COLUMN_NAME_2)); + + // Drop multiple columns. + dropColumnParams = AlterTableDropColumnParams.builder() + .tableName(TABLE_NAME) + .columns(Set.of(NEW_COLUMN_NAME, NEW_COLUMN_NAME_2)) + .ifColumnExists(false) + .build(); + + assertThat(service.dropColumn(dropColumnParams), willBe((Object) null)); + + // Validate both columns dropped. + schema = service.activeSchema(System.currentTimeMillis()); + + assertNull(schema.table(TABLE_NAME).column(NEW_COLUMN_NAME)); + assertNull(schema.table(TABLE_NAME).column(NEW_COLUMN_NAME_2)); + + // Check adding of existed column + addColumnParams = AlterTableAddColumnParams.builder() + .tableName(TABLE_NAME) + .columns(List.of( + ColumnParams.builder().name(NEW_COLUMN_NAME).type(ColumnType.INT32).nullable(true).build(), + ColumnParams.builder().name(COLUMN_NAME).type(ColumnType.INT32).nullable(true).build() + )) + .build(); + + assertThat(service.addColumn(addColumnParams), willThrow(ColumnAlreadyExistsException.class)); + + // Check dropping of non-existing column + dropColumnParams = AlterTableDropColumnParams.builder() + .tableName(TABLE_NAME) + .columns(Set.of(NEW_COLUMN_NAME, COLUMN_NAME)) + .build(); + + assertThat(service.dropColumn(dropColumnParams), willThrow(ColumnNotFoundException.class)); + } + @Test public void operationWillBeRetriedFiniteAmountOfTimes() { UpdateLog updateLogMock = Mockito.mock(UpdateLog.class); diff --git a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/ItSchemaChangeTableViewTest.java b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/ItSchemaChangeTableViewTest.java index e389f99377..9906221940 100644 --- a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/ItSchemaChangeTableViewTest.java +++ b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/ItSchemaChangeTableViewTest.java @@ -29,6 +29,7 @@ import org.apache.ignite.internal.schema.SchemaMismatchException; import org.apache.ignite.table.RecordView; import org.apache.ignite.table.Table; import org.apache.ignite.table.Tuple; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; /** @@ -151,7 +152,9 @@ class ItSchemaChangeTableViewTest extends AbstractSchemaChangeTest { /** * Rename column then add a new column with same name. + * TODO IGNITE-19486: Add similar test for KV view. */ + @Disabled("https://issues.apache.org/jira/browse/IGNITE-19486";) @Test void testRenameThenAddColumnWithSameName() throws Exception { List<Ignite> grid = startGrid(); diff --git a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ddl/DdlCommandHandlerWrapper.java b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ddl/DdlCommandHandlerWrapper.java index 21f39ea169..f97ce9c8ed 100644 --- a/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ddl/DdlCommandHandlerWrapper.java +++ b/modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/ddl/DdlCommandHandlerWrapper.java @@ -22,11 +22,15 @@ import java.util.concurrent.CompletableFuture; import org.apache.ignite.internal.catalog.CatalogManager; import org.apache.ignite.internal.distributionzones.DistributionZoneManager; import org.apache.ignite.internal.index.IndexManager; +import org.apache.ignite.internal.sql.engine.prepare.ddl.AlterTableAddCommand; +import org.apache.ignite.internal.sql.engine.prepare.ddl.AlterTableDropCommand; import org.apache.ignite.internal.sql.engine.prepare.ddl.CreateTableCommand; import org.apache.ignite.internal.sql.engine.prepare.ddl.DdlCommand; import org.apache.ignite.internal.sql.engine.prepare.ddl.DropTableCommand; import org.apache.ignite.internal.storage.DataStorageManager; import org.apache.ignite.internal.table.distributed.TableManager; +import org.apache.ignite.lang.ColumnAlreadyExistsException; +import org.apache.ignite.lang.ColumnNotFoundException; import org.apache.ignite.lang.TableAlreadyExistsException; import org.apache.ignite.lang.TableNotFoundException; @@ -70,6 +74,22 @@ public class DdlCommandHandlerWrapper extends DdlCommandHandler { .thenCompose(res -> catalogManager.dropTable(DdlToCatalogCommandConverter.convert((DropTableCommand) cmd)) .handle(handleModificationResult(((DropTableCommand) cmd).ifTableExists(), TableNotFoundException.class)) ); + } else if (cmd instanceof AlterTableAddCommand) { + AlterTableAddCommand addCommand = (AlterTableAddCommand) cmd; + + return ddlCommandFuture + .thenCompose(res -> catalogManager.addColumn(DdlToCatalogCommandConverter.convert(addCommand)) + .handle(handleModificationResult(addCommand.ifTableExists(), TableNotFoundException.class)) + .handle(handleModificationResult(addCommand.ifColumnNotExists(), ColumnAlreadyExistsException.class)) + ); + } else if (cmd instanceof AlterTableDropCommand) { + AlterTableDropCommand dropCommand = (AlterTableDropCommand) cmd; + + return ddlCommandFuture + .thenCompose(res -> catalogManager.dropColumn(DdlToCatalogCommandConverter.convert(dropCommand)) + .handle(handleModificationResult(dropCommand.ifTableExists(), TableNotFoundException.class)) + .handle(handleModificationResult(dropCommand.ifColumnExists(), ColumnNotFoundException.class)) + ); } return ddlCommandFuture; 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 e0e9f4f520..bce839b04c 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 @@ -19,10 +19,14 @@ package org.apache.ignite.internal.sql.engine.exec.ddl; import java.util.List; import java.util.stream.Collectors; +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.CreateTableParams; import org.apache.ignite.internal.catalog.commands.DefaultValue; import org.apache.ignite.internal.catalog.commands.DropTableParams; +import org.apache.ignite.internal.sql.engine.prepare.ddl.AlterTableAddCommand; +import org.apache.ignite.internal.sql.engine.prepare.ddl.AlterTableDropCommand; import org.apache.ignite.internal.sql.engine.prepare.ddl.ColumnDefinition; import org.apache.ignite.internal.sql.engine.prepare.ddl.CreateTableCommand; import org.apache.ignite.internal.sql.engine.prepare.ddl.DefaultValueDefinition; @@ -56,8 +60,38 @@ class DdlToCatalogCommandConverter { .build(); } + static AlterTableAddColumnParams convert(AlterTableAddCommand cmd) { + List<ColumnParams> columns = cmd.columns().stream().map(DdlToCatalogCommandConverter::convert).collect(Collectors.toList()); + + return AlterTableAddColumnParams.builder() + .schemaName(cmd.schemaName()) + .tableName(cmd.tableName()) + + .columns(columns) + .ifColumnNotExists(cmd.ifColumnNotExists()) + + .build(); + } + + static AlterTableDropColumnParams convert(AlterTableDropCommand cmd) { + return AlterTableDropColumnParams.builder() + .schemaName(cmd.schemaName()) + .tableName(cmd.tableName()) + + .columns(cmd.columns()) + .ifColumnExists(cmd.ifColumnExists()) + + .build(); + } + + private static ColumnParams convert(ColumnDefinition def) { - return new ColumnParams(def.name(), TypeUtils.columnType(def.type()), convert(def.defaultValueDefinition()), def.nullable()); + return ColumnParams.builder() + .name(def.name()) + .type(TypeUtils.columnType(def.type())) + .nullable(def.nullable()) + .defaultValue(convert(def.defaultValueDefinition())) + .build(); } private static DefaultValue convert(DefaultValueDefinition def) { diff --git a/modules/table/src/test/java/org/apache/ignite/internal/table/distributed/schema/FullTableSchemaTest.java b/modules/table/src/test/java/org/apache/ignite/internal/table/distributed/schema/FullTableSchemaTest.java index fbe2ce2830..5b3d44d615 100644 --- a/modules/table/src/test/java/org/apache/ignite/internal/table/distributed/schema/FullTableSchemaTest.java +++ b/modules/table/src/test/java/org/apache/ignite/internal/table/distributed/schema/FullTableSchemaTest.java @@ -47,7 +47,7 @@ class FullTableSchemaTest { @NotNull private static HashIndexDescriptor someIndex(int id, String name) { - return new HashIndexDescriptor(id, name, 1, List.of("a")); + return new HashIndexDescriptor(id, name, 1, List.of("a"), true); } @NotNull
