This is an automated email from the ASF dual-hosted git repository.
ppa 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 27d63dadb1 IGNITE-20724 Fixed incorrect schema column mapping when
deleting a column (#2748)
27d63dadb1 is described below
commit 27d63dadb1ec29f3bff03afce3aca40e986ee1d9
Author: Pavel Pereslegin <[email protected]>
AuthorDate: Fri Oct 27 14:35:21 2023 +0300
IGNITE-20724 Fixed incorrect schema column mapping when deleting a column
(#2748)
---
.../internal/sql/engine/ItCreateTableDdlTest.java | 69 +++++++++
.../apache/ignite/internal/schema/SchemaUtils.java | 10 +-
.../schema/registry/UpgradingRowAdapter.java | 28 ++++
.../internal/schema/SchemaColumnMapperTest.java | 169 +++++++++++++++++++++
4 files changed, 275 insertions(+), 1 deletion(-)
diff --git
a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItCreateTableDdlTest.java
b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItCreateTableDdlTest.java
index bf3beea782..323db5123f 100644
---
a/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItCreateTableDdlTest.java
+++
b/modules/runner/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItCreateTableDdlTest.java
@@ -17,18 +17,28 @@
package org.apache.ignite.internal.sql.engine;
+import static org.apache.ignite.internal.lang.IgniteStringFormatter.format;
import static
org.apache.ignite.internal.sql.engine.util.SqlTestUtils.assertThrowsSqlException;
import static org.apache.ignite.internal.table.TableTestUtils.getTableStrict;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.hasSize;
+import static org.hamcrest.Matchers.is;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import java.util.List;
+import java.util.Set;
+import org.apache.calcite.rel.type.RelDataType;
import org.apache.ignite.internal.app.IgniteImpl;
+import org.apache.ignite.internal.lang.IgniteStringBuilder;
import org.apache.ignite.internal.schema.Column;
+import org.apache.ignite.internal.schema.SchemaTestUtils;
import org.apache.ignite.internal.sql.BaseSqlIntegrationTest;
+import org.apache.ignite.internal.sql.engine.util.Commons;
+import org.apache.ignite.internal.sql.engine.util.TypeUtils;
import org.apache.ignite.internal.table.TableImpl;
+import org.apache.ignite.internal.type.NativeType;
+import org.apache.ignite.internal.type.NativeTypeSpec;
import org.apache.ignite.lang.ErrorGroups.Sql;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Test;
@@ -179,6 +189,65 @@ public class ItCreateTableDdlTest extends
BaseSqlIntegrationTest {
res = sql("SELECT c4 FROM my WHERE c1=3");
assertEquals(3, res.get(0).get(0));
+
+ // Checking the correctness of reading a row created on a different
version of the schema.
+ sql("ALTER TABLE my ADD COLUMN (c5 VARCHAR, c6 BOOLEAN)");
+ sql("ALTER TABLE my DROP COLUMN c4");
+ assertQuery("SELECT * FROM my WHERE c1=3")
+ .returns(3, "2", 3, null, null)
+ .check();
+ }
+
+ /**
+ * Adds columns of all supported types and checks that the row
+ * created on the old schema version is read correctly.
+ */
+ @Test
+ public void testDropAndAddColumnsAllTypes() {
+ List<NativeType> allTypes = SchemaTestUtils.ALL_TYPES;
+
+ Set<NativeTypeSpec> unsupportedTypes = Set.of(
+ // TODO https://issues.apache.org/jira/browse/IGNITE-18431
+ NativeTypeSpec.BITMASK,
+ // TODO https://issues.apache.org/jira/browse/IGNITE-19274
+ NativeTypeSpec.TIMESTAMP
+ );
+
+ // List of columns for 'ADD COLUMN' statement.
+ IgniteStringBuilder addColumnsList = new IgniteStringBuilder();
+ // List of columns for 'DROP COLUMN' statement.
+ IgniteStringBuilder dropColumnsList = new IgniteStringBuilder();
+
+ for (int i = 0; i < allTypes.size(); i++) {
+ NativeType type = allTypes.get(i);
+
+ if (unsupportedTypes.contains(type.spec())) {
+ continue;
+ }
+
+ RelDataType relDataType =
TypeUtils.native2relationalType(Commons.typeFactory(), type);
+
+ if (addColumnsList.length() > 0) {
+ addColumnsList.app(',');
+ dropColumnsList.app(',');
+ }
+
+ addColumnsList.app("c").app(i).app('
').app(relDataType.getSqlTypeName());
+ dropColumnsList.app("c").app(i);
+ }
+
+ sql("CREATE TABLE test (id INT PRIMARY KEY, val INT)");
+ sql("INSERT INTO test VALUES (0, 1)");
+ sql(format("ALTER TABLE test ADD COLUMN ({})",
addColumnsList.toString()));
+
+ List<List<Object>> res = sql("SELECT * FROM test");
+ assertThat(res.size(), is(1));
+ assertThat(res.get(0).size(), is(allTypes.size() -
unsupportedTypes.size() + /* initial columns */ 2));
+
+ sql(format("ALTER TABLE test DROP COLUMN ({})",
dropColumnsList.toString()));
+ assertQuery("SELECT * FROM test")
+ .returns(0, 1)
+ .check();
}
/**
diff --git
a/modules/schema/src/main/java/org/apache/ignite/internal/schema/SchemaUtils.java
b/modules/schema/src/main/java/org/apache/ignite/internal/schema/SchemaUtils.java
index dec51ca92d..72bfa76cb3 100644
---
a/modules/schema/src/main/java/org/apache/ignite/internal/schema/SchemaUtils.java
+++
b/modules/schema/src/main/java/org/apache/ignite/internal/schema/SchemaUtils.java
@@ -85,7 +85,15 @@ public class SchemaUtils {
mapper = ColumnMapping.createMapper(newDesc);
}
- mapper.add(newCol.schemaIndex(), oldCol.schemaIndex());
+ if (newCol.name().equals(oldCol.name())) {
+ mapper.add(newCol.schemaIndex(), oldCol.schemaIndex());
+ } else {
+ Column oldIdx = oldDesc.column(newCol.name());
+
+ assert oldIdx != null : newCol.name();
+
+ mapper.add(newCol.schemaIndex(), oldIdx.schemaIndex());
+ }
}
} else {
if (mapper == null) {
diff --git
a/modules/schema/src/main/java/org/apache/ignite/internal/schema/registry/UpgradingRowAdapter.java
b/modules/schema/src/main/java/org/apache/ignite/internal/schema/registry/UpgradingRowAdapter.java
index 3f8313fa34..6d37177bdd 100644
---
a/modules/schema/src/main/java/org/apache/ignite/internal/schema/registry/UpgradingRowAdapter.java
+++
b/modules/schema/src/main/java/org/apache/ignite/internal/schema/registry/UpgradingRowAdapter.java
@@ -111,6 +111,34 @@ public class UpgradingRowAdapter extends Row {
: newBinaryTupleSchema.value(this, colIdx);
}
+ /** {@inheritDoc} */
+ @Override
+ public boolean booleanValue(int colIdx) {
+ int mappedId = mapColumn(colIdx);
+
+ Column column = mappedId < 0 ? mapper.mappedColumn(colIdx) :
super.schema().column(mappedId);
+
+ if (NativeTypeSpec.BOOLEAN != column.type().spec()) {
+ throw new SchemaException("Type conversion is not supported yet.");
+ }
+
+ return mappedId < 0 ? (boolean) column.defaultValue() :
super.booleanValue(mappedId);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public Boolean booleanValueBoxed(int colIdx) {
+ int mappedId = mapColumn(colIdx);
+
+ Column column = mappedId < 0 ? mapper.mappedColumn(colIdx) :
super.schema().column(mappedId);
+
+ if (NativeTypeSpec.BOOLEAN != column.type().spec()) {
+ throw new SchemaException("Type conversion is not supported yet.");
+ }
+
+ return mappedId < 0 ? (Boolean) column.defaultValue() :
super.booleanValueBoxed(mappedId);
+ }
+
/** {@inheritDoc} */
@Override
public byte byteValue(int colIdx) throws InvalidTypeException {
diff --git
a/modules/schema/src/test/java/org/apache/ignite/internal/schema/SchemaColumnMapperTest.java
b/modules/schema/src/test/java/org/apache/ignite/internal/schema/SchemaColumnMapperTest.java
new file mode 100644
index 0000000000..fa9ebe8bc4
--- /dev/null
+++
b/modules/schema/src/test/java/org/apache/ignite/internal/schema/SchemaColumnMapperTest.java
@@ -0,0 +1,169 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.ignite.internal.schema;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+
+import java.util.Arrays;
+import java.util.Map;
+import java.util.Random;
+import java.util.Set;
+import java.util.function.Function;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+import org.apache.ignite.internal.schema.mapping.ColumnMapper;
+import org.apache.ignite.internal.testframework.BaseIgniteAbstractTest;
+import org.apache.ignite.internal.type.NativeType;
+import org.apache.ignite.internal.type.NativeTypes;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+/**
+ * Test checks the correctness of the {@link SchemaDescriptor#columnMapping()
mapper} created using
+ * the {@link SchemaUtils#columnMapper(SchemaDescriptor, SchemaDescriptor)}
method.
+ */
+public class SchemaColumnMapperTest extends BaseIgniteAbstractTest {
+ private static final int TOTAL_ITERATIONS = 30;
+
+ private static final SchemaDescriptor INITIAL_SCHEMA =
+ new SchemaDescriptor(0, new Column[]{new Column("ID",
NativeTypes.INT32, false)}, new Column[0]);
+
+ @ParameterizedTest
+ @ValueSource(ints = {1, 2, 3})
+ public void removeFirstColumns(int batchSize) {
+ SchemaDescriptor newSchema = INITIAL_SCHEMA;
+
+ // Sequentially add columns and check the mapping for the previous
version.
+ for (int i = 0; i < TOTAL_ITERATIONS; i += batchSize) {
+ SchemaDescriptor oldSchema = newSchema;
+ newSchema = addColumns(oldSchema, makeColumns(i, batchSize));
+
+ verifyMapping(oldSchema, newSchema);
+ }
+
+ // Sequentially remove columns located at the beginning, according to
the column order.
+ for (int i = 0; i < TOTAL_ITERATIONS; i += batchSize) {
+ SchemaDescriptor oldSchema = newSchema;
+ int[] idxs = IntStream.range(0, batchSize).toArray();
+ newSchema = removeColumns(newSchema, idxs);
+
+ verifyMapping(oldSchema, newSchema);
+ }
+ }
+
+ @ParameterizedTest
+ @ValueSource(ints = {1, 2, 3})
+ public void removeLastColumns(int batchSize) {
+ SchemaDescriptor newSchema = addColumns(INITIAL_SCHEMA, makeColumns(0,
TOTAL_ITERATIONS));
+
+ for (int i = TOTAL_ITERATIONS - 1; i >= 0; i -= batchSize) {
+ SchemaDescriptor oldSchema = newSchema;
+ int[] idxs = IntStream.range(i, i - batchSize).toArray();
+ newSchema = removeColumns(newSchema, idxs);
+
+ verifyMapping(oldSchema, newSchema);
+ }
+ }
+
+ @ParameterizedTest
+ @ValueSource(ints = {1, 2, 3})
+ public void removeRandomColumns(int batchSize) {
+ long seed = System.currentTimeMillis();
+
+ log.info("Using seed: " + seed);
+
+ Random rnd = new Random(seed);
+
+ SchemaDescriptor newSchema = addColumns(INITIAL_SCHEMA, makeColumns(0,
TOTAL_ITERATIONS));
+
+ for (int i = TOTAL_ITERATIONS - 1; i >= batchSize; i -= batchSize) {
+ int[] idxs = rnd.ints(i - batchSize,
i).distinct().limit(batchSize).toArray();
+
+ SchemaDescriptor oldSchema = newSchema;
+ newSchema = removeColumns(newSchema, idxs);
+
+ verifyMapping(oldSchema, newSchema);
+ }
+ }
+
+ private static void verifyMapping(SchemaDescriptor oldSchema,
SchemaDescriptor newSchema) {
+ Column[] oldCols = allColumns(oldSchema);
+ Column[] newCols = allColumns(newSchema);
+ ColumnMapper mapper = SchemaUtils.columnMapper(oldSchema, newSchema);
+
+ Map<Integer, Column> schemaIndexMap = Arrays.stream(oldCols)
+ .collect(Collectors.toMap(Column::schemaIndex,
Function.identity()));
+
+ for (Column column : newCols) {
+ int newSchemaIdx = column.schemaIndex();
+ int oldSchemaIdx = mapper.map(newSchemaIdx);
+
+ Column oldCol = oldSchemaIdx < 0 ?
mapper.mappedColumn(newSchemaIdx) : schemaIndexMap.get(oldSchemaIdx);
+
+ assertThat("old=" + oldSchema + ", new=" + newSchema, oldCol,
equalTo(column));
+ }
+ }
+
+ private static SchemaDescriptor addColumns(SchemaDescriptor schema, Column
... newColumns) {
+ Column[] oldColumns = schema.valueColumns().columns();
+ Column[] columns = Arrays.copyOf(oldColumns, oldColumns.length +
newColumns.length);
+ System.arraycopy(newColumns, 0, columns, oldColumns.length,
newColumns.length);
+
+ return new SchemaDescriptor(schema.version() + 1,
schema.keyColumns().columns(), columns);
+ }
+
+ private static SchemaDescriptor removeColumns(SchemaDescriptor schema, int
... idxs) {
+ Column[] oldColumns = schema.valueColumns().columns();
+ Column[] newColumns = new Column[oldColumns.length - idxs.length];
+ Set<Integer> colIdxsSet =
Arrays.stream(idxs).boxed().collect(Collectors.toSet());
+
+ int n = 0;
+ for (int i = 0; i < oldColumns.length; i++) {
+ if (!colIdxsSet.contains(i)) {
+ newColumns[n++] = oldColumns[i];
+ }
+ }
+
+ return new SchemaDescriptor(schema.version() + 1,
schema.keyColumns().columns(), newColumns);
+ }
+
+ private static Column[] makeColumns(int offset, int count) {
+ Column[] columns = new Column[count];
+
+ for (int i = 0; i < count; i++) {
+ int index = offset + i;
+
+ NativeType type =
SchemaTestUtils.ALL_TYPES.get((SchemaTestUtils.ALL_TYPES.size() - 1) % (index +
1));
+
+ columns[i] = new Column(index, "COL" + index, type, false);
+ }
+
+ return columns;
+ }
+
+ private static Column[] allColumns(SchemaDescriptor schemaDescriptor) {
+ Column[] keyColumns = schemaDescriptor.keyColumns().columns();
+ Column[] valueColumns = schemaDescriptor.valueColumns().columns();
+
+ Column[] columns = Arrays.copyOf(keyColumns, keyColumns.length +
valueColumns.length);
+ System.arraycopy(valueColumns, 0, columns, keyColumns.length,
valueColumns.length);
+
+ return columns;
+ }
+}
