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/07/11 13:18:39 UTC
[GitHub] [ignite] Vladsz83 opened a new pull request, #10153: Calcite. Make 'min()/max()' use first/last index value .
Vladsz83 opened a new pull request, #10153:
URL: https://github.com/apache/ignite/pull/10153
Thank you for submitting the pull request to the Apache Ignite.
In order to streamline the review of the contribution
we ask you to ensure the following steps have been taken:
### The Contribution Checklist
- [ ] There is a single JIRA ticket related to the pull request.
- [ ] The web-link to the pull request is attached to the JIRA ticket.
- [ ] The JIRA ticket has the _Patch Available_ state.
- [ ] The pull request body describes changes that have been made.
The description explains _WHAT_ and _WHY_ was made instead of _HOW_.
- [ ] The pull request title is treated as the final commit message.
The following pattern must be used: `IGNITE-XXXX Change summary` where `XXXX` - number of JIRA issue.
- [ ] A reviewer has been mentioned through the JIRA comments
(see [the Maintainers list](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute#HowtoContribute-ReviewProcessandMaintainers))
- [ ] The pull request has been checked by the Teamcity Bot and
the `green visa` attached to the JIRA ticket (see [TC.Bot: Check PR](https://mtcga.gridgain.com/prs.html))
### Notes
- [How to Contribute](https://cwiki.apache.org/confluence/display/IGNITE/How+to+Contribute)
- [Coding abbreviation rules](https://cwiki.apache.org/confluence/display/IGNITE/Abbreviation+Rules)
- [Coding Guidelines](https://cwiki.apache.org/confluence/display/IGNITE/Coding+Guidelines)
- [Apache Ignite Teamcity Bot](https://cwiki.apache.org/confluence/display/IGNITE/Apache+Ignite+Teamcity+Bot)
If you need any help, please email dev@ignite.apache.org or ask anу advice on http://asf.slack.com _#ignite_ channel.
--
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
[GitHub] [ignite] Vladsz83 commented on a diff in pull request #10153: IGNITE-17250 : Calcite. Make 'min()/max()' use first/last index value. V2
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on code in PR #10153:
URL: https://github.com/apache/ignite/pull/10153#discussion_r1009518493
##########
modules/core/src/main/java/org/apache/ignite/internal/cache/query/index/sorted/inline/InlineIndexImpl.java:
##########
@@ -639,5 +658,48 @@ private static class SegmentedIndexCursor implements GridCursor<IndexRow> {
@Override public IndexRow get() throws IgniteCheckedException {
return head;
}
+
+ /** */
+ protected Queue<GridCursor<IndexRow>> cursorsQueue(GridCursor<IndexRow>[] cursors)
+ throws IgniteCheckedException {
+ PriorityQueue<GridCursor<IndexRow>> q = new PriorityQueue<>(cursors.length, cursorComp);
+
+ for (GridCursor<IndexRow> c: cursors) {
+ if (c.next())
+ q.add(c);
+ }
+
+ return q;
+ }
+ }
+
+ /** First-only, single-value-only segmented cursor. */
+ private static class FirstSegmentSingleValueIndexCursor extends SegmentedIndexCursor {
+ /**
+ * @param cursors
+ * @param idxDef
+ */
+ FirstSegmentSingleValueIndexCursor(
Review Comment:
Fixed
--
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
[GitHub] [ignite] Vladsz83 commented on a diff in pull request #10153: IGNITE-17250 : Calcite. Make 'min()/max()' use first/last index value. V2
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on code in PR #10153:
URL: https://github.com/apache/ignite/pull/10153#discussion_r1009439389
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/IndexFirstLastScan.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * 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 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.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.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,
Review Comment:
Fixed
--
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
[GitHub] [ignite] Vladsz83 commented on a diff in pull request #10153: IGNITE-17250 : Calcite. Make 'min()/max()' use first/last index value. V2
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on code in PR #10153:
URL: https://github.com/apache/ignite/pull/10153#discussion_r1009520054
##########
modules/core/src/main/java/org/apache/ignite/internal/cache/query/index/sorted/inline/InlineIndexImpl.java:
##########
@@ -639,5 +658,48 @@ private static class SegmentedIndexCursor implements GridCursor<IndexRow> {
@Override public IndexRow get() throws IgniteCheckedException {
return head;
}
+
+ /** */
+ protected Queue<GridCursor<IndexRow>> cursorsQueue(GridCursor<IndexRow>[] cursors)
+ throws IgniteCheckedException {
+ PriorityQueue<GridCursor<IndexRow>> q = new PriorityQueue<>(cursors.length, cursorComp);
+
+ for (GridCursor<IndexRow> c: cursors) {
+ if (c.next())
+ q.add(c);
+ }
+
+ return q;
+ }
+ }
+
+ /** First-only, single-value-only segmented cursor. */
+ private static class FirstSegmentSingleValueIndexCursor extends SegmentedIndexCursor {
+ /**
+ * @param cursors
+ * @param idxDef
Review Comment:
Fixed
--
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
[GitHub] [ignite] Vladsz83 commented on a diff in pull request #10153: IGNITE-17250 : Calcite. Make 'min()/max()' use first/last index value. V2
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on code in PR #10153:
URL: https://github.com/apache/ignite/pull/10153#discussion_r1009438935
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/LogicalRelImplementor.java:
##########
@@ -419,6 +422,57 @@ public LogicalRelImplementor(
}
}
+ /** {@inheritDoc} */
+ @Override public Node<Row> visit(IgniteIndexBound idxBndRel) {
+ IgniteTable tbl = idxBndRel.getTable().unwrap(IgniteTable.class);
+ IgniteIndex idx = tbl.getIndex(idxBndRel.indexName());
+ IgniteTypeFactory typeFactory = ctx.getTypeFactory();
+ ColocationGroup grp = ctx.group(idxBndRel.sourceId());
+ ImmutableBitSet requiredColumns = idxBndRel.requiredColumns();
+ RelDataType rowType = tbl.getRowType(typeFactory, requiredColumns);
+
+ if (idx != null && !tbl.isIndexRebuildInProgress())
+ return new ScanNode<>(ctx, rowType, idx.firstOrLast(idxBndRel.first(), ctx, grp, requiredColumns));
+ else {
+ assert requiredColumns.asList().size() == 1;
+
+ RexBuilder b = new RexBuilder(typeFactory);
+
+ Iterable<Row> rowsIter = tbl.scan(
+ ctx,
+ grp,
+ expressionFactory.predicate(b.makeCall(SqlStdOperatorTable.IS_NOT_NULL,
+ b.makeLocalRef(rowType.getFieldList().get(0).getType(), 0)), rowType),
Review Comment:
Fixed
--
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
[GitHub] [ignite] Vladsz83 commented on a diff in pull request #10153: IGNITE-17250 : Calcite. Make 'min()/max()' use first/last index value. V2
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on code in PR #10153:
URL: https://github.com/apache/ignite/pull/10153#discussion_r1009522414
##########
modules/core/src/main/java/org/apache/ignite/internal/cache/query/index/sorted/inline/InlineIndexImpl.java:
##########
@@ -121,6 +124,27 @@ public InlineIndexImpl(GridCacheContext<?, ?> cctx, SortedIndexDefinition def, I
}
}
+ /**
+ * Takes only one not-null-valued first or last index record.
Review Comment:
Yep. Good notice.
--
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
[GitHub] [ignite] Vladsz83 commented on a diff in pull request #10153: IGNITE-17250 : Calcite. Make 'min()/max()' use first/last index value. V2
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on code in PR #10153:
URL: https://github.com/apache/ignite/pull/10153#discussion_r1009439738
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/IndexFirstLastScan.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * 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 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.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.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) {
Review Comment:
Fixed
--
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
[GitHub] [ignite] Vladsz83 commented on a diff in pull request #10153: Calcite. Make 'min()/max()' use first/last index value. V2
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on code in PR #10153:
URL: https://github.com/apache/ignite/pull/10153#discussion_r1000490014
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/IndexScan.java:
##########
@@ -159,6 +162,44 @@ public IndexScan(
}
}
+ /**
+ * Gets first or last records from all segments.
Review Comment:
Refactored except SingleValueSegmentedIndexCursor. Should we? _InlineIndexImpl.findFirst()/.findLast()_ gives _SingleCursor_.
--
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
[GitHub] [ignite] Vladsz83 commented on a diff in pull request #10153: IGNITE-17250 : Calcite. Make 'min()/max()' use first/last index value. V2
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on code in PR #10153:
URL: https://github.com/apache/ignite/pull/10153#discussion_r1009517496
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/IndexFirstLastScan.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * 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 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.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.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();
+
+ return new IndexQueryContext(res.cacheFilter(), new BPlusTree.TreeRowClosure<IndexRow, IndexRow>() {
+ /**
+ * {@inheritDoc}
+ */
+ @Override public boolean apply(BPlusTree<IndexRow, IndexRow> tree, BPlusIO<IndexRow> io, long pageAddr,
+ int idx) throws IgniteCheckedException {
+ if (f != null && !f.apply(tree, io, pageAddr, idx))
+ return false;
+
+ return io.getLookupRow(tree, pageAddr, idx).key(0).type() != IndexKeyType.NULL;
+ }
+ }, res.mvccSnapshot());
+ }
+
+ /** */
+ private static class FirstLastIndexWrapper extends IndexScan.TreeIndexWrapper {
+ /** */
+ private final boolean first;
+
+ /**
+ * @param idx Index
+ * @param first {@code True} to take first index value. {@code False} to take last value.
+ */
+ protected FirstLastIndexWrapper(InlineIndexImpl idx, boolean first) {
+ super(idx);
+ this.first = first;
+ }
+
+ /** {@inheritDoc} */
+ @Override public GridCursor<IndexRow> find(IndexRow lower, IndexRow upper, boolean lowerInclude,
Review Comment:
Fixed
--
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
[GitHub] [ignite] Vladsz83 commented on a diff in pull request #10153: Calcite. Make 'min()/max()' use first/last index value. V2
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on code in PR #10153:
URL: https://github.com/apache/ignite/pull/10153#discussion_r994233092
##########
modules/core/src/main/java/org/apache/ignite/internal/util/lang/GridFunc.java:
##########
@@ -1186,6 +1187,42 @@ public static <T> boolean isEmptyOrNulls(@Nullable T[] c) {
return true;
}
+ /**
+ * Tests if the given object is null, is empty array, empty map or contains only nulls.
+ *
+ * @param o Object to test.
+ * @return {@code True}, is given object is null, is empty array, empty map or contains only nulls.
+ * {@code False} otherwise.
+ */
+ public static boolean isEmptyOrNulls(@Nullable Object o) {
Review Comment:
Fixed with a not-implemented exception.
--
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
[GitHub] [ignite] Vladsz83 commented on a diff in pull request #10153: Calcite. Make 'min()/max()' use first/last index value. V2
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on code in PR #10153:
URL: https://github.com/apache/ignite/pull/10153#discussion_r994246812
##########
modules/core/src/main/java/org/apache/ignite/cache/QueryEntity.java:
##########
@@ -780,7 +780,7 @@ private static void processAnnotationsInClass(boolean key, Class<?> cls, QueryEn
@Nullable QueryEntityClassProperty parent) {
if (U.isJdk(cls) || QueryUtils.isGeometryClass(cls)) {
if (parent == null && !key && QueryUtils.isSqlType(cls)) { // We have to index primitive _val.
- String idxName = cls.getSimpleName() + "_" + QueryUtils.VAL_FIELD_NAME + "_idx";
+ String idxName = QueryUtils.indexName(cls.getSimpleName(), QueryUtils.VAL_FIELD_NAME);
Review Comment:
Fixed
--
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
[GitHub] [ignite] Vladsz83 commented on a diff in pull request #10153: IGNITE-17250 : Calcite. Make 'min()/max()' use first/last index value. V2
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on code in PR #10153:
URL: https://github.com/apache/ignite/pull/10153#discussion_r1009519465
##########
modules/core/src/main/java/org/apache/ignite/internal/cache/query/index/sorted/inline/InlineIndexImpl.java:
##########
@@ -639,5 +658,48 @@ private static class SegmentedIndexCursor implements GridCursor<IndexRow> {
@Override public IndexRow get() throws IgniteCheckedException {
return head;
}
+
+ /** */
+ protected Queue<GridCursor<IndexRow>> cursorsQueue(GridCursor<IndexRow>[] cursors)
+ throws IgniteCheckedException {
+ PriorityQueue<GridCursor<IndexRow>> q = new PriorityQueue<>(cursors.length, cursorComp);
+
+ for (GridCursor<IndexRow> c: cursors) {
+ if (c.next())
+ q.add(c);
+ }
+
+ return q;
+ }
+ }
+
+ /** First-only, single-value-only segmented cursor. */
+ private static class FirstSegmentSingleValueIndexCursor extends SegmentedIndexCursor {
+ /**
+ * @param cursors
+ * @param idxDef
+ */
+ FirstSegmentSingleValueIndexCursor(
+ GridCursor<IndexRow>[] cursors, SortedIndexDefinition idxDef) throws IgniteCheckedException {
+ super(cursors, idxDef);
+ }
+
+ /** {@inheritDoc} */
+ @Override protected Queue<GridCursor<IndexRow>> cursorsQueue(
Review Comment:
One param. 'throw' fixed
--
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
[GitHub] [ignite] alex-plekhanov commented on a diff in pull request #10153: IGNITE-17250 : Calcite. Make 'min()/max()' use first/last index value. V2
Posted by GitBox <gi...@apache.org>.
alex-plekhanov commented on code in PR #10153:
URL: https://github.com/apache/ignite/pull/10153#discussion_r1019069792
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/schema/IgniteIndex.java:
##########
@@ -104,6 +104,11 @@ public <Row> Iterable<Row> scan(
* @param requiredColumns Required columns.
* @return Index records for {@code grp}.
*/
- public <Row> Iterable<Row> firstOrLast(boolean first, ExecutionContext<Row> ectx, ColocationGroup grp,
- @Nullable ImmutableBitSet requiredColumns);
+ public <Row> Iterable<Row> firstOrLast(
+ boolean first,
+ ExecutionContext<Row> ectx,
+ ColocationGroup grp,
+ @Nullable ImmutableBitSet
+ requiredColumns
Review Comment:
Redundant NL
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/LogicalRelImplementor.java:
##########
@@ -419,6 +421,56 @@ public LogicalRelImplementor(
}
}
+ /** {@inheritDoc} */
+ @Override public Node<Row> visit(IgniteIndexBound idxBndRel) {
+ IgniteTable tbl = idxBndRel.getTable().unwrap(IgniteTable.class);
+ IgniteIndex idx = tbl.getIndex(idxBndRel.indexName());
+ IgniteTypeFactory typeFactory = ctx.getTypeFactory();
+ ColocationGroup grp = ctx.group(idxBndRel.sourceId());
+ ImmutableBitSet requiredColumns = idxBndRel.requiredColumns();
+ RelDataType rowType = tbl.getRowType(typeFactory, requiredColumns);
+
+ if (idx != null && !tbl.isIndexRebuildInProgress())
+ return new ScanNode<>(ctx, rowType, idx.firstOrLast(idxBndRel.first(), ctx, grp, requiredColumns));
+ else {
+ assert requiredColumns.asList().size() == 1;
Review Comment:
`requiredColumns.cardinality()`
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/LogicalRelImplementor.java:
##########
@@ -419,6 +421,56 @@ public LogicalRelImplementor(
}
}
+ /** {@inheritDoc} */
+ @Override public Node<Row> visit(IgniteIndexBound idxBndRel) {
+ IgniteTable tbl = idxBndRel.getTable().unwrap(IgniteTable.class);
+ IgniteIndex idx = tbl.getIndex(idxBndRel.indexName());
+ IgniteTypeFactory typeFactory = ctx.getTypeFactory();
+ ColocationGroup grp = ctx.group(idxBndRel.sourceId());
+ ImmutableBitSet requiredColumns = idxBndRel.requiredColumns();
+ RelDataType rowType = tbl.getRowType(typeFactory, requiredColumns);
+
+ if (idx != null && !tbl.isIndexRebuildInProgress())
+ return new ScanNode<>(ctx, rowType, idx.firstOrLast(idxBndRel.first(), ctx, grp, requiredColumns));
+ else {
+ assert requiredColumns.asList().size() == 1;
+
+ RexBuilder b = new RexBuilder(typeFactory);
Review Comment:
Redundant
##########
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:
Still not fixed
##########
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/AbstractBasicIntegrationTest.java:
##########
@@ -250,6 +250,12 @@ public DelegatingIgniteIndex(IgniteIndex delegate) {
@Override public long count(ExecutionContext<?> ectx, ColocationGroup grp) {
return delegate.count(ectx, grp);
}
+
+ /** {@inheritDoc} */
+ @Override public <Row> Iterable<Row> firstOrLast(boolean first, ExecutionContext<Row> ectx, ColocationGroup grp,
Review Comment:
Checkstyle
--
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
[GitHub] [ignite] Vladsz83 commented on a diff in pull request #10153: Calcite. Make 'min()/max()' use first/last index value. V2
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on code in PR #10153:
URL: https://github.com/apache/ignite/pull/10153#discussion_r994269116
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/LogicalRelImplementor.java:
##########
@@ -428,6 +429,44 @@ public LogicalRelImplementor(
}
}
+ /** {@inheritDoc} */
+ @Override public Node<Row> visit(IgniteIndexBound rel) {
+ IgniteTable tbl = rel.getTable().unwrap(IgniteTable.class);
+ IgniteIndex idx = tbl.getIndex(rel.indexName());
+ IgniteTypeFactory typeFactory = ctx.getTypeFactory();
+ ColocationGroup grp = ctx.group(rel.sourceId());
+ ImmutableBitSet requiredColumns = rel.requiredColumns();
+ RelDataType rowType = tbl.getRowType(typeFactory, requiredColumns);
+
+ if (idx != null && !tbl.isIndexRebuildInProgress()) {
+ return new ScanNode<>(ctx, rowType, idx.findFirstOrLast(rel.first(), ctx, ctx.group(rel.sourceId()),
+ requiredColumns));
+ }
+ else {
+ Iterable<Row> rowsIter = tbl.scan(
+ ctx,
+ grp,
+ null,
+ null,
+ rel.requiredColumns()
+ );
+
+ Node<Row> scanNode = new ScanNode<>(ctx, rowType, rowsIter);
+
+ RelCollation collation = idx.collation().apply(LogicalScanConverterRule.createMapping(
+ null,
+ requiredColumns,
+ tbl.getRowType(typeFactory).getFieldCount()
+ ));
+
+ SortNode<Row> sortNode = new SortNode<>(ctx, rowType, expressionFactory.comparator(collation));
Review Comment:
Fixed
--
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
[GitHub] [ignite] Vladsz83 commented on a diff in pull request #10153: IGNITE-17250 : Calcite. Make 'min()/max()' use first/last index value. V2
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on code in PR #10153:
URL: https://github.com/apache/ignite/pull/10153#discussion_r1009658097
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/IndexFirstLastScan.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * 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 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.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.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();
+
+ return new IndexQueryContext(res.cacheFilter(), new BPlusTree.TreeRowClosure<IndexRow, IndexRow>() {
+ /**
+ * {@inheritDoc}
+ */
+ @Override public boolean apply(BPlusTree<IndexRow, IndexRow> tree, BPlusIO<IndexRow> io, long pageAddr,
+ int idx) throws IgniteCheckedException {
+ if (f != null && !f.apply(tree, io, pageAddr, idx))
+ return false;
+
+ return io.getLookupRow(tree, pageAddr, idx).key(0).type() != IndexKeyType.NULL;
+ }
+ }, res.mvccSnapshot());
Review Comment:
Cool! You men 'return key != NullIndexKey.INSTANCE;' I guess
--
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
[GitHub] [ignite] alex-plekhanov commented on a diff in pull request #10153: IGNITE-17250 : Calcite. Make 'min()/max()' use first/last index value. V2
Posted by GitBox <gi...@apache.org>.
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
[GitHub] [ignite] alex-plekhanov commented on a diff in pull request #10153: IGNITE-17250 : Calcite. Make 'min()/max()' use first/last index value. V2
Posted by GitBox <gi...@apache.org>.
alex-plekhanov commented on code in PR #10153:
URL: https://github.com/apache/ignite/pull/10153#discussion_r1007982697
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/IndexFirstLastScan.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * 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 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.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.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,
Review Comment:
NL after `(`
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/IndexFirstLastScan.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * 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 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.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.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) {
Review Comment:
NL before `)`
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/schema/IgniteIndex.java:
##########
@@ -94,4 +94,16 @@ public <Row> Iterable<Row> scan(
* @return Index records number for {@code group}.
*/
public long count(ExecutionContext<?> ectx, ColocationGroup grp);
+
+ /**
+ * Takes only first or last not-null index value.
+ *
+ * @param first {@code True} to take first index not-null value. {@code False} for last.
+ * @param ectx Execution context.
+ * @param grp Colocation group.
+ * @param requiredColumns Required columns.
+ * @return Index records for {@code grp}.
+ */
+ public <Row> Iterable<Row> firstOrLast(boolean first, ExecutionContext<Row> ectx, ColocationGroup grp,
Review Comment:
Codestyle: each parameter should be on it's own line.
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/schema/SystemViewIndexImpl.java:
##########
@@ -101,6 +102,12 @@ public SystemViewIndexImpl(SystemViewTableImpl tbl) {
return tbl.descriptor().systemView().size();
}
+ /** {@inheritDoc} */
+ @Override public <Row> Iterable<Row> firstOrLast(boolean first, ExecutionContext<Row> ectx, ColocationGroup grp,
Review Comment:
Codestyle: each parameter should be on it's own line.
##########
modules/core/src/main/java/org/apache/ignite/internal/cache/query/index/sorted/inline/InlineIndexImpl.java:
##########
@@ -639,5 +658,48 @@ private static class SegmentedIndexCursor implements GridCursor<IndexRow> {
@Override public IndexRow get() throws IgniteCheckedException {
return head;
}
+
+ /** */
+ protected Queue<GridCursor<IndexRow>> cursorsQueue(GridCursor<IndexRow>[] cursors)
+ throws IgniteCheckedException {
+ PriorityQueue<GridCursor<IndexRow>> q = new PriorityQueue<>(cursors.length, cursorComp);
+
+ for (GridCursor<IndexRow> c: cursors) {
+ if (c.next())
+ q.add(c);
+ }
+
+ return q;
+ }
+ }
+
+ /** First-only, single-value-only segmented cursor. */
+ private static class FirstSegmentSingleValueIndexCursor extends SegmentedIndexCursor {
Review Comment:
Why `FirstSegment`? It takes min/max from all segments, not first. I think `SingleValueSegmentedIndexCursor` will be better.
##########
modules/core/src/main/java/org/apache/ignite/internal/cache/query/index/sorted/inline/InlineIndexImpl.java:
##########
@@ -639,5 +658,48 @@ private static class SegmentedIndexCursor implements GridCursor<IndexRow> {
@Override public IndexRow get() throws IgniteCheckedException {
return head;
}
+
+ /** */
+ protected Queue<GridCursor<IndexRow>> cursorsQueue(GridCursor<IndexRow>[] cursors)
+ throws IgniteCheckedException {
+ PriorityQueue<GridCursor<IndexRow>> q = new PriorityQueue<>(cursors.length, cursorComp);
+
+ for (GridCursor<IndexRow> c: cursors) {
+ if (c.next())
+ q.add(c);
+ }
+
+ return q;
+ }
+ }
+
+ /** First-only, single-value-only segmented cursor. */
+ private static class FirstSegmentSingleValueIndexCursor extends SegmentedIndexCursor {
+ /**
+ * @param cursors
+ * @param idxDef
+ */
+ FirstSegmentSingleValueIndexCursor(
+ GridCursor<IndexRow>[] cursors, SortedIndexDefinition idxDef) throws IgniteCheckedException {
+ super(cursors, idxDef);
+ }
+
+ /** {@inheritDoc} */
+ @Override protected Queue<GridCursor<IndexRow>> cursorsQueue(
Review Comment:
Codestyle: each parameter should be on it's own line.
##########
modules/core/src/main/java/org/apache/ignite/internal/cache/query/index/sorted/inline/InlineIndexImpl.java:
##########
@@ -639,5 +658,48 @@ private static class SegmentedIndexCursor implements GridCursor<IndexRow> {
@Override public IndexRow get() throws IgniteCheckedException {
return head;
}
+
+ /** */
+ protected Queue<GridCursor<IndexRow>> cursorsQueue(GridCursor<IndexRow>[] cursors)
+ throws IgniteCheckedException {
+ PriorityQueue<GridCursor<IndexRow>> q = new PriorityQueue<>(cursors.length, cursorComp);
+
+ for (GridCursor<IndexRow> c: cursors) {
+ if (c.next())
+ q.add(c);
+ }
+
+ return q;
+ }
+ }
+
+ /** First-only, single-value-only segmented cursor. */
+ private static class FirstSegmentSingleValueIndexCursor extends SegmentedIndexCursor {
+ /**
+ * @param cursors
+ * @param idxDef
Review Comment:
If there is no description these lines should be removed.
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/IndexFirstLastScan.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * 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 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.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.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();
+
+ return new IndexQueryContext(res.cacheFilter(), new BPlusTree.TreeRowClosure<IndexRow, IndexRow>() {
+ /**
+ * {@inheritDoc}
+ */
+ @Override public boolean apply(BPlusTree<IndexRow, IndexRow> tree, BPlusIO<IndexRow> io, long pageAddr,
+ int idx) throws IgniteCheckedException {
+ if (f != null && !f.apply(tree, io, pageAddr, idx))
+ return false;
+
+ return io.getLookupRow(tree, pageAddr, idx).key(0).type() != IndexKeyType.NULL;
+ }
+ }, res.mvccSnapshot());
+ }
+
+ /** */
+ private static class FirstLastIndexWrapper extends IndexScan.TreeIndexWrapper {
+ /** */
+ private final boolean first;
+
+ /**
+ * @param idx Index
+ * @param first {@code True} to take first index value. {@code False} to take last value.
+ */
+ protected FirstLastIndexWrapper(InlineIndexImpl idx, boolean first) {
+ super(idx);
+ this.first = first;
+ }
+
+ /** {@inheritDoc} */
+ @Override public GridCursor<IndexRow> find(IndexRow lower, IndexRow upper, boolean lowerInclude,
Review Comment:
Codestyle: each parameter should be on it's own line.
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/LogicalRelImplementor.java:
##########
@@ -419,6 +422,57 @@ public LogicalRelImplementor(
}
}
+ /** {@inheritDoc} */
+ @Override public Node<Row> visit(IgniteIndexBound idxBndRel) {
+ IgniteTable tbl = idxBndRel.getTable().unwrap(IgniteTable.class);
+ IgniteIndex idx = tbl.getIndex(idxBndRel.indexName());
+ IgniteTypeFactory typeFactory = ctx.getTypeFactory();
+ ColocationGroup grp = ctx.group(idxBndRel.sourceId());
+ ImmutableBitSet requiredColumns = idxBndRel.requiredColumns();
+ RelDataType rowType = tbl.getRowType(typeFactory, requiredColumns);
+
+ if (idx != null && !tbl.isIndexRebuildInProgress())
+ return new ScanNode<>(ctx, rowType, idx.firstOrLast(idxBndRel.first(), ctx, grp, requiredColumns));
+ else {
+ assert requiredColumns.asList().size() == 1;
+
+ RexBuilder b = new RexBuilder(typeFactory);
+
+ Iterable<Row> rowsIter = tbl.scan(
+ ctx,
+ grp,
+ expressionFactory.predicate(b.makeCall(SqlStdOperatorTable.IS_NOT_NULL,
+ b.makeLocalRef(rowType.getFieldList().get(0).getType(), 0)), rowType),
Review Comment:
```suggestion
r -> ctx.rowHandler().get(0, r) != null,
```
##########
modules/core/src/main/java/org/apache/ignite/internal/cache/query/index/sorted/inline/InlineIndexImpl.java:
##########
@@ -121,6 +124,27 @@ public InlineIndexImpl(GridCacheContext<?, ?> cctx, SortedIndexDefinition def, I
}
}
+ /**
+ * Takes only one not-null-valued first or last index record.
+ */
+ public GridCursor<IndexRow> takeFirstOrLast(IndexQueryContext qryCtx, boolean first) throws IgniteCheckedException {
Review Comment:
`findFirstOrLast` to match other methods?
Perhaps move declaration to `SortedSegmentedIndex`? In this case in `IndexFirstLastScan` idx can be used without casting to `IgniteIndexImpl` (and interface can be used instead of class)
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/schema/CacheIndexImpl.java:
##########
@@ -117,6 +119,19 @@ public Index queryIndex() {
return Collections.emptyList();
}
+ /** {@inheritDoc} */
+ @Override public <Row> Iterable<Row> firstOrLast(boolean first, ExecutionContext<Row> ectx, ColocationGroup grp,
Review Comment:
Codestyle: each parameter should be on it's own line.
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/IndexFirstLastScan.java:
##########
@@ -0,0 +1,105 @@
+/*
+ * 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 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.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.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();
+
+ return new IndexQueryContext(res.cacheFilter(), new BPlusTree.TreeRowClosure<IndexRow, IndexRow>() {
+ /**
+ * {@inheritDoc}
+ */
+ @Override public boolean apply(BPlusTree<IndexRow, IndexRow> tree, BPlusIO<IndexRow> io, long pageAddr,
+ int idx) throws IgniteCheckedException {
+ if (f != null && !f.apply(tree, io, pageAddr, idx))
+ return false;
+
+ return io.getLookupRow(tree, pageAddr, idx).key(0).type() != IndexKeyType.NULL;
+ }
+ }, res.mvccSnapshot());
Review Comment:
`getLookupRow` reads full cache row, it can be not effective, I propose to try to read the inlined value first:
```suggestion
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,
int idx
) throws IgniteCheckedException {
if (f != null && !f.apply(tree, io, pageAddr, idx))
return false;
if (keyType != null && io instanceof InlineIO) {
IndexKey key = keyType.get(pageAddr, io.offset(idx), ((InlineIO)io).inlineSize());
if (key != null)
return key == NullIndexKey.INSTANCE;
};
return io.getLookupRow(tree, pageAddr, idx).key(0).type() != IndexKeyType.NULL;
}
}, res.mvccSnapshot());
```
##########
modules/core/src/main/java/org/apache/ignite/internal/cache/query/index/sorted/inline/InlineIndexImpl.java:
##########
@@ -121,6 +124,27 @@ public InlineIndexImpl(GridCacheContext<?, ?> cctx, SortedIndexDefinition def, I
}
}
+ /**
+ * Takes only one not-null-valued first or last index record.
Review Comment:
`not-null-valued` - it can be null, depends on qryCtx
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/rule/IndexMinMaxRule.java:
##########
@@ -0,0 +1,95 @@
+/*
+ * 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.rule;
+
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.sql.SqlAggFunction;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.ignite.internal.processors.query.calcite.rel.IgniteAggregate;
+import org.apache.ignite.internal.processors.query.calcite.rel.IgniteIndexBound;
+import org.apache.ignite.internal.processors.query.calcite.rel.IgniteIndexScan;
+import org.apache.ignite.internal.processors.query.calcite.schema.IgniteIndex;
+import org.apache.ignite.internal.processors.query.calcite.schema.IgniteTable;
+import org.apache.ignite.internal.processors.query.calcite.trait.RewindabilityTrait;
+import org.apache.ignite.internal.util.typedef.F;
+import org.immutables.value.Value;
+
+/**
+ * Tries to optimize MIN() and MAX() so that it takes only first or last not-null index record.
+ */
+@Value.Enclosing
+public class IndexMinMaxRule extends RelRule<IndexMinMaxRule.Config> {
+ /** */
+ public static final IndexMinMaxRule INSTANCE = Config.DEFAULT.toRule();
+
+ /** Ctor. */
+ private IndexMinMaxRule(IndexMinMaxRule.Config cfg) {
+ super(cfg);
+ }
+
+ /** */
+ @Override public void onMatch(RelOptRuleCall call) {
+ IgniteAggregate aggr = call.rel(0);
+ IgniteIndexScan idxScan = call.rel(1);
+ IgniteTable table = idxScan.getTable().unwrap(IgniteTable.class);
+ IgniteIndex idx = table.getIndex(idxScan.indexName());
+
+ if (
+ table.isIndexRebuildInProgress() ||
+ idxScan.condition() != null ||
+ idxScan.projects() != null ||
+ aggr.getGroupCount() > 0 ||
+ aggr.getAggCallList().stream().filter(a -> a.getAggregation().getKind() == SqlKind.MIN
+ || a.getAggregation().getKind() == SqlKind.MAX).count() != 1 ||
+ idx.collation().getFieldCollations().isEmpty() ||
+ idx.collation().getFieldCollations().get(0).getFieldIndex() != idxScan.requiredColumns().asList().get(0)
Review Comment:
`asList().get(0)` -> `nextSetBit(0)`?
##########
modules/core/src/main/java/org/apache/ignite/internal/cache/query/index/sorted/inline/InlineIndexImpl.java:
##########
@@ -639,5 +658,48 @@ private static class SegmentedIndexCursor implements GridCursor<IndexRow> {
@Override public IndexRow get() throws IgniteCheckedException {
return head;
}
+
+ /** */
+ protected Queue<GridCursor<IndexRow>> cursorsQueue(GridCursor<IndexRow>[] cursors)
+ throws IgniteCheckedException {
+ PriorityQueue<GridCursor<IndexRow>> q = new PriorityQueue<>(cursors.length, cursorComp);
+
+ for (GridCursor<IndexRow> c: cursors) {
+ if (c.next())
+ q.add(c);
+ }
+
+ return q;
+ }
+ }
+
+ /** First-only, single-value-only segmented cursor. */
+ private static class FirstSegmentSingleValueIndexCursor extends SegmentedIndexCursor {
+ /**
+ * @param cursors
+ * @param idxDef
+ */
+ FirstSegmentSingleValueIndexCursor(
Review Comment:
Codestyle: each parameter should be on it's own line.
--
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
[GitHub] [ignite] Vladsz83 commented on a diff in pull request #10153: IGNITE-17250 : Calcite. Make 'min()/max()' use first/last index value. V2
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on code in PR #10153:
URL: https://github.com/apache/ignite/pull/10153#discussion_r1009521579
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/rule/IndexMinMaxRule.java:
##########
@@ -0,0 +1,95 @@
+/*
+ * 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.rule;
+
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.sql.SqlAggFunction;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.ignite.internal.processors.query.calcite.rel.IgniteAggregate;
+import org.apache.ignite.internal.processors.query.calcite.rel.IgniteIndexBound;
+import org.apache.ignite.internal.processors.query.calcite.rel.IgniteIndexScan;
+import org.apache.ignite.internal.processors.query.calcite.schema.IgniteIndex;
+import org.apache.ignite.internal.processors.query.calcite.schema.IgniteTable;
+import org.apache.ignite.internal.processors.query.calcite.trait.RewindabilityTrait;
+import org.apache.ignite.internal.util.typedef.F;
+import org.immutables.value.Value;
+
+/**
+ * Tries to optimize MIN() and MAX() so that it takes only first or last not-null index record.
+ */
+@Value.Enclosing
+public class IndexMinMaxRule extends RelRule<IndexMinMaxRule.Config> {
+ /** */
+ public static final IndexMinMaxRule INSTANCE = Config.DEFAULT.toRule();
+
+ /** Ctor. */
+ private IndexMinMaxRule(IndexMinMaxRule.Config cfg) {
+ super(cfg);
+ }
+
+ /** */
+ @Override public void onMatch(RelOptRuleCall call) {
+ IgniteAggregate aggr = call.rel(0);
+ IgniteIndexScan idxScan = call.rel(1);
+ IgniteTable table = idxScan.getTable().unwrap(IgniteTable.class);
+ IgniteIndex idx = table.getIndex(idxScan.indexName());
+
+ if (
+ table.isIndexRebuildInProgress() ||
+ idxScan.condition() != null ||
+ idxScan.projects() != null ||
+ aggr.getGroupCount() > 0 ||
+ aggr.getAggCallList().stream().filter(a -> a.getAggregation().getKind() == SqlKind.MIN
+ || a.getAggregation().getKind() == SqlKind.MAX).count() != 1 ||
+ idx.collation().getFieldCollations().isEmpty() ||
+ idx.collation().getFieldCollations().get(0).getFieldIndex() != idxScan.requiredColumns().asList().get(0)
Review Comment:
Fixed
--
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
[GitHub] [ignite] Vladsz83 commented on a diff in pull request #10153: Calcite. Make 'min()/max()' use first/last index value. V2
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on code in PR #10153:
URL: https://github.com/apache/ignite/pull/10153#discussion_r994237319
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/rule/IndexMinMaxRule.java:
##########
@@ -0,0 +1,95 @@
+/*
+ * 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.rule;
+
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.sql.SqlAggFunction;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.ignite.internal.processors.query.calcite.rel.AbstractIndexScan;
+import org.apache.ignite.internal.processors.query.calcite.rel.IgniteAggregate;
+import org.apache.ignite.internal.processors.query.calcite.rel.IgniteIndexBound;
+import org.apache.ignite.internal.processors.query.calcite.rel.IgniteIndexScan;
+import org.apache.ignite.internal.processors.query.calcite.schema.IgniteIndex;
+import org.apache.ignite.internal.processors.query.calcite.schema.IgniteTable;
+import org.apache.ignite.internal.processors.query.calcite.trait.RewindabilityTrait;
+import org.apache.ignite.internal.util.typedef.F;
+import org.immutables.value.Value;
+
+/**
+ * Tries to optimize MIN() and MAX() so that taking only first or last index record is engaged.
+ */
+@Value.Enclosing
+public class IndexMinMaxRule extends RelRule<IndexMinMaxRule.Config> {
+ /** */
+ public static final IndexMinMaxRule INSTANCE = Config.DEFAULT.toRule();
+
+ /** Ctor. */
+ private IndexMinMaxRule(IndexMinMaxRule.Config cfg) {
+ super(cfg);
+ }
+
+ /** */
+ @Override public void onMatch(RelOptRuleCall call) {
+ IgniteAggregate aggr = call.rel(0);
+ IgniteIndexScan idxScan = call.rel(1);
+ IgniteTable table = idxScan.getTable().unwrap(IgniteTable.class);
+ IgniteIndex idx = table.getIndex(idxScan.indexName());
+
+ if (
+ table.isIndexRebuildInProgress() ||
+ idxScan.condition() != null ||
+ aggr.getGroupCount() > 0 ||
+ aggr.getAggCallList().stream().filter(a -> a.getAggregation().getKind() == SqlKind.MIN
+ || a.getAggregation().getKind() == SqlKind.MAX).count() != 1 ||
+ idx.collation().getFieldCollations().isEmpty() ||
+ idx.collation().getFieldCollations().get(0).getFieldIndex() != idxScan.requiredColumns().asList().get(0)
+ )
+ return;
+
+ SqlAggFunction aggFun = aggr.getAggCallList().get(0).getAggregation();
+ boolean firstIdxValue = (aggFun.getKind() == SqlKind.MIN) !=
+ idx.collation().getFieldCollations().get(0).getDirection().isDescending();
+
+ IgniteIndexBound newAggrInput = new IgniteIndexBound(
+ idxScan.getTable(),
+ idxScan.getCluster(),
+ idxScan.getTraitSet().replace(RewindabilityTrait.REWINDABLE),
+ idxScan.indexName(),
+ firstIdxValue,
+ idx.collation()
+ );
+
+ call.transformTo(aggr.clone(aggr.getCluster(), F.asList(newAggrInput)));
+ }
+
+ /** The rule config. */
+ @Value.Immutable
+ public interface Config extends RelRule.Config {
+ /** */
+ IndexMinMaxRule.Config DEFAULT = ImmutableIndexMinMaxRule.Config.of()
+ .withDescription("IndexMinMaxRule")
+ .withOperandSupplier(r -> r.operand(IgniteAggregate.class)
+ .oneInput(i -> i.operand(AbstractIndexScan.class).anyInputs()));
Review Comment:
Fixed
--
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
[GitHub] [ignite] Vladsz83 commented on a diff in pull request #10153: Calcite. Make 'min()/max()' use first/last index value. V2
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on code in PR #10153:
URL: https://github.com/apache/ignite/pull/10153#discussion_r994236184
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/schema/SystemViewIndexImpl.java:
##########
@@ -102,6 +105,25 @@ public SystemViewIndexImpl(SystemViewTableImpl tbl) {
return tbl.descriptor().systemView().size();
}
+ /** {@inheritDoc} */
+ @Override public <Row> List<Row> findFirstOrLast(boolean first, ExecutionContext<Row> ectx,
+ ColocationGroup grp, @Nullable ImmutableBitSet requiredColumns) {
+ Iterator<Row> it = scan(ectx, grp, null, null, null, null, requiredColumns).iterator();
Review Comment:
Fixed with a not-implemented exception.
--
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
[GitHub] [ignite] Vladsz83 commented on a diff in pull request #10153: Calcite. Make 'min()/max()' use first/last index value. V2
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on code in PR #10153:
URL: https://github.com/apache/ignite/pull/10153#discussion_r994250906
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/rule/IndexMinMaxRule.java:
##########
@@ -0,0 +1,95 @@
+/*
+ * 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.rule;
+
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.sql.SqlAggFunction;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.ignite.internal.processors.query.calcite.rel.AbstractIndexScan;
+import org.apache.ignite.internal.processors.query.calcite.rel.IgniteAggregate;
+import org.apache.ignite.internal.processors.query.calcite.rel.IgniteIndexBound;
+import org.apache.ignite.internal.processors.query.calcite.rel.IgniteIndexScan;
+import org.apache.ignite.internal.processors.query.calcite.schema.IgniteIndex;
+import org.apache.ignite.internal.processors.query.calcite.schema.IgniteTable;
+import org.apache.ignite.internal.processors.query.calcite.trait.RewindabilityTrait;
+import org.apache.ignite.internal.util.typedef.F;
+import org.immutables.value.Value;
+
+/**
+ * Tries to optimize MIN() and MAX() so that taking only first or last index record is engaged.
+ */
+@Value.Enclosing
+public class IndexMinMaxRule extends RelRule<IndexMinMaxRule.Config> {
+ /** */
+ public static final IndexMinMaxRule INSTANCE = Config.DEFAULT.toRule();
+
+ /** Ctor. */
+ private IndexMinMaxRule(IndexMinMaxRule.Config cfg) {
+ super(cfg);
+ }
+
+ /** */
+ @Override public void onMatch(RelOptRuleCall call) {
+ IgniteAggregate aggr = call.rel(0);
+ IgniteIndexScan idxScan = call.rel(1);
+ IgniteTable table = idxScan.getTable().unwrap(IgniteTable.class);
+ IgniteIndex idx = table.getIndex(idxScan.indexName());
+
+ if (
+ table.isIndexRebuildInProgress() ||
+ idxScan.condition() != null ||
Review Comment:
Fixed
--
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
[GitHub] [ignite] alex-plekhanov commented on a diff in pull request #10153: Calcite. Make 'min()/max()' use first/last index value. V2
Posted by GitBox <gi...@apache.org>.
alex-plekhanov commented on code in PR #10153:
URL: https://github.com/apache/ignite/pull/10153#discussion_r961652350
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/IndexScan.java:
##########
@@ -159,6 +162,44 @@ public IndexScan(
}
}
+ /**
+ * Gets first or last records from all segments.
Review Comment:
1. Theoretically, `IndexScan` should not provide to users any information about segments.
2. Output of `firstOrLast` should be sorted, with hash aggregate it will work as is, but with sort aggregate it will return wrong results.
3. It should return `Cursor`, not `List`. In the current implementation first/last scan will be executed during execution nodes building (in `LogicalRelImplementor`), but actually scan should only be run on execution stage.
I propose to refactor a little bit:
1. Extend `IndexScan` with some kind of new class (`IndexFirstLastScan` for example), implement `indexQueryContext` and `iterator` methods, and new tree index wrapper class which will wrap `idx.findFirst`/`idx.findLast` methods.
2. Add new methods findFirst/findLast to the `InlineIndexImpl` to get sorted values for all segments (see find method and `SegmentedIndexCursor` class), perhaps it's worth to return only one value by these methods to maintain API consistency (for example create some `SingleValueSegmentedIndexCursor extends SegmentedIndexCursor`)
##########
modules/core/src/main/java/org/apache/ignite/internal/util/lang/GridFunc.java:
##########
@@ -1186,6 +1187,42 @@ public static <T> boolean isEmptyOrNulls(@Nullable T[] c) {
return true;
}
+ /**
+ * Tests if the given object is null, is empty array, empty map or contains only nulls.
+ *
+ * @param o Object to test.
+ * @return {@code True}, is given object is null, is empty array, empty map or contains only nulls.
+ * {@code False} otherwise.
+ */
+ public static boolean isEmptyOrNulls(@Nullable Object o) {
Review Comment:
Redundant (if system view first/last scan will be removed).
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/rule/IndexMinMaxRule.java:
##########
@@ -0,0 +1,95 @@
+/*
+ * 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.rule;
+
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.sql.SqlAggFunction;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.ignite.internal.processors.query.calcite.rel.AbstractIndexScan;
+import org.apache.ignite.internal.processors.query.calcite.rel.IgniteAggregate;
+import org.apache.ignite.internal.processors.query.calcite.rel.IgniteIndexBound;
+import org.apache.ignite.internal.processors.query.calcite.rel.IgniteIndexScan;
+import org.apache.ignite.internal.processors.query.calcite.schema.IgniteIndex;
+import org.apache.ignite.internal.processors.query.calcite.schema.IgniteTable;
+import org.apache.ignite.internal.processors.query.calcite.trait.RewindabilityTrait;
+import org.apache.ignite.internal.util.typedef.F;
+import org.immutables.value.Value;
+
+/**
+ * Tries to optimize MIN() and MAX() so that taking only first or last index record is engaged.
+ */
+@Value.Enclosing
+public class IndexMinMaxRule extends RelRule<IndexMinMaxRule.Config> {
+ /** */
+ public static final IndexMinMaxRule INSTANCE = Config.DEFAULT.toRule();
+
+ /** Ctor. */
+ private IndexMinMaxRule(IndexMinMaxRule.Config cfg) {
+ super(cfg);
+ }
+
+ /** */
+ @Override public void onMatch(RelOptRuleCall call) {
+ IgniteAggregate aggr = call.rel(0);
+ IgniteIndexScan idxScan = call.rel(1);
+ IgniteTable table = idxScan.getTable().unwrap(IgniteTable.class);
+ IgniteIndex idx = table.getIndex(idxScan.indexName());
+
+ if (
+ table.isIndexRebuildInProgress() ||
+ idxScan.condition() != null ||
Review Comment:
Projections should also be null
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/schema/SystemViewIndexImpl.java:
##########
@@ -102,6 +105,25 @@ public SystemViewIndexImpl(SystemViewTableImpl tbl) {
return tbl.descriptor().systemView().size();
}
+ /** {@inheritDoc} */
+ @Override public <Row> List<Row> findFirstOrLast(boolean first, ExecutionContext<Row> ectx,
+ ColocationGroup grp, @Nullable ImmutableBitSet requiredColumns) {
+ Iterator<Row> it = scan(ectx, grp, null, null, null, null, requiredColumns).iterator();
Review Comment:
Since collation for system view indexes is always empty, there is impossible to rich this code, I think we should just throw assertion error here instead of implementation.
In any case, `isNullOrEmpty` should not be used, see https://github.com/apache/ignite/pull/10117#discussion_r916578233
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/LogicalRelImplementor.java:
##########
@@ -428,6 +429,44 @@ public LogicalRelImplementor(
}
}
+ /** {@inheritDoc} */
+ @Override public Node<Row> visit(IgniteIndexBound rel) {
+ IgniteTable tbl = rel.getTable().unwrap(IgniteTable.class);
+ IgniteIndex idx = tbl.getIndex(rel.indexName());
+ IgniteTypeFactory typeFactory = ctx.getTypeFactory();
+ ColocationGroup grp = ctx.group(rel.sourceId());
+ ImmutableBitSet requiredColumns = rel.requiredColumns();
+ RelDataType rowType = tbl.getRowType(typeFactory, requiredColumns);
+
+ if (idx != null && !tbl.isIndexRebuildInProgress()) {
+ return new ScanNode<>(ctx, rowType, idx.findFirstOrLast(rel.first(), ctx, ctx.group(rel.sourceId()),
+ requiredColumns));
+ }
+ else {
+ Iterable<Row> rowsIter = tbl.scan(
+ ctx,
+ grp,
+ null,
+ null,
+ rel.requiredColumns()
+ );
+
+ Node<Row> scanNode = new ScanNode<>(ctx, rowType, rowsIter);
+
+ RelCollation collation = idx.collation().apply(LogicalScanConverterRule.createMapping(
+ null,
+ requiredColumns,
+ tbl.getRowType(typeFactory).getFieldCount()
+ ));
+
+ SortNode<Row> sortNode = new SortNode<>(ctx, rowType, expressionFactory.comparator(collation));
Review Comment:
Sort with limit(1) should be used
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/rule/IndexMinMaxRule.java:
##########
@@ -0,0 +1,95 @@
+/*
+ * 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.rule;
+
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.sql.SqlAggFunction;
+import org.apache.calcite.sql.SqlKind;
+import org.apache.ignite.internal.processors.query.calcite.rel.AbstractIndexScan;
+import org.apache.ignite.internal.processors.query.calcite.rel.IgniteAggregate;
+import org.apache.ignite.internal.processors.query.calcite.rel.IgniteIndexBound;
+import org.apache.ignite.internal.processors.query.calcite.rel.IgniteIndexScan;
+import org.apache.ignite.internal.processors.query.calcite.schema.IgniteIndex;
+import org.apache.ignite.internal.processors.query.calcite.schema.IgniteTable;
+import org.apache.ignite.internal.processors.query.calcite.trait.RewindabilityTrait;
+import org.apache.ignite.internal.util.typedef.F;
+import org.immutables.value.Value;
+
+/**
+ * Tries to optimize MIN() and MAX() so that taking only first or last index record is engaged.
+ */
+@Value.Enclosing
+public class IndexMinMaxRule extends RelRule<IndexMinMaxRule.Config> {
+ /** */
+ public static final IndexMinMaxRule INSTANCE = Config.DEFAULT.toRule();
+
+ /** Ctor. */
+ private IndexMinMaxRule(IndexMinMaxRule.Config cfg) {
+ super(cfg);
+ }
+
+ /** */
+ @Override public void onMatch(RelOptRuleCall call) {
+ IgniteAggregate aggr = call.rel(0);
+ IgniteIndexScan idxScan = call.rel(1);
+ IgniteTable table = idxScan.getTable().unwrap(IgniteTable.class);
+ IgniteIndex idx = table.getIndex(idxScan.indexName());
+
+ if (
+ table.isIndexRebuildInProgress() ||
+ idxScan.condition() != null ||
+ aggr.getGroupCount() > 0 ||
+ aggr.getAggCallList().stream().filter(a -> a.getAggregation().getKind() == SqlKind.MIN
+ || a.getAggregation().getKind() == SqlKind.MAX).count() != 1 ||
+ idx.collation().getFieldCollations().isEmpty() ||
+ idx.collation().getFieldCollations().get(0).getFieldIndex() != idxScan.requiredColumns().asList().get(0)
+ )
+ return;
+
+ SqlAggFunction aggFun = aggr.getAggCallList().get(0).getAggregation();
+ boolean firstIdxValue = (aggFun.getKind() == SqlKind.MIN) !=
+ idx.collation().getFieldCollations().get(0).getDirection().isDescending();
+
+ IgniteIndexBound newAggrInput = new IgniteIndexBound(
+ idxScan.getTable(),
+ idxScan.getCluster(),
+ idxScan.getTraitSet().replace(RewindabilityTrait.REWINDABLE),
+ idxScan.indexName(),
+ firstIdxValue,
+ idx.collation()
+ );
+
+ call.transformTo(aggr.clone(aggr.getCluster(), F.asList(newAggrInput)));
+ }
+
+ /** The rule config. */
+ @Value.Immutable
+ public interface Config extends RelRule.Config {
+ /** */
+ IndexMinMaxRule.Config DEFAULT = ImmutableIndexMinMaxRule.Config.of()
+ .withDescription("IndexMinMaxRule")
+ .withOperandSupplier(r -> r.operand(IgniteAggregate.class)
+ .oneInput(i -> i.operand(AbstractIndexScan.class).anyInputs()));
Review Comment:
Why `AbstractIndexScan` but not `IgniteIndexScan` (physical node)? Using only physical node will reduce search space in my opinion.
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/agg/Accumulators.java:
##########
@@ -973,7 +973,7 @@ private DecimalMinMax(AggregateCall aggCall, RowHandler<Row> hnd, boolean min) {
private final boolean min;
/** */
- private final Function<IgniteTypeFactory, RelDataType> typeSupplier;
+ private final transient Function<IgniteTypeFactory, RelDataType> typeSupplier;
Review Comment:
Is it really required? Looks like accumulators is not serialized
##########
modules/core/src/main/java/org/apache/ignite/cache/QueryEntity.java:
##########
@@ -780,7 +780,7 @@ private static void processAnnotationsInClass(boolean key, Class<?> cls, QueryEn
@Nullable QueryEntityClassProperty parent) {
if (U.isJdk(cls) || QueryUtils.isGeometryClass(cls)) {
if (parent == null && !key && QueryUtils.isSqlType(cls)) { // We have to index primitive _val.
- String idxName = cls.getSimpleName() + "_" + QueryUtils.VAL_FIELD_NAME + "_idx";
+ String idxName = QueryUtils.indexName(cls.getSimpleName(), QueryUtils.VAL_FIELD_NAME);
Review Comment:
1. Local method is enough
2. Looks like redundant check at all
--
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
[GitHub] [ignite] Vladsz83 commented on a diff in pull request #10153: Calcite. Make 'min()/max()' use first/last index value. V2
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on code in PR #10153:
URL: https://github.com/apache/ignite/pull/10153#discussion_r994252791
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/agg/Accumulators.java:
##########
@@ -973,7 +973,7 @@ private DecimalMinMax(AggregateCall aggCall, RowHandler<Row> hnd, boolean min) {
private final boolean min;
/** */
- private final Function<IgniteTypeFactory, RelDataType> typeSupplier;
+ private final transient Function<IgniteTypeFactory, RelDataType> typeSupplier;
Review Comment:
Fixed
--
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
[GitHub] [ignite] asfgit closed pull request #10153: IGNITE-17250 : Calcite. Make 'min()/max()' use first/last index value. V2
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #10153: IGNITE-17250 : Calcite. Make 'min()/max()' use first/last index value. V2
URL: https://github.com/apache/ignite/pull/10153
--
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
[GitHub] [ignite] Vladsz83 commented on a diff in pull request #10153: Calcite. Make 'min()/max()' use first/last index value. V2
Posted by GitBox <gi...@apache.org>.
Vladsz83 commented on code in PR #10153:
URL: https://github.com/apache/ignite/pull/10153#discussion_r1000490014
##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/IndexScan.java:
##########
@@ -159,6 +162,44 @@ public IndexScan(
}
}
+ /**
+ * Gets first or last records from all segments.
Review Comment:
Refactored.
--
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