This is an automated email from the ASF dual-hosted git repository. amashenkov pushed a commit to branch ignite-13667 in repository https://gitbox.apache.org/repos/asf/ignite-3.git
commit 3b8c408156b8b5e1082941be728f0d8c5f80e19b Author: Andrew Mashenkov <[email protected]> AuthorDate: Mon Oct 18 13:56:40 2021 +0300 Add column order. --- .../schema/definition/PrimaryKeyDefinition.java | 6 +-- .../ignite/schema/definition/TableDefinition.java | 9 +++-- .../org/apache/ignite/internal/schema/Column.java | 40 ++++++++++++++++-- .../ignite/internal/schema/SchemaDescriptor.java | 2 + .../SchemaConfigurationConverter.java | 9 ++--- .../configuration/SchemaDescriptorConverter.java | 40 +++++++++--------- .../schema/configuration/TableValidatorImpl.java | 9 +---- .../schema/definition/TableDefinitionImpl.java | 47 +++++++--------------- .../builder/PrimaryKeyDefinitionBuilderImpl.java | 8 ++-- .../internal/schema/SchemaDescriptorTest.java | 35 ++++++++++++++++ .../SchemaConfigurationConverterTest.java | 2 +- 11 files changed, 126 insertions(+), 81 deletions(-) diff --git a/modules/api/src/main/java/org/apache/ignite/schema/definition/PrimaryKeyDefinition.java b/modules/api/src/main/java/org/apache/ignite/schema/definition/PrimaryKeyDefinition.java index 77d057f..e3249f0 100644 --- a/modules/api/src/main/java/org/apache/ignite/schema/definition/PrimaryKeyDefinition.java +++ b/modules/api/src/main/java/org/apache/ignite/schema/definition/PrimaryKeyDefinition.java @@ -27,16 +27,16 @@ public interface PrimaryKeyDefinition extends SchemaObject { String PRIMARY_KEY_NAME = "PK"; /** - * Configured index columns. + * Returns key columns. * - * @return Index columns. + * @return Key columns. */ Set<String> columns(); /** * Returns affinity columns. * - * @return Columns list. + * @return Affinity columns. */ Set<String> affinityColumns(); } diff --git a/modules/api/src/main/java/org/apache/ignite/schema/definition/TableDefinition.java b/modules/api/src/main/java/org/apache/ignite/schema/definition/TableDefinition.java index 21b1a15..58e192c 100644 --- a/modules/api/src/main/java/org/apache/ignite/schema/definition/TableDefinition.java +++ b/modules/api/src/main/java/org/apache/ignite/schema/definition/TableDefinition.java @@ -18,6 +18,7 @@ package org.apache.ignite.schema.definition; import java.util.Collection; +import java.util.Set; import org.apache.ignite.schema.definition.index.IndexDefinition; import org.apache.ignite.schema.modification.TableModificationBuilder; @@ -38,21 +39,21 @@ public interface TableDefinition extends SchemaObject { * * @return List of columns. */ - Collection<ColumnDefinition> keyColumns(); + Set<String> keyColumns(); /** * Returns affinity columns. * * @return List of columns. */ - Collection<ColumnDefinition> affinityColumns(); + Set<String> affinityColumns(); /** - * Returns value columns. + * Returns table columns. * * @return List of columns. */ - Collection<ColumnDefinition> valueColumns(); + Collection<ColumnDefinition> columns(); /** * Returns table indices. diff --git a/modules/schema/src/main/java/org/apache/ignite/internal/schema/Column.java b/modules/schema/src/main/java/org/apache/ignite/internal/schema/Column.java index 24cf1b1..a636fb7 100644 --- a/modules/schema/src/main/java/org/apache/ignite/internal/schema/Column.java +++ b/modules/schema/src/main/java/org/apache/ignite/internal/schema/Column.java @@ -24,15 +24,20 @@ import org.apache.ignite.internal.tostring.S; import org.jetbrains.annotations.NotNull; /** - * Column description for a type schema. Column contains a column name, a column type and a nullability flag. + * Column descriptor which contains a column name, a type and a nullability flag. * <p> * Column instances are comparable in lexicographic order, native type first and then column name. Nullability * flag is not taken into account when columns are compared. + * <p> + * Because of columns must be written to a row in specific order, column write order may differs from the user order */ public class Column implements Comparable<Column>, Serializable { /** Absolute index in schema descriptor. */ private final int schemaIndex; + /** User column order as defined in table definition. */ + private final int columnOrder; + /** * Column name. */ @@ -64,26 +69,44 @@ public class Column implements Comparable<Column>, Serializable { NativeType type, boolean nullable ) { - this(-1, name, type, nullable, (Supplier<Object> & Serializable)() -> null); + this(-1, -1, name, type, nullable, (Supplier<Object> & Serializable)() -> null); + } + + /** + * @param name Column name. + * @param type An instance of column data type. + * @param nullable If {@code false}, null values will not be allowed for this column. + * @param defValSup Default value supplier. + */ + public Column( + String name, + NativeType type, + boolean nullable, + @NotNull Supplier<Object> defValSup + ) { + this(-1, -1, name, type, nullable, defValSup); } /** + * @param columnOrder Column order in table definition. * @param name Column name. * @param type An instance of column data type. * @param nullable If {@code false}, null values will not be allowed for this column. * @param defValSup Default value supplier. */ public Column( + int columnOrder, String name, NativeType type, boolean nullable, @NotNull Supplier<Object> defValSup ) { - this(-1, name, type, nullable, defValSup); + this(-1, columnOrder, name, type, nullable, defValSup); } /** * @param schemaIndex Absolute index of this column in its schema descriptor. + * @param columnOrder Column order defined in table definition. * @param name Column name. * @param type An instance of column data type. * @param nullable If {@code false}, null values will not be allowed for this column. @@ -91,12 +114,14 @@ public class Column implements Comparable<Column>, Serializable { */ private Column( int schemaIndex, + int columnOrder, String name, NativeType type, boolean nullable, @NotNull Supplier<Object> defValSup ) { this.schemaIndex = schemaIndex; + this.columnOrder = columnOrder; this.name = name; this.type = type; this.nullable = nullable; @@ -111,6 +136,13 @@ public class Column implements Comparable<Column>, Serializable { } /** + * @return User column order as defined in table definition. + */ + public int columnOrder() { + return columnOrder; + } + + /** * @return Column name. */ public String name() { @@ -198,7 +230,7 @@ public class Column implements Comparable<Column>, Serializable { * @return Column. */ public Column copy(int schemaIndex) { - return new Column(schemaIndex, name, type, nullable, defValSup); + return new Column(schemaIndex, columnOrder, name, type, nullable, defValSup); } /** {@inheritDoc} */ diff --git a/modules/schema/src/main/java/org/apache/ignite/internal/schema/SchemaDescriptor.java b/modules/schema/src/main/java/org/apache/ignite/internal/schema/SchemaDescriptor.java index 5c87a0e..b5708ce 100644 --- a/modules/schema/src/main/java/org/apache/ignite/internal/schema/SchemaDescriptor.java +++ b/modules/schema/src/main/java/org/apache/ignite/internal/schema/SchemaDescriptor.java @@ -98,6 +98,8 @@ public class SchemaDescriptor implements Serializable { * @return {@code true} if the column belongs to the key chunk, {@code false} otherwise. */ public boolean isKeyColumn(int idx) { + validateColumnIndex(idx); + return idx < keyCols.length(); } diff --git a/modules/schema/src/main/java/org/apache/ignite/internal/schema/configuration/SchemaConfigurationConverter.java b/modules/schema/src/main/java/org/apache/ignite/internal/schema/configuration/SchemaConfigurationConverter.java index 496202c..0f85414 100644 --- a/modules/schema/src/main/java/org/apache/ignite/internal/schema/configuration/SchemaConfigurationConverter.java +++ b/modules/schema/src/main/java/org/apache/ignite/internal/schema/configuration/SchemaConfigurationConverter.java @@ -402,16 +402,13 @@ public class SchemaConfigurationConverter { tblChg.changeColumns(colsChg -> { int colIdx = 0; - for (ColumnDefinition col : tbl.keyColumns()) - colsChg.create(String.valueOf(colIdx++), colChg -> convert(col, colChg)); - - for (ColumnDefinition col : tbl.valueColumns()) + for (ColumnDefinition col : tbl.columns()) colsChg.create(String.valueOf(colIdx++), colChg -> convert(col, colChg)); }); tblChg.changePrimaryKey(pkCng -> { - pkCng.changeColumns(tbl.keyColumns().stream().map(ColumnDefinition::name).toArray(String[]::new)) - .changeAffinityColumns(tbl.affinityColumns().stream().map(ColumnDefinition::name).toArray(String[]::new)); + pkCng.changeColumns(tbl.keyColumns().toArray(String[]::new)) + .changeAffinityColumns(tbl.affinityColumns().toArray(String[]::new)); }); return tblChg; diff --git a/modules/schema/src/main/java/org/apache/ignite/internal/schema/configuration/SchemaDescriptorConverter.java b/modules/schema/src/main/java/org/apache/ignite/internal/schema/configuration/SchemaDescriptorConverter.java index b343a6f..9f6dcbb 100644 --- a/modules/schema/src/main/java/org/apache/ignite/internal/schema/configuration/SchemaDescriptorConverter.java +++ b/modules/schema/src/main/java/org/apache/ignite/internal/schema/configuration/SchemaDescriptorConverter.java @@ -27,6 +27,7 @@ import java.time.LocalDateTime; import java.time.LocalTime; import java.util.ArrayList; import java.util.List; +import java.util.Set; import java.util.function.Supplier; import org.apache.ignite.internal.schema.Column; import org.apache.ignite.internal.schema.DecimalNativeType; @@ -143,15 +144,15 @@ public class SchemaDescriptorConverter { } /** - * Convert column from public configuration to internal. + * Creates a column for given column definition. * - * @param colCfg Column to confvert. + * @param colCfg Column definition. * @return Internal Column. */ - private static Column convert(ColumnDefinition colCfg) { + private static Column convert(int columnOrder, ColumnDefinition colCfg) { NativeType type = convert(colCfg.type()); - return new Column(colCfg.name(), type, colCfg.nullable(), new ConstantSupplier(convertDefault(type, (String)colCfg.defaultValue()))); + return new Column(columnOrder, colCfg.name(), type, colCfg.nullable(), new ConstantSupplier(convertDefault(type, (String)colCfg.defaultValue()))); } /** @@ -165,8 +166,6 @@ public class SchemaDescriptorConverter { if (dflt == null || dflt.isEmpty()) return null; - assert dflt instanceof String; - switch (type.spec()) { case INT8: return Byte.parseByte(dflt); @@ -209,24 +208,27 @@ public class SchemaDescriptorConverter { * @return SchemaDescriptor. */ public static SchemaDescriptor convert(int schemaVer, TableDefinition tblCfg) { - List<ColumnDefinition> keyColsCfg = new ArrayList<>(tblCfg.keyColumns()); - - Column[] keyCols = new Column[keyColsCfg.size()]; + Set<String> keyColumnsNames = tblCfg.keyColumns(); - for (int i = 0; i < keyCols.length; i++) - keyCols[i] = convert(keyColsCfg.get(i)); + List<Column> keyCols = new ArrayList<>(keyColumnsNames.size()); + List<Column> valCols = new ArrayList<>(tblCfg.columns().size() - keyColumnsNames.size()); - String[] affCols = tblCfg.affinityColumns().stream().map(ColumnDefinition::name) - .toArray(String[]::new); + int idx = 0; - List<ColumnDefinition> valColsCfg = new ArrayList<>(tblCfg.valueColumns()); + for (ColumnDefinition col : tblCfg.columns()) { + if (keyColumnsNames.contains(col.name())) + keyCols.add(convert(idx, col)); + else + valCols.add(convert(idx, col)); - Column[] valCols = new Column[valColsCfg.size()]; - - for (int i = 0; i < valCols.length; i++) - valCols[i] = convert(valColsCfg.get(i)); + idx++; + } - return new SchemaDescriptor(schemaVer, keyCols, affCols, valCols); + return new SchemaDescriptor( + schemaVer, + keyCols.toArray(Column[]::new), + tblCfg.affinityColumns().toArray(String[]::new), + valCols.toArray(Column[]::new)); } /** diff --git a/modules/schema/src/main/java/org/apache/ignite/internal/schema/configuration/TableValidatorImpl.java b/modules/schema/src/main/java/org/apache/ignite/internal/schema/configuration/TableValidatorImpl.java index 9f4f782..ed7867b 100644 --- a/modules/schema/src/main/java/org/apache/ignite/internal/schema/configuration/TableValidatorImpl.java +++ b/modules/schema/src/main/java/org/apache/ignite/internal/schema/configuration/TableValidatorImpl.java @@ -17,9 +17,6 @@ package org.apache.ignite.internal.schema.configuration; -import java.util.ArrayList; -import java.util.Collection; -import java.util.stream.Collectors; import org.apache.ignite.configuration.NamedListView; import org.apache.ignite.configuration.schemas.table.TableValidator; import org.apache.ignite.configuration.schemas.table.TableView; @@ -28,7 +25,6 @@ import org.apache.ignite.configuration.validation.ValidationIssue; import org.apache.ignite.configuration.validation.Validator; import org.apache.ignite.internal.schema.definition.TableDefinitionImpl; import org.apache.ignite.internal.schema.definition.builder.TableSchemaBuilderImpl; -import org.apache.ignite.schema.definition.ColumnDefinition; /** * Table schema configuration validator implementation. @@ -50,10 +46,7 @@ public class TableValidatorImpl implements Validator<TableValidator, NamedListVi assert !tbl.keyColumns().isEmpty(); assert !tbl.affinityColumns().isEmpty(); - Collection<ColumnDefinition> allColumns = new ArrayList<>(tbl.keyColumns()); - allColumns.addAll(tbl.valueColumns()); - - TableSchemaBuilderImpl.validateIndices(tbl.indices(), allColumns, tbl.affinityColumns().stream().map(ColumnDefinition::name).collect(Collectors.toSet())); + TableSchemaBuilderImpl.validateIndices(tbl.indices(), tbl.columns(), tbl.affinityColumns()); } catch (IllegalArgumentException e) { ctx.addIssue(new ValidationIssue("Validator works success by key " + ctx.currentKey() + ". Found " diff --git a/modules/schema/src/main/java/org/apache/ignite/internal/schema/definition/TableDefinitionImpl.java b/modules/schema/src/main/java/org/apache/ignite/internal/schema/definition/TableDefinitionImpl.java index 95a0aab..e355b9c 100644 --- a/modules/schema/src/main/java/org/apache/ignite/internal/schema/definition/TableDefinitionImpl.java +++ b/modules/schema/src/main/java/org/apache/ignite/internal/schema/definition/TableDefinitionImpl.java @@ -17,11 +17,9 @@ package org.apache.ignite.internal.schema.definition; -import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.LinkedHashMap; -import java.util.List; import java.util.Map; import java.util.Set; import org.apache.ignite.internal.schema.modification.TableModificationBuilderImpl; @@ -41,58 +39,41 @@ public class TableDefinitionImpl extends AbstractSchemaObject implements TableDe private final String schemaName; /** Key columns. */ - private final LinkedHashMap<String, ColumnDefinition> cols; + private final LinkedHashMap<String, ColumnDefinition> colMap; /** Indices. */ private final Map<String, IndexDefinition> indices; /** Cached key columns. */ - private final List<ColumnDefinition> keyCols; + private final Set<String> keyCols; /** Cached key affinity columns. */ - private final List<ColumnDefinition> affCols; - - /** Cached value columns. */ - private final List<ColumnDefinition> valCols; + private final Set<String> affCols; /** * Constructor. * * @param schemaName Schema name. * @param tableName Table name. - * @param cols Columns. + * @param colMap Columns. * @param primaryKeyDefinition Primary key. * @param indices Indices. */ public TableDefinitionImpl( String schemaName, String tableName, - LinkedHashMap<String, ColumnDefinition> cols, + LinkedHashMap<String, ColumnDefinition> colMap, PrimaryKeyDefinition primaryKeyDefinition, Map<String, IndexDefinition> indices ) { super(tableName); this.schemaName = schemaName; - this.cols = cols; + this.colMap = colMap; this.indices = indices; - Set<String> pkCols = primaryKeyDefinition.columns(); - Set<String> pkAffCols = primaryKeyDefinition.affinityColumns(); - - keyCols = new ArrayList<>(pkCols.size()); - affCols = new ArrayList<>(pkAffCols.size()); - valCols = new ArrayList<>(cols.size() - pkCols.size()); - - for (Map.Entry<String, ColumnDefinition> e : cols.entrySet()) { - if (pkCols.contains(e.getKey())) { - keyCols.add(e.getValue()); - - if (pkAffCols.contains(e.getValue().name())) - affCols.add(e.getValue()); - } else - valCols.add(e.getValue()); - } + keyCols = primaryKeyDefinition.columns(); + affCols = primaryKeyDefinition.affinityColumns(); } /** {@inheritDoc} */ @@ -101,18 +82,18 @@ public class TableDefinitionImpl extends AbstractSchemaObject implements TableDe } /** {@inheritDoc} */ - @Override public Collection<ColumnDefinition> keyColumns() { + @Override public Set<String> keyColumns() { return keyCols; } /** {@inheritDoc} */ - @Override public Collection<ColumnDefinition> affinityColumns() { + @Override public Set<String> affinityColumns() { return affCols; } /** {@inheritDoc} */ - @Override public Collection<ColumnDefinition> valueColumns() { - return valCols; + @Override public Collection<ColumnDefinition> columns() { + return colMap.values(); } /** {@inheritDoc} */ @@ -130,7 +111,7 @@ public class TableDefinitionImpl extends AbstractSchemaObject implements TableDe * @return {@code True} if column with given name already exists, {@code false} otherwise. */ public boolean hasColumn(String name) { - return cols.containsKey(name); + return colMap.containsKey(name); } /** @@ -138,7 +119,7 @@ public class TableDefinitionImpl extends AbstractSchemaObject implements TableDe * @return {@code True} if key column with given name already exists, {@code false} otherwise. */ public boolean hasKeyColumn(String name) { - return keyCols.stream().anyMatch(c -> c.name().equals(name)); + return keyCols.contains(name); } /** {@inheritDoc} */ diff --git a/modules/schema/src/main/java/org/apache/ignite/internal/schema/definition/builder/PrimaryKeyDefinitionBuilderImpl.java b/modules/schema/src/main/java/org/apache/ignite/internal/schema/definition/builder/PrimaryKeyDefinitionBuilderImpl.java index b85006c..d112c85 100644 --- a/modules/schema/src/main/java/org/apache/ignite/internal/schema/definition/builder/PrimaryKeyDefinitionBuilderImpl.java +++ b/modules/schema/src/main/java/org/apache/ignite/internal/schema/definition/builder/PrimaryKeyDefinitionBuilderImpl.java @@ -21,6 +21,7 @@ import java.util.Map; import java.util.Set; import org.apache.ignite.internal.schema.definition.index.PrimaryKeyDefinitionImpl; import org.apache.ignite.internal.tostring.IgniteToStringInclude; +import org.apache.ignite.internal.util.ArrayUtils; import org.apache.ignite.schema.definition.PrimaryKeyDefinition; import org.apache.ignite.schema.definition.builder.PrimaryKeyDefinitionBuilder; import org.apache.ignite.schema.definition.builder.SchemaObjectBuilder; @@ -70,13 +71,14 @@ public class PrimaryKeyDefinitionBuilderImpl implements SchemaObjectBuilder, Pri Set<String> affCols; - if (affinityColumns != null) { + if (ArrayUtils.nullOrEmpty(affinityColumns)) + affCols = cols; + else { affCols = Set.of(affinityColumns); if (!cols.containsAll(affCols)) throw new IllegalStateException("Schema definition error: All affinity columns must be part of key."); - } else - affCols = cols; + } return new PrimaryKeyDefinitionImpl(cols, affCols); } diff --git a/modules/schema/src/test/java/org/apache/ignite/internal/schema/SchemaDescriptorTest.java b/modules/schema/src/test/java/org/apache/ignite/internal/schema/SchemaDescriptorTest.java index 70ac008..43abf2f 100644 --- a/modules/schema/src/test/java/org/apache/ignite/internal/schema/SchemaDescriptorTest.java +++ b/modules/schema/src/test/java/org/apache/ignite/internal/schema/SchemaDescriptorTest.java @@ -17,6 +17,9 @@ package org.apache.ignite.internal.schema; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -49,4 +52,36 @@ public class SchemaDescriptorTest { for (int i = 0; i < desc.length(); i++) assertEquals(i, desc.column(i).schemaIndex()); } + + /** + * + */ + @Test + public void columnOrderedAccess() { + Column[] keyColumns = { + new Column(0, "columnA", NativeTypes.INT8, false, () -> null), + new Column(1, "columnB", NativeTypes.UUID, false, () -> null), + new Column(2, "columnC", NativeTypes.INT32, false, () -> null), + }; + + Column[] valColumns = { + new Column(3, "columnD", NativeTypes.INT8, false, () -> null), + new Column(4, "columnE", NativeTypes.UUID, false, () -> null), + new Column(5, "columnF", NativeTypes.INT32, false, () -> null), + }; + + List<Column> columns = new ArrayList<>(); + Collections.addAll(columns, keyColumns); + Collections.addAll(columns, valColumns); + + SchemaDescriptor desc = new SchemaDescriptor(1, keyColumns, valColumns); + + assertEquals(6, desc.length()); + + for (int i = 0; i < columns.size(); i++) { + Column col = desc.column(i); + + assertEquals(columns.get(col.columnOrder()), col); + } + } } diff --git a/modules/schema/src/test/java/org/apache/ignite/internal/schema/configuration/SchemaConfigurationConverterTest.java b/modules/schema/src/test/java/org/apache/ignite/internal/schema/configuration/SchemaConfigurationConverterTest.java index 6707be1..2b4ad91 100644 --- a/modules/schema/src/test/java/org/apache/ignite/internal/schema/configuration/SchemaConfigurationConverterTest.java +++ b/modules/schema/src/test/java/org/apache/ignite/internal/schema/configuration/SchemaConfigurationConverterTest.java @@ -245,7 +245,7 @@ public class SchemaConfigurationConverterTest { assertEquals(tbl.indices().size(), tbl2.indices().size()); assertEquals(tbl.keyColumns().size(), tbl2.keyColumns().size()); assertEquals(tbl.affinityColumns().size(), tbl2.affinityColumns().size()); - assertEquals(tbl.valueColumns().size(), tbl2.valueColumns().size()); + assertEquals(tbl.columns().size(), tbl2.columns().size()); } /**
