You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by al...@apache.org on 2022/09/02 08:31:57 UTC
[ignite] branch ignite-2.14 updated: IGNITE-17597 SQL Calcite: Fix indexes registration after add/drop column - Fixes #10233.
This is an automated email from the ASF dual-hosted git repository.
alexpl pushed a commit to branch ignite-2.14
in repository https://gitbox.apache.org/repos/asf/ignite.git
The following commit(s) were added to refs/heads/ignite-2.14 by this push:
new 53709b4f623 IGNITE-17597 SQL Calcite: Fix indexes registration after add/drop column - Fixes #10233.
53709b4f623 is described below
commit 53709b4f6230555751f8c436ad251d20bb65d169
Author: Aleksey Plekhanov <pl...@gmail.com>
AuthorDate: Fri Sep 2 11:26:21 2022 +0300
IGNITE-17597 SQL Calcite: Fix indexes registration after add/drop column - Fixes #10233.
Signed-off-by: Aleksey Plekhanov <pl...@gmail.com>
(cherry picked from commit 6c8cb76ad3276189b98b5b6e31527a6e59a4d906)
---
.../query/calcite/schema/CacheIndexImpl.java | 5 +
.../calcite/schema/CacheTableDescriptorImpl.java | 8 ++
.../query/calcite/schema/SchemaHolderImpl.java | 124 +++++++++++++++------
.../schema/SystemViewTableDescriptorImpl.java | 4 +-
.../query/calcite/schema/TableDescriptor.java | 8 ++
.../integration/TableDdlIntegrationTest.java | 46 ++++++++
.../query/calcite/planner/AbstractPlannerTest.java | 5 +
7 files changed, 163 insertions(+), 37 deletions(-)
diff --git a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/schema/CacheIndexImpl.java b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/schema/CacheIndexImpl.java
index 59eecbbab34..5b969b03baa 100644
--- a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/schema/CacheIndexImpl.java
+++ b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/schema/CacheIndexImpl.java
@@ -83,6 +83,11 @@ public class CacheIndexImpl implements IgniteIndex {
return tbl;
}
+ /** Underlying query index. */
+ public Index queryIndex() {
+ return idx;
+ }
+
/** {@inheritDoc} */
@Override public IgniteLogicalIndexScan toRel(
RelOptCluster cluster,
diff --git a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/schema/CacheTableDescriptorImpl.java b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/schema/CacheTableDescriptorImpl.java
index 5eb394c2f41..6d23cde0a3e 100644
--- a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/schema/CacheTableDescriptorImpl.java
+++ b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/schema/CacheTableDescriptorImpl.java
@@ -20,7 +20,10 @@ package org.apache.ignite.internal.processors.query.calcite.schema;
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.BitSet;
+import java.util.Collection;
+import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Objects;
@@ -557,6 +560,11 @@ public class CacheTableDescriptorImpl extends NullInitializerExpressionFactory
return fieldName == null ? null : descriptorsMap.get(fieldName);
}
+ /** {@inheritDoc} */
+ @Override public Collection<CacheColumnDescriptor> columnDescriptors() {
+ return Collections.unmodifiableList(Arrays.asList(descriptors));
+ }
+
/** {@inheritDoc} */
@Override public ColocationGroup colocationGroup(MappingQueryContext ctx) {
GridCacheContext<?, ?> cctx = cacheContext();
diff --git a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/schema/SchemaHolderImpl.java b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/schema/SchemaHolderImpl.java
index a810d8e56bf..009f0dc03ea 100644
--- a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/schema/SchemaHolderImpl.java
+++ b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/schema/SchemaHolderImpl.java
@@ -28,6 +28,8 @@ import org.apache.calcite.rel.RelCollations;
import org.apache.calcite.rel.RelFieldCollation;
import org.apache.calcite.schema.SchemaPlus;
import org.apache.calcite.tools.Frameworks;
+import org.apache.calcite.util.ImmutableBitSet;
+import org.apache.calcite.util.mapping.Mappings;
import org.apache.ignite.cache.CacheMode;
import org.apache.ignite.cache.affinity.AffinityFunction;
import org.apache.ignite.cluster.ClusterNode;
@@ -42,6 +44,7 @@ import org.apache.ignite.internal.processors.query.calcite.exec.exp.IgniteScalar
import org.apache.ignite.internal.processors.query.calcite.trait.TraitUtils;
import org.apache.ignite.internal.processors.query.calcite.type.IgniteTypeFactory;
import org.apache.ignite.internal.processors.query.calcite.util.AbstractService;
+import org.apache.ignite.internal.processors.query.calcite.util.Commons;
import org.apache.ignite.internal.processors.query.schema.SchemaChangeListener;
import org.apache.ignite.internal.processors.query.schema.management.IndexDescriptor;
import org.apache.ignite.internal.processors.subscription.GridInternalSubscriptionProcessor;
@@ -160,33 +163,24 @@ public class SchemaHolderImpl extends AbstractService implements SchemaHolder, S
}
/** {@inheritDoc} */
- @Override public synchronized void onSchemaCreated(String schemaName) {
+ @Override public void onSchemaCreated(String schemaName) {
igniteSchemas.putIfAbsent(schemaName, new IgniteSchema(schemaName));
rebuild();
}
/** {@inheritDoc} */
- @Override public synchronized void onSchemaDropped(String schemaName) {
+ @Override public void onSchemaDropped(String schemaName) {
igniteSchemas.remove(schemaName);
rebuild();
}
/** {@inheritDoc} */
- @Override public synchronized void onSqlTypeCreated(
+ @Override public void onSqlTypeCreated(
String schemaName,
GridQueryTypeDescriptor typeDesc,
GridCacheContextInfo<?, ?> cacheInfo
) {
- IgniteSchema schema = igniteSchemas.computeIfAbsent(schemaName, IgniteSchema::new);
-
- String tblName = typeDesc.tableName();
-
- CacheTableDescriptorImpl desc =
- new CacheTableDescriptorImpl(cacheInfo, typeDesc, affinityIdentity(cacheInfo.config()));
-
- schema.addTable(tblName, new CacheTableImpl(ctx, desc));
-
- rebuild();
+ publishTable(schemaName, typeDesc.tableName(), createTable(typeDesc, cacheInfo));
}
/** {@inheritDoc} */
@@ -196,7 +190,19 @@ public class SchemaHolderImpl extends AbstractService implements SchemaHolder, S
GridCacheContextInfo<?, ?> cacheInfo,
List<QueryField> cols
) {
- onSqlTypeCreated(schemaName, typeDesc, cacheInfo);
+ IgniteCacheTable oldTbl = table(schemaName, typeDesc.tableName());
+ assert oldTbl != null;
+
+ IgniteCacheTable newTbl = createTable(typeDesc, cacheInfo);
+
+ // Recreate indexes for the new table without columns shift.
+ for (IgniteIndex idx : oldTbl.indexes().values()) {
+ CacheIndexImpl idx0 = (CacheIndexImpl)idx;
+
+ newTbl.addIndex(new CacheIndexImpl(idx0.collation(), idx0.name(), idx0.queryIndex(), newTbl));
+ }
+
+ publishTable(schemaName, typeDesc.tableName(), newTbl);
}
/** {@inheritDoc} */
@@ -206,7 +212,53 @@ public class SchemaHolderImpl extends AbstractService implements SchemaHolder, S
GridCacheContextInfo<?, ?> cacheInfo,
List<String> cols
) {
- onSqlTypeCreated(schemaName, typeDesc, cacheInfo);
+ IgniteCacheTable oldTbl = table(schemaName, typeDesc.tableName());
+ assert oldTbl != null;
+
+ IgniteCacheTable newTbl = createTable(typeDesc, cacheInfo);
+
+ // Recreate indexes for the new table with columns shift.
+ int colsCnt = oldTbl.descriptor().columnDescriptors().size();
+ ImmutableBitSet.Builder retainedCols = ImmutableBitSet.builder();
+ retainedCols.set(0, colsCnt);
+
+ for (String droppedCol : cols)
+ retainedCols.clear(oldTbl.descriptor().columnDescriptor(droppedCol).fieldIndex());
+
+ Mappings.TargetMapping mapping = Commons.mapping(retainedCols.build(), colsCnt);
+
+ for (IgniteIndex idx : oldTbl.indexes().values()) {
+ CacheIndexImpl idx0 = (CacheIndexImpl)idx;
+
+ newTbl.addIndex(new CacheIndexImpl(RelCollations.permute(idx0.collation(), mapping), idx0.name(),
+ idx0.queryIndex(), newTbl));
+ }
+
+ publishTable(schemaName, typeDesc.tableName(), newTbl);
+ }
+
+ /** */
+ private IgniteCacheTable createTable(
+ GridQueryTypeDescriptor typeDesc,
+ GridCacheContextInfo<?, ?> cacheInfo
+ ) {
+ CacheTableDescriptorImpl desc =
+ new CacheTableDescriptorImpl(cacheInfo, typeDesc, affinityIdentity(cacheInfo.config()));
+
+ return new CacheTableImpl(ctx, desc);
+ }
+
+ /** */
+ private void publishTable(
+ String schemaName,
+ String tblName,
+ IgniteTable tbl
+ ) {
+ IgniteSchema schema = igniteSchemas.computeIfAbsent(schemaName, IgniteSchema::new);
+
+ schema.addTable(tblName, tbl);
+
+ rebuild();
}
/** */
@@ -217,7 +269,7 @@ public class SchemaHolderImpl extends AbstractService implements SchemaHolder, S
}
/** {@inheritDoc} */
- @Override public synchronized void onSqlTypeDropped(
+ @Override public void onSqlTypeDropped(
String schemaName,
GridQueryTypeDescriptor typeDesc,
boolean destroy
@@ -230,12 +282,13 @@ public class SchemaHolderImpl extends AbstractService implements SchemaHolder, S
}
/** {@inheritDoc} */
- @Override public synchronized void onIndexCreated(String schemaName, String tblName, String idxName,
- IndexDescriptor idxDesc) {
- IgniteSchema schema = igniteSchemas.get(schemaName);
- assert schema != null;
-
- IgniteCacheTable tbl = (IgniteCacheTable)schema.getTable(tblName);
+ @Override public void onIndexCreated(
+ String schemaName,
+ String tblName,
+ String idxName,
+ IndexDescriptor idxDesc
+ ) {
+ IgniteCacheTable tbl = table(schemaName, tblName);
assert tbl != null;
RelCollation idxCollation = deriveSecondaryIndexCollation(idxDesc, tbl);
@@ -269,11 +322,8 @@ public class SchemaHolderImpl extends AbstractService implements SchemaHolder, S
}
/** {@inheritDoc} */
- @Override public synchronized void onIndexDropped(String schemaName, String tblName, String idxName) {
- IgniteSchema schema = igniteSchemas.get(schemaName);
- assert schema != null;
-
- IgniteTable tbl = (IgniteTable)schema.getTable(tblName);
+ @Override public void onIndexDropped(String schemaName, String tblName, String idxName) {
+ IgniteTable tbl = table(schemaName, tblName);
assert tbl != null;
tbl.removeIndex(idxName);
@@ -283,10 +333,7 @@ public class SchemaHolderImpl extends AbstractService implements SchemaHolder, S
/** {@inheritDoc} */
@Override public void onIndexRebuildStarted(String schemaName, String tblName) {
- IgniteSchema schema = igniteSchemas.get(schemaName);
- assert schema != null;
-
- IgniteTable tbl = (IgniteTable)schema.getTable(tblName);
+ IgniteTable tbl = table(schemaName, tblName);
assert tbl != null;
tbl.markIndexRebuildInProgress(true);
@@ -294,10 +341,7 @@ public class SchemaHolderImpl extends AbstractService implements SchemaHolder, S
/** {@inheritDoc} */
@Override public void onIndexRebuildFinished(String schemaName, String tblName) {
- IgniteSchema schema = igniteSchemas.get(schemaName);
- assert schema != null;
-
- IgniteTable tbl = (IgniteTable)schema.getTable(tblName);
+ IgniteTable tbl = table(schemaName, tblName);
assert tbl != null;
tbl.markIndexRebuildInProgress(false);
@@ -328,6 +372,16 @@ public class SchemaHolderImpl extends AbstractService implements SchemaHolder, S
return schema != null ? calciteSchema.getSubSchema(schema) : calciteSchema;
}
+ /** */
+ private IgniteCacheTable table(String schemaName, String tableName) {
+ IgniteSchema schema = igniteSchemas.get(schemaName);
+
+ if (schema != null)
+ return (IgniteCacheTable)schema.getTable(tableName);
+
+ return null;
+ }
+
/** */
private void rebuild() {
SchemaPlus newCalciteSchema = Frameworks.createRootSchema(false);
diff --git a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/schema/SystemViewTableDescriptorImpl.java b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/schema/SystemViewTableDescriptorImpl.java
index 921b96a2910..9fdf58d68af 100644
--- a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/schema/SystemViewTableDescriptorImpl.java
+++ b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/schema/SystemViewTableDescriptorImpl.java
@@ -189,8 +189,8 @@ public class SystemViewTableDescriptorImpl<ViewRow> extends NullInitializerExpre
return fieldName == null ? null : descriptorsMap.get(fieldName);
}
- /** Column descriptors. */
- public Collection<SystemViewColumnDescriptor> columnDescriptors() {
+ /** {@inheritDoc} */
+ @Override public Collection<SystemViewColumnDescriptor> columnDescriptors() {
return Collections.unmodifiableList(Arrays.asList(descriptors));
}
diff --git a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/schema/TableDescriptor.java b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/schema/TableDescriptor.java
index 67afc6fba37..af3b72a382b 100644
--- a/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/schema/TableDescriptor.java
+++ b/modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/schema/TableDescriptor.java
@@ -17,6 +17,7 @@
package org.apache.ignite.internal.processors.query.calcite.schema;
+import java.util.Collection;
import org.apache.calcite.plan.RelOptTable;
import org.apache.calcite.rel.type.RelDataType;
import org.apache.calcite.rel.type.RelDataTypeFactory;
@@ -115,4 +116,11 @@ public interface TableDescriptor<TableRow> extends RelProtoDataType, Initializer
* @return Column descriptor.
*/
ColumnDescriptor columnDescriptor(String fieldName);
+
+ /**
+ * Returns table column descriptors.
+ *
+ * @return Column descriptors.
+ */
+ Collection<? extends ColumnDescriptor> columnDescriptors();
}
diff --git a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/TableDdlIntegrationTest.java b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/TableDdlIntegrationTest.java
index ba47dc5b293..e8932f5dd27 100644
--- a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/TableDdlIntegrationTest.java
+++ b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/TableDdlIntegrationTest.java
@@ -36,6 +36,7 @@ import org.apache.ignite.cache.QueryEntity;
import org.apache.ignite.configuration.CacheConfiguration;
import org.apache.ignite.internal.processors.cache.IgniteInternalCache;
import org.apache.ignite.internal.processors.query.IgniteSQLException;
+import org.apache.ignite.internal.processors.query.calcite.QueryChecker;
import org.apache.ignite.internal.util.typedef.F;
import org.apache.ignite.testframework.GridTestUtils;
import org.hamcrest.CustomMatcher;
@@ -776,6 +777,51 @@ public class TableDdlIntegrationTest extends AbstractDdlIntegrationTest {
assertEquals(2, sql("select * from my_table").size());
}
+ /**
+ * Drop or add column and check indexes works.
+ */
+ @Test
+ public void alterTableChangeColumnAndUseIndex() {
+ sql("create table my_table(c1 int, c2 int, c3 int, c4 int)");
+ sql("create index my_index on my_table(c4)");
+
+ for (int i = 0; i < 100; i++)
+ sql("insert into my_table values (1, 2, 3, ?)", i);
+
+ assertQuery("select * from my_table where c4 = 50")
+ .matches(QueryChecker.containsIndexScan("PUBLIC", "MY_TABLE", "MY_INDEX"))
+ .returns(1, 2, 3, 50)
+ .check();
+
+ sql("alter table my_table add column c5 int");
+
+ assertQuery("select * from my_table where c4 = 50")
+ .matches(QueryChecker.containsIndexScan("PUBLIC", "MY_TABLE", "MY_INDEX"))
+ .returns(1, 2, 3, 50, null)
+ .check();
+
+ sql("alter table my_table drop column c2");
+
+ assertQuery("select * from my_table where c4 = 50")
+ .matches(QueryChecker.containsIndexScan("PUBLIC", "MY_TABLE", "MY_INDEX"))
+ .returns(1, 3, 50, null)
+ .check();
+
+ sql("alter table my_table drop column (c1, c3)");
+
+ assertQuery("select * from my_table where c4 = 50")
+ .matches(QueryChecker.containsIndexScan("PUBLIC", "MY_TABLE", "MY_INDEX"))
+ .returns(50, null)
+ .check();
+
+ sql("alter table my_table drop column (c5)");
+
+ assertQuery("select * from my_table where c4 = 50")
+ .matches(QueryChecker.containsIndexScan("PUBLIC", "MY_TABLE", "MY_INDEX"))
+ .returns(50)
+ .check();
+ }
+
/**
* Alter table logging/nologing.
*/
diff --git a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/planner/AbstractPlannerTest.java b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/planner/AbstractPlannerTest.java
index 618fd7810c0..9a62151ed58 100644
--- a/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/planner/AbstractPlannerTest.java
+++ b/modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/planner/AbstractPlannerTest.java
@@ -749,6 +749,11 @@ public abstract class AbstractPlannerTest extends GridCommonAbstractTest {
return new TestColumnDescriptor(field.getIndex(), fieldName);
}
+ /** {@inheritDoc} */
+ @Override public Collection<ColumnDescriptor> columnDescriptors() {
+ throw new AssertionError();
+ }
+
/** {@inheritDoc} */
@Override public GridQueryTypeDescriptor typeDescription() {
throw new AssertionError();