You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2022/11/01 06:56:18 UTC
[GitHub] [ignite] alex-plekhanov commented on a diff in pull request #10153: IGNITE-17250 : Calcite. Make 'min()/max()' use first/last index value. V2
alex-plekhanov commented on code in PR #10153:
URL: https://github.com/apache/ignite/pull/10153#discussion_r1010106892
##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/AggregatesIntegrationTest.java:
##########
@@ -21,28 +21,91 @@
import org.apache.ignite.IgniteCache;
import org.apache.ignite.IgniteCheckedException;
import org.apache.ignite.cache.CacheMode;
+import org.apache.ignite.cache.QueryEntity;
+import org.apache.ignite.cache.query.annotations.QuerySqlField;
+import org.apache.ignite.configuration.CacheConfiguration;
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.junit.Test;
/**
*
*/
public class AggregatesIntegrationTest extends AbstractBasicIntegrationTest {
+ /** */
+ @Test
+ public void testMinMaxWithTable() {
+ String[] indexes = new String[] {
+ "val0",
+ "val0 desc",
+ "val0, val1",
+ "val0 desc, val1 desc",
+ "val0 asc, val1 desc",
+ "val0 desc, val1 asc"
+ };
+
+ for (String idx : indexes) {
+ for (int backups = -1; backups < 3; ++backups) {
+ executeSql("create table tbl(id integer primary key, val0 integer, val1 float, val2 varchar) " +
+ "with template=" + (backups < 0 ? "replicated" : "partitioned,backups=" + backups));
+
+ executeSql("create index test_idx on tbl(" + idx + ")");
+
+ fillTestTbl();
+
+ assertQuery("select min(val0) from tbl").returns(1).check();
+ assertQuery("select min(val1) from tbl").returns(10.0f).check();
+ assertQuery("select max(val0) from tbl").returns(5).check();
+ assertQuery("select max(val1) from tbl").returns(50.0f).check();
+
+ executeSql("drop table tbl");
+ }
+ }
+ }
+
+ /** */
+ private void fillTestTbl() {
+ executeSql("insert into tbl values(-1, null, null, 'value_-1')");
+ executeSql("insert into tbl values(1, 2, 20.0, 'value_1')");
+ executeSql("insert into tbl values(2, 3, 10.0, 'value_2')");
+// executeSql("insert into tbl values(3, null, 30.0, null)");
Review Comment:
Remove commented lines
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/IndexFirstLastScan.java:
##########
@@ -0,0 +1,127 @@
+/*
+ * 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.processors.query.calcite.exec;
+
+import java.util.List;
+import org.apache.calcite.util.ImmutableBitSet;
+import org.apache.calcite.util.ImmutableIntList;
+import org.apache.ignite.IgniteCheckedException;
+import org.apache.ignite.IgniteException;
+import org.apache.ignite.internal.cache.query.index.sorted.IndexKeyType;
+import org.apache.ignite.internal.cache.query.index.sorted.IndexRow;
+import org.apache.ignite.internal.cache.query.index.sorted.inline.IndexQueryContext;
+import org.apache.ignite.internal.cache.query.index.sorted.inline.InlineIndexImpl;
+import org.apache.ignite.internal.cache.query.index.sorted.inline.InlineIndexKeyType;
+import org.apache.ignite.internal.cache.query.index.sorted.inline.io.InlineIO;
+import org.apache.ignite.internal.cache.query.index.sorted.keys.IndexKey;
+import org.apache.ignite.internal.cache.query.index.sorted.keys.NullIndexKey;
+import org.apache.ignite.internal.processors.cache.persistence.tree.BPlusTree;
+import org.apache.ignite.internal.processors.cache.persistence.tree.io.BPlusIO;
+import org.apache.ignite.internal.processors.query.calcite.schema.CacheTableDescriptor;
+import org.apache.ignite.internal.util.lang.GridCursor;
+import org.apache.ignite.internal.util.typedef.F;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Takes only first or last index value excluding nulls.
+ */
+public class IndexFirstLastScan<Row> extends IndexScan<Row> {
+ /**
+ * @param first {@code True} to take first index value. {@code False} to take last value.
+ * @param ectx Execution context.
+ * @param desc Table descriptor.
+ * @param idx Phisycal index.
+ * @param idxFieldMapping Mapping from index keys to row fields.
+ * @param parts Mapping from index keys to row fields.
+ * @param requiredColumns Required columns.
+ */
+ public IndexFirstLastScan(
+ boolean first,
+ ExecutionContext<Row> ectx,
+ CacheTableDescriptor desc,
+ InlineIndexImpl idx,
+ ImmutableIntList idxFieldMapping,
+ int[] parts,
+ @Nullable ImmutableBitSet requiredColumns
+ ) {
+ super(ectx, desc, new FirstLastIndexWrapper(idx, first), idxFieldMapping, parts, null, null, null,
+ requiredColumns);
+ }
+
+ /** {@inheritDoc} */
+ @Override protected IndexQueryContext indexQueryContext() {
+ IndexQueryContext res = super.indexQueryContext();
+
+ BPlusTree.TreeRowClosure<IndexRow, IndexRow> f = res.rowFilter();
+
+ List<InlineIndexKeyType> inlineKeyTypes = idx.segment(0).rowHandler().inlineIndexKeyTypes();
+
+ InlineIndexKeyType keyType = F.isEmpty(inlineKeyTypes) ? null : inlineKeyTypes.get(0);
+
+ return new IndexQueryContext(res.cacheFilter(), new BPlusTree.TreeRowClosure<IndexRow, IndexRow>() {
+ /** {@inheritDoc} */
+ @Override public boolean apply(BPlusTree<IndexRow, IndexRow> tree, BPlusIO<IndexRow> io, long pageAddr,
Review Comment:
Codestyle: each parameter should be on it's own line.
##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/planner/IndexRebuildPlannerTest.java:
##########
@@ -92,6 +95,39 @@ public void testIndexCountAtIndexRebuild() throws Exception {
.and(nodeOrAnyChild(isInstanceOf(IgniteIndexCount.class)))));
}
+ /** Test Index min/max (first/last) is disabled when index is unavailable. */
+ @Test
+ public void testIndexMinMaxAtIndexRebuild() throws Exception {
+ checkMinMaxOptimized();
+
+ tbl.markIndexRebuildInProgress(true);
+
+ assertPlan("SELECT MIN(VAL) FROM TBL", publicSchema, isInstanceOf(IgniteAggregate.class)
+ .and(a -> a.getAggCallList().stream().filter(agg -> agg.getAggregation().getKind() == MIN).count() == 1)
+ .and(nodeOrAnyChild(isInstanceOf(IgniteTableScan.class))));
Review Comment:
It's better to use `hasChildThat` than `nodeOrAnyChild` here (current node can't be `IgniteTableScan`, since it is `IgniteAggregate`)
THe same for `checkMinMaxOptimized` method.
`isInstanceOf(IgniteTableScan.class)` can also be optimized to `isTableScan(tbl)`
##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/planner/HashAggregatePlannerTest.java:
##########
@@ -43,25 +47,85 @@
import org.junit.Assert;
import org.junit.Test;
+import static org.apache.calcite.rel.RelFieldCollation.Direction.ASCENDING;
+import static org.apache.calcite.rel.RelFieldCollation.Direction.DESCENDING;
+
/**
*
*/
@SuppressWarnings({"TooBroadScope", "FieldCanBeLocal", "TypeMayBeWeakened"})
public class HashAggregatePlannerTest extends AbstractAggregatePlannerTest {
+ /** */
+ @Test
+ public void indexMinMax() throws Exception {
+ IgniteSchema publicSchema = new IgniteSchema("PUBLIC");
+
+ RelCollation[] collations = new RelCollation[] {
+ RelCollations.of(new RelFieldCollation(1, ASCENDING)),
+ RelCollations.of(new RelFieldCollation(1, DESCENDING)),
+ RelCollations.of(new RelFieldCollation(1, ASCENDING), new RelFieldCollation(2, ASCENDING)),
+ RelCollations.of(new RelFieldCollation(1, DESCENDING), new RelFieldCollation(2, DESCENDING)),
+ RelCollations.of(new RelFieldCollation(1, ASCENDING), new RelFieldCollation(2, DESCENDING)),
+ RelCollations.of(new RelFieldCollation(1, DESCENDING), new RelFieldCollation(2, ASCENDING))
+ };
+
+ for (RelCollation cll : collations) {
+ for (IgniteDistribution distr : distributions()) {
+ TestTable tbl = createTable(distr);
+ publicSchema.addTable("TEST", tbl);
+
+ assertNoIndexFirstOrLastRecord("SELECT MIN(VAL0) FROM TEST", publicSchema);
+ assertNoIndexFirstOrLastRecord("SELECT MIN(VAL0) FROM TEST", publicSchema);
+
+ tbl.addIndex(cll, "TEST_IDX");
+
+ boolean targetFldIdxAcc = !cll.getFieldCollations().get(0).direction.isDescending();
+
+ assertIndexFirstOrLastRecord("SELECT MIN(VAL0) FROM TEST", targetFldIdxAcc, publicSchema);
+ assertIndexFirstOrLastRecord("SELECT MAX(VAL0) FROM TEST", !targetFldIdxAcc, publicSchema);
+ assertIndexFirstOrLastRecord("SELECT MIN(V) FROM (SELECT MIN(VAL0) AS V FROM TEST)",
+ targetFldIdxAcc, publicSchema);
+ assertIndexFirstOrLastRecord("SELECT MAX(V) FROM (SELECT MAX(VAL0) AS V FROM TEST)",
+ !targetFldIdxAcc, publicSchema);
+ assertIndexFirstOrLastRecord("SELECT MAX(VAL0) FROM TEST", !targetFldIdxAcc, publicSchema);
+
+ assertNoIndexFirstOrLastRecord("SELECT MIN(VAL0) FROM TEST GROUP BY GRP0", publicSchema);
+ assertNoIndexFirstOrLastRecord("SELECT MIN(VAL0) FROM TEST GROUP BY GRP0, GRP1 ORDER BY GRP1 DESC",
+ publicSchema);
+
+ assertNoIndexFirstOrLastRecord("SELECT MIN(VAL1) FROM TEST", publicSchema);
+ assertNoIndexFirstOrLastRecord("SELECT MAX(VAL1) FROM TEST", publicSchema);
+
+ assertNoIndexFirstOrLastRecord("SELECT MIN(VAL0) FROM TEST WHERE VAL1 > 1", publicSchema);
+ assertNoIndexFirstOrLastRecord("SELECT MAX(VAL0) FROM TEST WHERE VAL1 > 1", publicSchema);
+ assertNoIndexFirstOrLastRecord("SELECT VAL1, MIN(VAL0) FROM TEST GROUP BY VAL1", publicSchema);
+ assertNoIndexFirstOrLastRecord("SELECT VAL1, MAX(VAL0) FROM TEST GROUP BY VAL1", publicSchema);
+
+ publicSchema.removeTable("TEST");
+ }
+ }
+ }
+
+ /** */
+ private void assertIndexFirstOrLastRecord(String sql, boolean first, IgniteSchema schema) throws Exception {
+ assertPlan(sql, schema, nodeOrAnyChild(isInstanceOf(IgniteAggregate.class)
+ .and(nodeOrAnyChild(isInstanceOf(IgniteIndexBound.class)
+ .and(s -> first && s.first() || !first && !s.first())))));
+ }
+
+ /** */
+ private void assertNoIndexFirstOrLastRecord(String sql, IgniteSchema publicSchema) throws Exception {
+ assertPlan(sql, publicSchema, hasChildThat(isInstanceOf(IgniteIndexScan.class)).negate());
Review Comment:
`IgniteIndexScan` -> `IgniteIndexBound`
##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/planner/IndexRebuildPlannerTest.java:
##########
@@ -92,6 +95,39 @@ public void testIndexCountAtIndexRebuild() throws Exception {
.and(nodeOrAnyChild(isInstanceOf(IgniteIndexCount.class)))));
}
+ /** Test Index min/max (first/last) is disabled when index is unavailable. */
+ @Test
+ public void testIndexMinMaxAtIndexRebuild() throws Exception {
+ checkMinMaxOptimized();
+
+ tbl.markIndexRebuildInProgress(true);
+
+ assertPlan("SELECT MIN(VAL) FROM TBL", publicSchema, isInstanceOf(IgniteAggregate.class)
+ .and(a -> a.getAggCallList().stream().filter(agg -> agg.getAggregation().getKind() == MIN).count() == 1)
+ .and(nodeOrAnyChild(isInstanceOf(IgniteTableScan.class))));
+
+ assertPlan("SELECT MAX(VAL) FROM TBL", publicSchema, isInstanceOf(IgniteAggregate.class)
+ .and(a -> a.getAggCallList().stream().filter(agg -> agg.getAggregation().getKind() == MAX).count() == 1)
+ .and(nodeOrAnyChild(isInstanceOf(IgniteTableScan.class))));
+
+ tbl.markIndexRebuildInProgress(false);
+
+ checkMinMaxOptimized();
+ }
+
+ /** */
+ private void checkMinMaxOptimized() throws Exception {
+ assertPlan("SELECT MIN(VAL) FROM TBL", publicSchema, nodeOrAnyChild(isInstanceOf(IgniteAggregate.class)
Review Comment:
Perhaps `nodeOrAnyChild` is redundant, since there is only `isInstanceOf` in case index rebuilded.
##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/planner/HashAggregatePlannerTest.java:
##########
@@ -43,25 +47,85 @@
import org.junit.Assert;
import org.junit.Test;
+import static org.apache.calcite.rel.RelFieldCollation.Direction.ASCENDING;
+import static org.apache.calcite.rel.RelFieldCollation.Direction.DESCENDING;
+
/**
*
*/
@SuppressWarnings({"TooBroadScope", "FieldCanBeLocal", "TypeMayBeWeakened"})
public class HashAggregatePlannerTest extends AbstractAggregatePlannerTest {
+ /** */
+ @Test
+ public void indexMinMax() throws Exception {
+ IgniteSchema publicSchema = new IgniteSchema("PUBLIC");
+
+ RelCollation[] collations = new RelCollation[] {
+ RelCollations.of(new RelFieldCollation(1, ASCENDING)),
+ RelCollations.of(new RelFieldCollation(1, DESCENDING)),
+ RelCollations.of(new RelFieldCollation(1, ASCENDING), new RelFieldCollation(2, ASCENDING)),
+ RelCollations.of(new RelFieldCollation(1, DESCENDING), new RelFieldCollation(2, DESCENDING)),
+ RelCollations.of(new RelFieldCollation(1, ASCENDING), new RelFieldCollation(2, DESCENDING)),
+ RelCollations.of(new RelFieldCollation(1, DESCENDING), new RelFieldCollation(2, ASCENDING))
+ };
+
+ for (RelCollation cll : collations) {
+ for (IgniteDistribution distr : distributions()) {
+ TestTable tbl = createTable(distr);
+ publicSchema.addTable("TEST", tbl);
+
+ assertNoIndexFirstOrLastRecord("SELECT MIN(VAL0) FROM TEST", publicSchema);
+ assertNoIndexFirstOrLastRecord("SELECT MIN(VAL0) FROM TEST", publicSchema);
+
+ tbl.addIndex(cll, "TEST_IDX");
+
+ boolean targetFldIdxAcc = !cll.getFieldCollations().get(0).direction.isDescending();
+
+ assertIndexFirstOrLastRecord("SELECT MIN(VAL0) FROM TEST", targetFldIdxAcc, publicSchema);
+ assertIndexFirstOrLastRecord("SELECT MAX(VAL0) FROM TEST", !targetFldIdxAcc, publicSchema);
+ assertIndexFirstOrLastRecord("SELECT MIN(V) FROM (SELECT MIN(VAL0) AS V FROM TEST)",
+ targetFldIdxAcc, publicSchema);
+ assertIndexFirstOrLastRecord("SELECT MAX(V) FROM (SELECT MAX(VAL0) AS V FROM TEST)",
+ !targetFldIdxAcc, publicSchema);
+ assertIndexFirstOrLastRecord("SELECT MAX(VAL0) FROM TEST", !targetFldIdxAcc, publicSchema);
+
+ assertNoIndexFirstOrLastRecord("SELECT MIN(VAL0) FROM TEST GROUP BY GRP0", publicSchema);
+ assertNoIndexFirstOrLastRecord("SELECT MIN(VAL0) FROM TEST GROUP BY GRP0, GRP1 ORDER BY GRP1 DESC",
+ publicSchema);
+
+ assertNoIndexFirstOrLastRecord("SELECT MIN(VAL1) FROM TEST", publicSchema);
+ assertNoIndexFirstOrLastRecord("SELECT MAX(VAL1) FROM TEST", publicSchema);
+
+ assertNoIndexFirstOrLastRecord("SELECT MIN(VAL0) FROM TEST WHERE VAL1 > 1", publicSchema);
+ assertNoIndexFirstOrLastRecord("SELECT MAX(VAL0) FROM TEST WHERE VAL1 > 1", publicSchema);
+ assertNoIndexFirstOrLastRecord("SELECT VAL1, MIN(VAL0) FROM TEST GROUP BY VAL1", publicSchema);
+ assertNoIndexFirstOrLastRecord("SELECT VAL1, MAX(VAL0) FROM TEST GROUP BY VAL1", publicSchema);
Review Comment:
Add test for `assertNoIndexFirstOrLastRecord("SELECT MIN(VAL0 + 1) FROM TEST", publicSchema);` to check aggregate with projects is not converted to first/last scan
##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/planner/HashAggregatePlannerTest.java:
##########
@@ -43,25 +47,85 @@
import org.junit.Assert;
import org.junit.Test;
+import static org.apache.calcite.rel.RelFieldCollation.Direction.ASCENDING;
+import static org.apache.calcite.rel.RelFieldCollation.Direction.DESCENDING;
+
/**
*
*/
@SuppressWarnings({"TooBroadScope", "FieldCanBeLocal", "TypeMayBeWeakened"})
public class HashAggregatePlannerTest extends AbstractAggregatePlannerTest {
+ /** */
+ @Test
+ public void indexMinMax() throws Exception {
+ IgniteSchema publicSchema = new IgniteSchema("PUBLIC");
+
+ RelCollation[] collations = new RelCollation[] {
+ RelCollations.of(new RelFieldCollation(1, ASCENDING)),
Review Comment:
Use `TraitUtils.createFieldCollation` method instead of direct `RelFieldCollation` constructor, default nulls ordering for RelFieldCollation is not the same as Ignite's default.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org