You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by "lowka (via GitHub)" <gi...@apache.org> on 2024/02/08 11:20:44 UTC
[PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
lowka opened a new pull request, #3187:
URL: https://github.com/apache/ignite-3/pull/3187
https://issues.apache.org/jira/browse/IGNITE-21279
---
Thank you for submitting the pull request.
To streamline the review process of the patch and ensure better code quality
we ask both an author and a reviewer to verify the following:
### The Review Checklist
- [ ] **Formal criteria:** TC status, codestyle, mandatory documentation. Also make sure to complete the following:
\- 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 description of the JIRA ticket explains WHAT was made, WHY and 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.
- [ ] **Design:** new code conforms with the design principles of the components it is added to.
- [ ] **Patch quality:** patch cannot be split into smaller pieces, its size must be reasonable.
- [ ] **Code quality:** code is clean and readable, necessary developer documentation is added if needed.
- [ ] **Tests code quality:** test set covers positive/negative scenarios, happy/edge cases. Tests are effective in terms of execution time and resources.
### Notes
- [Apache Ignite Coding Guidelines](https://cwiki.apache.org/confluence/display/IGNITE/Java+Code+Style+Guide)
--
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1491404103
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/pruning/PartitionPruningPredicate.java:
##########
@@ -0,0 +1,197 @@
+/*
+ * 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.sql.engine.prepare.pruning;
+
+import it.unimi.dsi.fastutil.ints.Int2ObjectMap;
+import it.unimi.dsi.fastutil.ints.IntArraySet;
+import it.unimi.dsi.fastutil.ints.IntSet;
+import java.time.Instant;
+import java.time.LocalDate;
+import java.time.LocalDateTime;
+import java.time.LocalTime;
+import java.time.ZoneId;
+import java.util.ArrayList;
+import java.util.Calendar;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import org.apache.calcite.rex.RexDynamicParam;
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.util.ImmutableIntList;
+import org.apache.calcite.util.TimestampString;
+import org.apache.ignite.internal.sql.engine.exec.NodeWithConsistencyToken;
+import org.apache.ignite.internal.sql.engine.exec.PartitionWithConsistencyToken;
+import org.apache.ignite.internal.sql.engine.exec.mapping.ColocationGroup;
+import org.apache.ignite.internal.sql.engine.schema.IgniteTable;
+import org.apache.ignite.internal.sql.engine.schema.PartitionCalculator;
+import org.apache.ignite.internal.type.NativeType;
+import org.apache.ignite.internal.type.NativeTypeSpec;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Partition predicate encapsulates partition pruning logic.
+ */
+public final class PartitionPruningPredicate {
+
+ private static final ZoneId ZONE_ID_UTC = ZoneId.of("UTC");
+
+ private final int tablePartitions;
+
+ @Nullable
+ private final IntSet remainingPartitions;
+
+ /**
+ * Constructor.
+ *
+ * @param table Table.
+ * @param pruningColumns Columns.
+ * @param dynamicParameters Values of dynamic parameters.
+ */
+ public PartitionPruningPredicate(IgniteTable table, PartitionPruningColumns pruningColumns, Object[] dynamicParameters) {
+ this.tablePartitions = table.partitions();
+ this.remainingPartitions = computeRemainingPartitions(table, pruningColumns, dynamicParameters);
+ }
+
+ /**
+ * Applies partition pruning to the given colocation group. This group should have the same number of assignments as the source table.
+ *
+ * @param colocationGroup Colocation group.
+ *
+ * @return New colocation group.
+ */
+ public ColocationGroup prunePartitions(ColocationGroup colocationGroup) {
+ assert tablePartitions == colocationGroup.assignments().size() : "Number of partitions does not match";
+
+ if (remainingPartitions == null) {
+ return colocationGroup;
+ }
+
+ Map<String, List<PartitionWithConsistencyToken>> partitionsPerNode = new HashMap<>(colocationGroup.nodeNames().size());
+ Set<String> newNodes = new HashSet<>();
+
+ for (String nodeName : colocationGroup.nodeNames()) {
+ List<PartitionWithConsistencyToken> partsWithConsistencyTokens = new ArrayList<>();
+
+ for (int p = 0; p < colocationGroup.assignments().size(); p++) {
+ NodeWithConsistencyToken nodeWithConsistencyToken = colocationGroup.assignments().get(p);
+ if (!remainingPartitions.contains(p)) {
+ continue;
+ }
+
+ if (Objects.equals(nodeName, nodeWithConsistencyToken.name())) {
+ long t = nodeWithConsistencyToken.enlistmentConsistencyToken();
+
+ partsWithConsistencyTokens.add(new PartitionWithConsistencyToken(p, t));
+ newNodes.add(nodeName);
+ }
+ }
+
+ if (!partsWithConsistencyTokens.isEmpty()) {
+ partitionsPerNode.put(nodeName, partsWithConsistencyTokens);
+ }
+ }
+
+ // Keep assignments intact, because they are used by DestinationFactory.
+ return new ColocationGroup(
+ colocationGroup.sourceIds(),
+ List.copyOf(newNodes),
+ colocationGroup.assignments(),
+ partitionsPerNode
+ );
+ }
+
+ @Nullable
+ private static IntSet computeRemainingPartitions(
+ IgniteTable table,
+ PartitionPruningColumns pruningColumns,
+ Object[] dynamicParameters
+ ) {
+ ImmutableIntList keys = table.distribution().getKeys();
+ IntSet remainingPartitions = new IntArraySet(pruningColumns.columns().size());
+ PartitionCalculator partitionCalculator = table.partitionCalculator().get();
+
+ for (Int2ObjectMap<RexNode> columns : pruningColumns.columns()) {
+ for (int key : keys) {
+ RexNode node = columns.get(key);
+ NativeType physicalType = table.descriptor().columnDescriptor(key).physicalType();
+
+ // TODO: Custom data types in partition pruning.
+ if (physicalType.spec() == NativeTypeSpec.UUID) {
+ return null;
+ }
+
+ Object val = getNodeValue(physicalType, node, dynamicParameters);
+
+ partitionCalculator.append(val);
+ }
+
+ int p = partitionCalculator.partition();
+ remainingPartitions.add(p);
+ }
+
+ return remainingPartitions;
+ }
+
+ @Nullable
+ private static Object getNodeValue(NativeType physicalType, RexNode node, Object[] dynamicParameters) {
+ Object val;
+
+ if (node instanceof RexLiteral) {
+ val = getValueFromLiteral(physicalType, (RexLiteral) node);
+ } else if (node instanceof RexDynamicParam) {
+ RexDynamicParam dynamicParam = (RexDynamicParam) node;
+ val = dynamicParameters[dynamicParam.getIndex()];
+ } else {
+ throw new IllegalArgumentException("Unexpected column value node: " + node);
+ }
+
+ return val;
+ }
+
+ @Nullable
+ private static Object getValueFromLiteral(NativeType physicalType, RexLiteral lit) {
+ if (physicalType.spec() == NativeTypeSpec.DATE) {
Review Comment:
This code can be rewritten by using an expression compilation, but `ExpressionFactory` currently depends on `ExecutionContext` for no reason at all (it uses `TypeFactory` and `RowHandler`, if there are other dependencies I almost certain that they are also trivial).
It would be better to make `ExpressionFactory` independent of `ExecutionContext`.
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/pruning/PartitionPruningPredicate.java:
##########
@@ -0,0 +1,197 @@
+/*
+ * 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.sql.engine.prepare.pruning;
+
+import it.unimi.dsi.fastutil.ints.Int2ObjectMap;
+import it.unimi.dsi.fastutil.ints.IntArraySet;
+import it.unimi.dsi.fastutil.ints.IntSet;
+import java.time.Instant;
+import java.time.LocalDate;
+import java.time.LocalDateTime;
+import java.time.LocalTime;
+import java.time.ZoneId;
+import java.util.ArrayList;
+import java.util.Calendar;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import org.apache.calcite.rex.RexDynamicParam;
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.util.ImmutableIntList;
+import org.apache.calcite.util.TimestampString;
+import org.apache.ignite.internal.sql.engine.exec.NodeWithConsistencyToken;
+import org.apache.ignite.internal.sql.engine.exec.PartitionWithConsistencyToken;
+import org.apache.ignite.internal.sql.engine.exec.mapping.ColocationGroup;
+import org.apache.ignite.internal.sql.engine.schema.IgniteTable;
+import org.apache.ignite.internal.sql.engine.schema.PartitionCalculator;
+import org.apache.ignite.internal.type.NativeType;
+import org.apache.ignite.internal.type.NativeTypeSpec;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Partition predicate encapsulates partition pruning logic.
+ */
+public final class PartitionPruningPredicate {
+
+ private static final ZoneId ZONE_ID_UTC = ZoneId.of("UTC");
+
+ private final int tablePartitions;
+
+ @Nullable
+ private final IntSet remainingPartitions;
+
+ /**
+ * Constructor.
+ *
+ * @param table Table.
+ * @param pruningColumns Columns.
+ * @param dynamicParameters Values of dynamic parameters.
+ */
+ public PartitionPruningPredicate(IgniteTable table, PartitionPruningColumns pruningColumns, Object[] dynamicParameters) {
+ this.tablePartitions = table.partitions();
+ this.remainingPartitions = computeRemainingPartitions(table, pruningColumns, dynamicParameters);
+ }
+
+ /**
+ * Applies partition pruning to the given colocation group. This group should have the same number of assignments as the source table.
+ *
+ * @param colocationGroup Colocation group.
+ *
+ * @return New colocation group.
+ */
+ public ColocationGroup prunePartitions(ColocationGroup colocationGroup) {
+ assert tablePartitions == colocationGroup.assignments().size() : "Number of partitions does not match";
+
+ if (remainingPartitions == null) {
+ return colocationGroup;
+ }
+
+ Map<String, List<PartitionWithConsistencyToken>> partitionsPerNode = new HashMap<>(colocationGroup.nodeNames().size());
+ Set<String> newNodes = new HashSet<>();
+
+ for (String nodeName : colocationGroup.nodeNames()) {
+ List<PartitionWithConsistencyToken> partsWithConsistencyTokens = new ArrayList<>();
+
+ for (int p = 0; p < colocationGroup.assignments().size(); p++) {
+ NodeWithConsistencyToken nodeWithConsistencyToken = colocationGroup.assignments().get(p);
+ if (!remainingPartitions.contains(p)) {
+ continue;
+ }
+
+ if (Objects.equals(nodeName, nodeWithConsistencyToken.name())) {
+ long t = nodeWithConsistencyToken.enlistmentConsistencyToken();
+
+ partsWithConsistencyTokens.add(new PartitionWithConsistencyToken(p, t));
+ newNodes.add(nodeName);
+ }
+ }
+
+ if (!partsWithConsistencyTokens.isEmpty()) {
+ partitionsPerNode.put(nodeName, partsWithConsistencyTokens);
+ }
+ }
+
+ // Keep assignments intact, because they are used by DestinationFactory.
+ return new ColocationGroup(
+ colocationGroup.sourceIds(),
+ List.copyOf(newNodes),
+ colocationGroup.assignments(),
+ partitionsPerNode
+ );
+ }
+
+ @Nullable
+ private static IntSet computeRemainingPartitions(
+ IgniteTable table,
+ PartitionPruningColumns pruningColumns,
+ Object[] dynamicParameters
+ ) {
+ ImmutableIntList keys = table.distribution().getKeys();
+ IntSet remainingPartitions = new IntArraySet(pruningColumns.columns().size());
+ PartitionCalculator partitionCalculator = table.partitionCalculator().get();
+
+ for (Int2ObjectMap<RexNode> columns : pruningColumns.columns()) {
+ for (int key : keys) {
+ RexNode node = columns.get(key);
+ NativeType physicalType = table.descriptor().columnDescriptor(key).physicalType();
+
+ // TODO: Custom data types in partition pruning.
+ if (physicalType.spec() == NativeTypeSpec.UUID) {
+ return null;
+ }
+
+ Object val = getNodeValue(physicalType, node, dynamicParameters);
+
+ partitionCalculator.append(val);
+ }
+
+ int p = partitionCalculator.partition();
+ remainingPartitions.add(p);
+ }
+
+ return remainingPartitions;
+ }
+
+ @Nullable
+ private static Object getNodeValue(NativeType physicalType, RexNode node, Object[] dynamicParameters) {
+ Object val;
+
+ if (node instanceof RexLiteral) {
+ val = getValueFromLiteral(physicalType, (RexLiteral) node);
+ } else if (node instanceof RexDynamicParam) {
+ RexDynamicParam dynamicParam = (RexDynamicParam) node;
+ val = dynamicParameters[dynamicParam.getIndex()];
+ } else {
+ throw new IllegalArgumentException("Unexpected column value node: " + node);
+ }
+
+ return val;
+ }
+
+ @Nullable
+ private static Object getValueFromLiteral(NativeType physicalType, RexLiteral lit) {
+ if (physicalType.spec() == NativeTypeSpec.DATE) {
Review Comment:
This code can be rewritten by using expression compilation, but `ExpressionFactory` currently depends on `ExecutionContext` for no reason at all (it uses `TypeFactory` and `RowHandler`, if there are other dependencies I almost certain that they are also trivial).
It would be better to make `ExpressionFactory` independent of `ExecutionContext`.
--
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1491417234
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/PartitionWithConsistencyToken.java:
##########
@@ -55,4 +57,29 @@ public int partId() {
public long enlistmentConsistencyToken() {
return enlistmentConsistencyToken;
}
+
+ /** {@inheritDoc} */
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ }
+ if (o == null || getClass() != o.getClass()) {
+ return false;
+ }
+ PartitionWithConsistencyToken that = (PartitionWithConsistencyToken) o;
+ return partId == that.partId && enlistmentConsistencyToken == that.enlistmentConsistencyToken;
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public int hashCode() {
+ return Objects.hash(partId, enlistmentConsistencyToken);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public String toString() {
+ return partId + ":" + enlistmentConsistencyToken;
Review Comment:
Same as `NodeWithConsistencyToken`.
--
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1492489204
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/pruning/PartitionPrunerImpl.java:
##########
@@ -0,0 +1,190 @@
+/*
+ * 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.sql.engine.prepare.pruning;
+
+import it.unimi.dsi.fastutil.longs.Long2ObjectArrayMap;
+import it.unimi.dsi.fastutil.longs.Long2ObjectMap;
+import it.unimi.dsi.fastutil.longs.Long2ObjectMap.Entry;
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.ignite.internal.sql.engine.exec.mapping.ColocationGroup;
+import org.apache.ignite.internal.sql.engine.exec.mapping.MappedFragment;
+import org.apache.ignite.internal.sql.engine.prepare.Fragment;
+import org.apache.ignite.internal.sql.engine.prepare.IgniteRelShuttle;
+import org.apache.ignite.internal.sql.engine.rel.IgniteReceiver;
+import org.apache.ignite.internal.sql.engine.rel.IgniteRel;
+import org.apache.ignite.internal.sql.engine.rel.IgniteSender;
+import org.apache.ignite.internal.sql.engine.schema.IgniteTable;
+import org.jetbrains.annotations.Nullable;
+
+/** Applies partition pruning. */
+public class PartitionPrunerImpl implements PartitionPruner {
+
+ /** Constructor. */
+ public PartitionPrunerImpl() {
+
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public List<MappedFragment> apply(
+ List<MappedFragment> mappedFragments,
+ Object[] dynamicParameters
+ ) {
+ List<MappedFragment> updatedFragments = new ArrayList<>(mappedFragments.size());
+ Long2ObjectMap<List<String>> newNodesByExchangeId = new Long2ObjectArrayMap<>();
+
+ // Partition pruning (PP). For each fragment:
+ //
+ // 1. Extract PP metadata from eac in the form of [colo_col1=<val>, ..] (see PartitionPruningMetadataExtractor)
+ //
+ // 2. If PP metadata exists then update fragment's colocation group
+ // to retain partition that are necessary to perform an operator (e.g. for a scan operator such
+ // partitions only include that ones that can contain data).
+ //
+ // Iterate over fragments again to update fragments that receive data from fragments updated at step 2.
+ // This is accomplished by updating `sourcesByExchangeId`.
+ //
+
+ for (MappedFragment mappedFragment : mappedFragments) {
+ // Fragment that contains colocated operators has exactly one colocation group.
+ // Do not attempt to apply PP to other fragments.
+ if (mappedFragment.groups().size() != 1) {
+ updatedFragments.add(mappedFragment);
+ continue;
+ }
+
+ Fragment fragment = mappedFragment.fragment();
+ if (fragment.tables().isEmpty()) {
+ updatedFragments.add(mappedFragment);
+ continue;
+ }
+
+ PartitionPruningMetadataExtractor extractor = new PartitionPruningMetadataExtractor();
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1492487882
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/pruning/PartitionPrunerImpl.java:
##########
@@ -0,0 +1,190 @@
+/*
+ * 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.sql.engine.prepare.pruning;
+
+import it.unimi.dsi.fastutil.longs.Long2ObjectArrayMap;
+import it.unimi.dsi.fastutil.longs.Long2ObjectMap;
+import it.unimi.dsi.fastutil.longs.Long2ObjectMap.Entry;
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.ignite.internal.sql.engine.exec.mapping.ColocationGroup;
+import org.apache.ignite.internal.sql.engine.exec.mapping.MappedFragment;
+import org.apache.ignite.internal.sql.engine.prepare.Fragment;
+import org.apache.ignite.internal.sql.engine.prepare.IgniteRelShuttle;
+import org.apache.ignite.internal.sql.engine.rel.IgniteReceiver;
+import org.apache.ignite.internal.sql.engine.rel.IgniteRel;
+import org.apache.ignite.internal.sql.engine.rel.IgniteSender;
+import org.apache.ignite.internal.sql.engine.schema.IgniteTable;
+import org.jetbrains.annotations.Nullable;
+
+/** Applies partition pruning. */
+public class PartitionPrunerImpl implements PartitionPruner {
+
+ /** Constructor. */
+ public PartitionPrunerImpl() {
+
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public List<MappedFragment> apply(
Review Comment:
You are right. Initial implementation of this code was a class with a single static method.
I added an interface so we can test mapping separately.
--
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1492520922
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/pruning/PartitionPruningColumns.java:
##########
@@ -56,7 +55,7 @@ public List<Int2ObjectMap<RexNode>> columns() {
/** {@inheritDoc} */
@Override
public String toString() {
- return S.toString(this);
+ return columns.toString();
Review Comment:
Removed IgniteToStringInclude.
--
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "AMashenkov (via GitHub)" <gi...@apache.org>.
AMashenkov commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1497570698
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/mapping/QuerySplitter.java:
##########
@@ -188,12 +191,13 @@ public IgniteRel visit(IgniteTableScan rel) {
IgniteTable table = rel.getTable().unwrap(IgniteTable.class);
assert table != null;
+ long sourceId = idGenerator.nextId();
if (curr.seenRelations.add(table.id())) {
- curr.tables.add(table);
+ curr.tables.put(sourceId, table);
Review Comment:
But table/view may have different sourceId? e.g. if they occurs twice within same query.
Let's add a comment with explanation, why this behaviour is correct.
--
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1490679838
##########
modules/sql-engine/src/test/resources/mapping/test_partition_pruning.test:
##########
@@ -0,0 +1,50 @@
+# Pruning from a simple scan
+N1
+SELECT * FROM t1_n1n2n3 WHERE id = 0
+---
+Fragment#0 root
+ executionNodes: [N1]
+ remoteFragments: [1]
+ exchangeSourceNodes: {1=[N2]}
+ tree:
Review Comment:
Maybe we can include information regarding used partitions to this out?
--
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1491417234
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/PartitionWithConsistencyToken.java:
##########
@@ -55,4 +57,29 @@ public int partId() {
public long enlistmentConsistencyToken() {
return enlistmentConsistencyToken;
}
+
+ /** {@inheritDoc} */
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ }
+ if (o == null || getClass() != o.getClass()) {
+ return false;
+ }
+ PartitionWithConsistencyToken that = (PartitionWithConsistencyToken) o;
+ return partId == that.partId && enlistmentConsistencyToken == that.enlistmentConsistencyToken;
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public int hashCode() {
+ return Objects.hash(partId, enlistmentConsistencyToken);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public String toString() {
+ return partId + ":" + enlistmentConsistencyToken;
Review Comment:
See `NodeWithConsistencyToken::toString`.
--
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#issuecomment-1948321670
Fixed benchmarks code.
--
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1499092196
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/mapping/QuerySplitter.java:
##########
@@ -188,12 +191,13 @@ public IgniteRel visit(IgniteTableScan rel) {
IgniteTable table = rel.getTable().unwrap(IgniteTable.class);
assert table != null;
+ long sourceId = idGenerator.nextId();
if (curr.seenRelations.add(table.id())) {
- curr.tables.add(table);
+ curr.tables.put(sourceId, table);
Review Comment:
Well, I was not correct. Updated splitter to every mention of a table in a particular fragment.
--
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1492480741
##########
.idea/codeStyles/Project.xml:
##########
@@ -42,7 +42,7 @@
</value>
</option>
</AndroidXmlCodeStyleSettings>
- <JSCodeStyleSettings>
+ <JSCodeStyleSettings version="0">
Review Comment:
Fixed. Looks like IDEA changes them.
--
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "AMashenkov (via GitHub)" <gi...@apache.org>.
AMashenkov commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1494433227
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/mapping/QuerySplitter.java:
##########
@@ -188,12 +191,13 @@ public IgniteRel visit(IgniteTableScan rel) {
IgniteTable table = rel.getTable().unwrap(IgniteTable.class);
assert table != null;
+ long sourceId = idGenerator.nextId();
if (curr.seenRelations.add(table.id())) {
- curr.tables.add(table);
+ curr.tables.put(sourceId, table);
Review Comment:
Does this mean table will be added only once, the same table with different sourceId will be ignored and later can't be found?
--
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1494439367
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/mapping/QuerySplitter.java:
##########
@@ -188,12 +191,13 @@ public IgniteRel visit(IgniteTableScan rel) {
IgniteTable table = rel.getTable().unwrap(IgniteTable.class);
assert table != null;
+ long sourceId = idGenerator.nextId();
if (curr.seenRelations.add(table.id())) {
- curr.tables.add(table);
+ curr.tables.put(sourceId, table);
Review Comment:
Thanks. 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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1491404103
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/pruning/PartitionPruningPredicate.java:
##########
@@ -0,0 +1,197 @@
+/*
+ * 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.sql.engine.prepare.pruning;
+
+import it.unimi.dsi.fastutil.ints.Int2ObjectMap;
+import it.unimi.dsi.fastutil.ints.IntArraySet;
+import it.unimi.dsi.fastutil.ints.IntSet;
+import java.time.Instant;
+import java.time.LocalDate;
+import java.time.LocalDateTime;
+import java.time.LocalTime;
+import java.time.ZoneId;
+import java.util.ArrayList;
+import java.util.Calendar;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import org.apache.calcite.rex.RexDynamicParam;
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.util.ImmutableIntList;
+import org.apache.calcite.util.TimestampString;
+import org.apache.ignite.internal.sql.engine.exec.NodeWithConsistencyToken;
+import org.apache.ignite.internal.sql.engine.exec.PartitionWithConsistencyToken;
+import org.apache.ignite.internal.sql.engine.exec.mapping.ColocationGroup;
+import org.apache.ignite.internal.sql.engine.schema.IgniteTable;
+import org.apache.ignite.internal.sql.engine.schema.PartitionCalculator;
+import org.apache.ignite.internal.type.NativeType;
+import org.apache.ignite.internal.type.NativeTypeSpec;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Partition predicate encapsulates partition pruning logic.
+ */
+public final class PartitionPruningPredicate {
+
+ private static final ZoneId ZONE_ID_UTC = ZoneId.of("UTC");
+
+ private final int tablePartitions;
+
+ @Nullable
+ private final IntSet remainingPartitions;
+
+ /**
+ * Constructor.
+ *
+ * @param table Table.
+ * @param pruningColumns Columns.
+ * @param dynamicParameters Values of dynamic parameters.
+ */
+ public PartitionPruningPredicate(IgniteTable table, PartitionPruningColumns pruningColumns, Object[] dynamicParameters) {
+ this.tablePartitions = table.partitions();
+ this.remainingPartitions = computeRemainingPartitions(table, pruningColumns, dynamicParameters);
+ }
+
+ /**
+ * Applies partition pruning to the given colocation group. This group should have the same number of assignments as the source table.
+ *
+ * @param colocationGroup Colocation group.
+ *
+ * @return New colocation group.
+ */
+ public ColocationGroup prunePartitions(ColocationGroup colocationGroup) {
+ assert tablePartitions == colocationGroup.assignments().size() : "Number of partitions does not match";
+
+ if (remainingPartitions == null) {
+ return colocationGroup;
+ }
+
+ Map<String, List<PartitionWithConsistencyToken>> partitionsPerNode = new HashMap<>(colocationGroup.nodeNames().size());
+ Set<String> newNodes = new HashSet<>();
+
+ for (String nodeName : colocationGroup.nodeNames()) {
+ List<PartitionWithConsistencyToken> partsWithConsistencyTokens = new ArrayList<>();
+
+ for (int p = 0; p < colocationGroup.assignments().size(); p++) {
+ NodeWithConsistencyToken nodeWithConsistencyToken = colocationGroup.assignments().get(p);
+ if (!remainingPartitions.contains(p)) {
+ continue;
+ }
+
+ if (Objects.equals(nodeName, nodeWithConsistencyToken.name())) {
+ long t = nodeWithConsistencyToken.enlistmentConsistencyToken();
+
+ partsWithConsistencyTokens.add(new PartitionWithConsistencyToken(p, t));
+ newNodes.add(nodeName);
+ }
+ }
+
+ if (!partsWithConsistencyTokens.isEmpty()) {
+ partitionsPerNode.put(nodeName, partsWithConsistencyTokens);
+ }
+ }
+
+ // Keep assignments intact, because they are used by DestinationFactory.
+ return new ColocationGroup(
+ colocationGroup.sourceIds(),
+ List.copyOf(newNodes),
+ colocationGroup.assignments(),
+ partitionsPerNode
+ );
+ }
+
+ @Nullable
+ private static IntSet computeRemainingPartitions(
+ IgniteTable table,
+ PartitionPruningColumns pruningColumns,
+ Object[] dynamicParameters
+ ) {
+ ImmutableIntList keys = table.distribution().getKeys();
+ IntSet remainingPartitions = new IntArraySet(pruningColumns.columns().size());
+ PartitionCalculator partitionCalculator = table.partitionCalculator().get();
+
+ for (Int2ObjectMap<RexNode> columns : pruningColumns.columns()) {
+ for (int key : keys) {
+ RexNode node = columns.get(key);
+ NativeType physicalType = table.descriptor().columnDescriptor(key).physicalType();
+
+ // TODO: Custom data types in partition pruning.
+ if (physicalType.spec() == NativeTypeSpec.UUID) {
+ return null;
+ }
+
+ Object val = getNodeValue(physicalType, node, dynamicParameters);
+
+ partitionCalculator.append(val);
+ }
+
+ int p = partitionCalculator.partition();
+ remainingPartitions.add(p);
+ }
+
+ return remainingPartitions;
+ }
+
+ @Nullable
+ private static Object getNodeValue(NativeType physicalType, RexNode node, Object[] dynamicParameters) {
+ Object val;
+
+ if (node instanceof RexLiteral) {
+ val = getValueFromLiteral(physicalType, (RexLiteral) node);
+ } else if (node instanceof RexDynamicParam) {
+ RexDynamicParam dynamicParam = (RexDynamicParam) node;
+ val = dynamicParameters[dynamicParam.getIndex()];
+ } else {
+ throw new IllegalArgumentException("Unexpected column value node: " + node);
+ }
+
+ return val;
+ }
+
+ @Nullable
+ private static Object getValueFromLiteral(NativeType physicalType, RexLiteral lit) {
+ if (physicalType.spec() == NativeTypeSpec.DATE) {
Review Comment:
This code can be rewritten by using expression compilation, but current implementation of `ExpressionFactory` uses `ExecutionContext` for no reason at all (it uses `TypeFactory` and `RowHandler`, if there are other dependencies I almost certain that they are also trivial).
It would be better to make `ExpressionFactory` independent of `ExecutionContext`.
--
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1492530647
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/mapping/MappedFragment.java:
##########
@@ -66,10 +71,30 @@ public class MappedFragment {
this.target = target;
}
+ /** Constructor. */
+ private MappedFragment(
+ Fragment fragment,
+ List<ColocationGroup> groups, List<String> nodes,
+ Long2ObjectMap<ColocationGroup> groupsBySourceId,
+ @Nullable Long2ObjectMap<List<String>> sourcesByExchangeId,
+ @Nullable ColocationGroup target
+ ) {
+ this.fragment = fragment;
+ this.nodes = List.copyOf(nodes);
+ this.groupsBySourceId = groupsBySourceId;
+ this.sourcesByExchangeId = sourcesByExchangeId;
Review Comment:
I think we should find a better name for `sourcesByExchangeId ` fields. Because it is not obvious that it stores node name (yes, it stores string which sorta-kinda looks like node names), sure having a `private final List<String> nodes` helps but still.
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/mapping/MappedFragment.java:
##########
@@ -66,10 +71,30 @@ public class MappedFragment {
this.target = target;
}
+ /** Constructor. */
+ private MappedFragment(
+ Fragment fragment,
+ List<ColocationGroup> groups, List<String> nodes,
+ Long2ObjectMap<ColocationGroup> groupsBySourceId,
+ @Nullable Long2ObjectMap<List<String>> sourcesByExchangeId,
+ @Nullable ColocationGroup target
+ ) {
+ this.fragment = fragment;
+ this.nodes = List.copyOf(nodes);
+ this.groupsBySourceId = groupsBySourceId;
+ this.sourcesByExchangeId = sourcesByExchangeId;
Review Comment:
I think we should find a better name for `sourcesByExchangeId ` field. Because it is not obvious that it stores node name (yes, it stores string which sorta-kinda looks like node names), sure having a `private final List<String> nodes` helps but still.
--
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1492530647
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/mapping/MappedFragment.java:
##########
@@ -66,10 +71,30 @@ public class MappedFragment {
this.target = target;
}
+ /** Constructor. */
+ private MappedFragment(
+ Fragment fragment,
+ List<ColocationGroup> groups, List<String> nodes,
+ Long2ObjectMap<ColocationGroup> groupsBySourceId,
+ @Nullable Long2ObjectMap<List<String>> sourcesByExchangeId,
+ @Nullable ColocationGroup target
+ ) {
+ this.fragment = fragment;
+ this.nodes = List.copyOf(nodes);
+ this.groupsBySourceId = groupsBySourceId;
+ this.sourcesByExchangeId = sourcesByExchangeId;
Review Comment:
I think we should find a better name for `sourcesByExchangeId ` field. Because it is not obvious that it stores node names (yes, it stores strings which sorta-kinda looks like node names), sure having a `private final List<String> nodes` helps but still.
--
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1491416347
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/NodeWithConsistencyToken.java:
##########
@@ -85,6 +84,6 @@ public int hashCode() {
/** {@inheritDoc} */
@Override
public String toString() {
- return S.toString(NodeWithConsistencyToken.class, this);
+ return name + ":" + enlistmentConsistencyToken;
Review Comment:
More concise representation, because `nodeName:1234567891011` is much more readable than `[NodeWithConsistencyToken [name=nodeName, enlistmentConsistencyToken=1234567891011] ]`.
The same was applied to `PartitionWithConsistencyToken`.
--
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1494636129
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/pruning/PartitionPruningMetadata.java:
##########
@@ -53,6 +55,6 @@ public Long2ObjectMap<PartitionPruningColumns> data() {
/** {@inheritDoc} */
@Override
public String toString() {
- return S.toString(this);
+ return data.toString();
Review Comment:
Replaced with String::toString(..., "collection", collection ) variants of S::toString, because contents of`it.unimi.dsi.fastutil` collections is displayed by 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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1491071264
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/mapping/MappingServiceImpl.java:
##########
@@ -147,7 +152,7 @@ private CompletableFuture<List<MappedFragment>> map0(MultiStepPlan multiStepPlan
return val;
});
- return cacheValue.mappedFragments;
+ return cacheValue.mappedFragments.thenApply(mappedFragments -> applyPartitionPruning(mappedFragments, parameters));
Review Comment:
I think when a query has no dynamic parameters, when can apply partition pruning to before adding fragments to cache.
And if there are dynamic parameters, we should apply partition pruning after retrieving fragments from the cache.
--
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "AMashenkov (via GitHub)" <gi...@apache.org>.
AMashenkov commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1494435502
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/mapping/QuerySplitter.java:
##########
@@ -188,12 +191,13 @@ public IgniteRel visit(IgniteTableScan rel) {
IgniteTable table = rel.getTable().unwrap(IgniteTable.class);
assert table != null;
+ long sourceId = idGenerator.nextId();
if (curr.seenRelations.add(table.id())) {
- curr.tables.add(table);
+ curr.tables.put(sourceId, table);
Review Comment:
If it was done intentionally, then what is the purpose of using Long2ObjectMap?
Adding cost O(n), `get` costs also O(n). It is worst alternative to a hash-based map and a list.
--
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1494439367
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/mapping/QuerySplitter.java:
##########
@@ -188,12 +191,13 @@ public IgniteRel visit(IgniteTableScan rel) {
IgniteTable table = rel.getTable().unwrap(IgniteTable.class);
assert table != null;
+ long sourceId = idGenerator.nextId();
if (curr.seenRelations.add(table.id())) {
- curr.tables.add(table);
+ curr.tables.put(sourceId, table);
Review Comment:
Thanks. fixed. Replaced with `Long2ObjectOpenHashMap`
--
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "AMashenkov (via GitHub)" <gi...@apache.org>.
AMashenkov commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1494466475
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/mapping/QuerySplitter.java:
##########
@@ -188,12 +191,13 @@ public IgniteRel visit(IgniteTableScan rel) {
IgniteTable table = rel.getTable().unwrap(IgniteTable.class);
assert table != null;
+ long sourceId = idGenerator.nextId();
if (curr.seenRelations.add(table.id())) {
- curr.tables.add(table);
+ curr.tables.put(sourceId, table);
Review Comment:
BTW, you missed the very first question. Should `if` clause be removed?
As I understand, you need a full `sourceID-> table` mapping.
The same table may occurs with more than once with different `sourceId`, but you will never (I guess) see `sourceId` duplicates, isn't it?
```
if (curr.seenRelations.add(table.id())) {
curr.tables.put(sourceId, table);
```
--
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1494475146
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/mapping/QuerySplitter.java:
##########
@@ -188,12 +191,13 @@ public IgniteRel visit(IgniteTableScan rel) {
IgniteTable table = rel.getTable().unwrap(IgniteTable.class);
assert table != null;
+ long sourceId = idGenerator.nextId();
if (curr.seenRelations.add(table.id())) {
- curr.tables.add(table);
+ curr.tables.put(sourceId, table);
Review Comment:
>you missed the very first question. Should if clause be removed?
No, it should not. The same table/view within a query have the same id.
--
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1492530647
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/mapping/MappedFragment.java:
##########
@@ -66,10 +71,30 @@ public class MappedFragment {
this.target = target;
}
+ /** Constructor. */
+ private MappedFragment(
+ Fragment fragment,
+ List<ColocationGroup> groups, List<String> nodes,
+ Long2ObjectMap<ColocationGroup> groupsBySourceId,
+ @Nullable Long2ObjectMap<List<String>> sourcesByExchangeId,
+ @Nullable ColocationGroup target
+ ) {
+ this.fragment = fragment;
+ this.nodes = List.copyOf(nodes);
+ this.groupsBySourceId = groupsBySourceId;
+ this.sourcesByExchangeId = sourcesByExchangeId;
Review Comment:
I think we should find a better name for `sourcesByExchangeId ` field. Because it is not obvious that it stores node names (yes, it stores strings which sorta-kinda looks like node names). Having a `private final List<String> nodes` helps a little bit, but still.
--
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1494225900
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/pruning/PartitionPruningPredicate.java:
##########
@@ -0,0 +1,201 @@
+/*
+ * 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.sql.engine.prepare.pruning;
+
+import static org.apache.ignite.internal.util.IgniteUtils.newHashMap;
+
+import it.unimi.dsi.fastutil.ints.Int2ObjectMap;
+import it.unimi.dsi.fastutil.ints.IntArraySet;
+import it.unimi.dsi.fastutil.ints.IntSet;
+import java.time.Instant;
+import java.time.LocalDate;
+import java.time.LocalDateTime;
+import java.time.LocalTime;
+import java.time.ZoneId;
+import java.util.ArrayList;
+import java.util.Calendar;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import org.apache.calcite.rex.RexDynamicParam;
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.util.ImmutableIntList;
+import org.apache.calcite.util.TimestampString;
+import org.apache.ignite.internal.sql.engine.exec.NodeWithConsistencyToken;
+import org.apache.ignite.internal.sql.engine.exec.PartitionWithConsistencyToken;
+import org.apache.ignite.internal.sql.engine.exec.mapping.ColocationGroup;
+import org.apache.ignite.internal.sql.engine.schema.IgniteTable;
+import org.apache.ignite.internal.sql.engine.schema.PartitionCalculator;
+import org.apache.ignite.internal.type.NativeType;
+import org.apache.ignite.internal.type.NativeTypeSpec;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Partition predicate encapsulates partition pruning logic.
+ */
+public final class PartitionPruningPredicate {
+
+ private static final ZoneId ZONE_ID_UTC = ZoneId.of("UTC");
+
+ private final int tablePartitions;
+
+ // TODO: https://issues.apache.org/jira/browse/IGNITE-21543 Remove after is resolved,
+ // remaining partitions should always be not null.
+ @Nullable
+ private final IntSet remainingPartitions;
+
+ /**
+ * Constructor.
+ *
+ * @param table Table.
+ * @param pruningColumns Columns.
+ * @param dynamicParameters Values of dynamic parameters.
+ */
+ public PartitionPruningPredicate(IgniteTable table, PartitionPruningColumns pruningColumns, Object[] dynamicParameters) {
+ this.tablePartitions = table.partitions();
+ this.remainingPartitions = computeRemainingPartitions(table, pruningColumns, dynamicParameters);
+ }
+
+ /**
+ * Applies partition pruning to the given colocation group. This group should have the same number of assignments as the source table.
+ *
+ * @param colocationGroup Colocation group.
+ *
+ * @return New colocation group.
+ */
+ public ColocationGroup prunePartitions(ColocationGroup colocationGroup) {
+ assert tablePartitions == colocationGroup.assignments().size() : "Number of partitions does not match";
+
+ if (remainingPartitions == null) {
+ return colocationGroup;
+ }
+
+ Map<String, List<PartitionWithConsistencyToken>> partitionsPerNode = newHashMap(colocationGroup.nodeNames().size());
+ Set<String> newNodes = new HashSet<>();
+
+ for (String nodeName : colocationGroup.nodeNames()) {
+ List<PartitionWithConsistencyToken> partsWithConsistencyTokens = new ArrayList<>();
+
+ for (int p = 0; p < colocationGroup.assignments().size(); p++) {
+ NodeWithConsistencyToken nodeWithConsistencyToken = colocationGroup.assignments().get(p);
+ if (!remainingPartitions.contains(p)) {
+ continue;
+ }
+
+ if (Objects.equals(nodeName, nodeWithConsistencyToken.name())) {
+ long t = nodeWithConsistencyToken.enlistmentConsistencyToken();
+
+ partsWithConsistencyTokens.add(new PartitionWithConsistencyToken(p, t));
+ newNodes.add(nodeName);
+ }
+ }
+
+ if (!partsWithConsistencyTokens.isEmpty()) {
+ partitionsPerNode.put(nodeName, partsWithConsistencyTokens);
+ }
+ }
+
+ // Keep assignments intact, because they are used by DestinationFactory.
+ return new ColocationGroup(
+ colocationGroup.sourceIds(),
+ List.copyOf(newNodes),
+ colocationGroup.assignments(),
+ partitionsPerNode
+ );
+ }
+
+ @Nullable
+ private static IntSet computeRemainingPartitions(
Review Comment:
Fixed
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/pruning/PartitionPruningColumns.java:
##########
@@ -56,7 +53,7 @@ public List<Int2ObjectMap<RexNode>> columns() {
/** {@inheritDoc} */
@Override
public String toString() {
- return S.toString(this);
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1492491267
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/pruning/PartitionPruningPredicate.java:
##########
@@ -0,0 +1,200 @@
+/*
+ * 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.sql.engine.prepare.pruning;
+
+import it.unimi.dsi.fastutil.ints.Int2ObjectMap;
+import it.unimi.dsi.fastutil.ints.IntArraySet;
+import it.unimi.dsi.fastutil.ints.IntSet;
+import java.time.Instant;
+import java.time.LocalDate;
+import java.time.LocalDateTime;
+import java.time.LocalTime;
+import java.time.ZoneId;
+import java.util.ArrayList;
+import java.util.Calendar;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import org.apache.calcite.rex.RexDynamicParam;
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.util.ImmutableIntList;
+import org.apache.calcite.util.TimestampString;
+import org.apache.ignite.internal.sql.engine.exec.NodeWithConsistencyToken;
+import org.apache.ignite.internal.sql.engine.exec.PartitionWithConsistencyToken;
+import org.apache.ignite.internal.sql.engine.exec.mapping.ColocationGroup;
+import org.apache.ignite.internal.sql.engine.schema.IgniteTable;
+import org.apache.ignite.internal.sql.engine.schema.PartitionCalculator;
+import org.apache.ignite.internal.type.NativeType;
+import org.apache.ignite.internal.type.NativeTypeSpec;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Partition predicate encapsulates partition pruning logic.
+ */
+public final class PartitionPruningPredicate {
+
+ private static final ZoneId ZONE_ID_UTC = ZoneId.of("UTC");
+
+ private final int tablePartitions;
+
+ // TODO: https://issues.apache.org/jira/browse/IGNITE-21543 Remove after is resolved,
+ // remaining partitions should always be not null.
+ @Nullable
+ private final IntSet remainingPartitions;
+
+ /**
+ * Constructor.
+ *
+ * @param table Table.
+ * @param pruningColumns Columns.
+ * @param dynamicParameters Values of dynamic parameters.
+ */
+ public PartitionPruningPredicate(IgniteTable table, PartitionPruningColumns pruningColumns, Object[] dynamicParameters) {
+ this.tablePartitions = table.partitions();
+ this.remainingPartitions = computeRemainingPartitions(table, pruningColumns, dynamicParameters);
+ }
+
+ /**
+ * Applies partition pruning to the given colocation group. This group should have the same number of assignments as the source table.
+ *
+ * @param colocationGroup Colocation group.
+ *
+ * @return New colocation group.
+ */
+ public ColocationGroup prunePartitions(ColocationGroup colocationGroup) {
+ assert tablePartitions == colocationGroup.assignments().size() : "Number of partitions does not match";
+
+ if (remainingPartitions == null) {
+ return colocationGroup;
+ }
+
+ Map<String, List<PartitionWithConsistencyToken>> partitionsPerNode = new HashMap<>(colocationGroup.nodeNames().size());
Review Comment:
Fixed.
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/pruning/PartitionPrunerImpl.java:
##########
@@ -0,0 +1,190 @@
+/*
+ * 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.sql.engine.prepare.pruning;
+
+import it.unimi.dsi.fastutil.longs.Long2ObjectArrayMap;
+import it.unimi.dsi.fastutil.longs.Long2ObjectMap;
+import it.unimi.dsi.fastutil.longs.Long2ObjectMap.Entry;
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.ignite.internal.sql.engine.exec.mapping.ColocationGroup;
+import org.apache.ignite.internal.sql.engine.exec.mapping.MappedFragment;
+import org.apache.ignite.internal.sql.engine.prepare.Fragment;
+import org.apache.ignite.internal.sql.engine.prepare.IgniteRelShuttle;
+import org.apache.ignite.internal.sql.engine.rel.IgniteReceiver;
+import org.apache.ignite.internal.sql.engine.rel.IgniteRel;
+import org.apache.ignite.internal.sql.engine.rel.IgniteSender;
+import org.apache.ignite.internal.sql.engine.schema.IgniteTable;
+import org.jetbrains.annotations.Nullable;
+
+/** Applies partition pruning. */
+public class PartitionPrunerImpl implements PartitionPruner {
+
+ /** Constructor. */
+ public PartitionPrunerImpl() {
+
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public List<MappedFragment> apply(
+ List<MappedFragment> mappedFragments,
+ Object[] dynamicParameters
+ ) {
+ List<MappedFragment> updatedFragments = new ArrayList<>(mappedFragments.size());
+ Long2ObjectMap<List<String>> newNodesByExchangeId = new Long2ObjectArrayMap<>();
+
+ // Partition pruning (PP). For each fragment:
+ //
+ // 1. Extract PP metadata from eac in the form of [colo_col1=<val>, ..] (see PartitionPruningMetadataExtractor)
Review Comment:
Updated the comment.
--
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "zstan (via GitHub)" <gi...@apache.org>.
zstan commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1492511455
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/mapping/MappedFragment.java:
##########
@@ -85,4 +110,45 @@ public Long2ObjectMap<ColocationGroup> groupsBySourceId() {
public @Nullable Long2ObjectMap<List<String>> sourcesByExchangeId() {
return sourcesByExchangeId;
}
+
+ /**
+ * Creates a fragment by replacing the given colocation groups.
+ *
+ * @param replacedGroups Groups to replace.
+ *
+ * @return New mapped fragment.
+ */
+ public MappedFragment replaceColocationGroups(Long2ObjectMap<ColocationGroup> replacedGroups) {
+ List<ColocationGroup> newGroups = new ArrayList<>();
Review Comment:
```suggestion
List<ColocationGroup> newGroups = new ArrayList<>(groupsBySourceId.long2ObjectEntrySet().size());
```
--
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "korlov42 (via GitHub)" <gi...@apache.org>.
korlov42 commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1493768157
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/pruning/PartitionPrunerImpl.java:
##########
@@ -0,0 +1,220 @@
+/*
+ * 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.sql.engine.prepare.pruning;
+
+import it.unimi.dsi.fastutil.longs.Long2ObjectArrayMap;
+import it.unimi.dsi.fastutil.longs.Long2ObjectMap;
+import it.unimi.dsi.fastutil.longs.Long2ObjectMap.Entry;
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.ignite.internal.sql.engine.exec.mapping.ColocationGroup;
+import org.apache.ignite.internal.sql.engine.exec.mapping.MappedFragment;
+import org.apache.ignite.internal.sql.engine.prepare.Fragment;
+import org.apache.ignite.internal.sql.engine.prepare.IgniteRelShuttle;
+import org.apache.ignite.internal.sql.engine.rel.IgniteReceiver;
+import org.apache.ignite.internal.sql.engine.rel.IgniteRel;
+import org.apache.ignite.internal.sql.engine.rel.IgniteSender;
+import org.apache.ignite.internal.sql.engine.schema.IgniteTable;
+import org.jetbrains.annotations.Nullable;
+
+/** Applies partition pruning. */
+public class PartitionPrunerImpl implements PartitionPruner {
+
+ /** Constructor. */
+ public PartitionPrunerImpl() {
+
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public List<MappedFragment> apply(
+ List<MappedFragment> mappedFragments,
+ Object[] dynamicParameters
+ ) {
+ List<MappedFragment> updatedFragments = new ArrayList<>(mappedFragments.size());
+ Long2ObjectMap<List<String>> newNodesByExchangeId = new Long2ObjectArrayMap<>();
+
+ // Partition pruning (PP). For each fragment:
+ //
+ // 1. Extract PP metadata from each fragment's root in the form of [colo_col1=<val>, ..] (see PartitionPruningMetadataExtractor)
+ //
+ // 2. If PP metadata exists then update fragment's colocation group
+ // to retain partition that are necessary to perform an operator (e.g. for a scan operator such
+ // partitions only include that ones that can contain data).
+ //
+ // Iterate over fragments again to update fragments that receive data from fragments updated at step 2.
+ // This is accomplished by updating `sourcesByExchangeId`.
+ //
+
+ PartitionPruningMetadataExtractor extractor = new PartitionPruningMetadataExtractor();
+
+ for (MappedFragment mappedFragment : mappedFragments) {
+ // Fragment that contains colocated operators has exactly one colocation group.
+ // Do not attempt to apply PP to other fragments.
Review Comment:
is there fundamental limitation to support PP for fragments containing more than a single group? If no, let's file ticket to add support for such fragments later
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/pruning/PartitionPruningColumns.java:
##########
@@ -56,7 +53,7 @@ public List<Int2ObjectMap<RexNode>> columns() {
/** {@inheritDoc} */
@Override
public String toString() {
- return S.toString(this);
Review Comment:
I think, it's better to keep `S.toString` to preserve uniformity. If for some reason you are not satisfied with its output, just use `ppc.columns().toString()` instead
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/NodeWithConsistencyToken.java:
##########
@@ -85,6 +84,6 @@ public int hashCode() {
/** {@inheritDoc} */
@Override
public String toString() {
- return S.toString(NodeWithConsistencyToken.class, this);
+ return name + ":" + enlistmentConsistencyToken;
Review Comment:
see comments above
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/pruning/PartitionPruningPredicate.java:
##########
@@ -0,0 +1,201 @@
+/*
+ * 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.sql.engine.prepare.pruning;
+
+import static org.apache.ignite.internal.util.IgniteUtils.newHashMap;
+
+import it.unimi.dsi.fastutil.ints.Int2ObjectMap;
+import it.unimi.dsi.fastutil.ints.IntArraySet;
+import it.unimi.dsi.fastutil.ints.IntSet;
+import java.time.Instant;
+import java.time.LocalDate;
+import java.time.LocalDateTime;
+import java.time.LocalTime;
+import java.time.ZoneId;
+import java.util.ArrayList;
+import java.util.Calendar;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import org.apache.calcite.rex.RexDynamicParam;
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.util.ImmutableIntList;
+import org.apache.calcite.util.TimestampString;
+import org.apache.ignite.internal.sql.engine.exec.NodeWithConsistencyToken;
+import org.apache.ignite.internal.sql.engine.exec.PartitionWithConsistencyToken;
+import org.apache.ignite.internal.sql.engine.exec.mapping.ColocationGroup;
+import org.apache.ignite.internal.sql.engine.schema.IgniteTable;
+import org.apache.ignite.internal.sql.engine.schema.PartitionCalculator;
+import org.apache.ignite.internal.type.NativeType;
+import org.apache.ignite.internal.type.NativeTypeSpec;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Partition predicate encapsulates partition pruning logic.
+ */
+public final class PartitionPruningPredicate {
+
+ private static final ZoneId ZONE_ID_UTC = ZoneId.of("UTC");
+
+ private final int tablePartitions;
+
+ // TODO: https://issues.apache.org/jira/browse/IGNITE-21543 Remove after is resolved,
+ // remaining partitions should always be not null.
+ @Nullable
+ private final IntSet remainingPartitions;
+
+ /**
+ * Constructor.
+ *
+ * @param table Table.
+ * @param pruningColumns Columns.
+ * @param dynamicParameters Values of dynamic parameters.
+ */
+ public PartitionPruningPredicate(IgniteTable table, PartitionPruningColumns pruningColumns, Object[] dynamicParameters) {
+ this.tablePartitions = table.partitions();
+ this.remainingPartitions = computeRemainingPartitions(table, pruningColumns, dynamicParameters);
+ }
+
+ /**
+ * Applies partition pruning to the given colocation group. This group should have the same number of assignments as the source table.
+ *
+ * @param colocationGroup Colocation group.
+ *
+ * @return New colocation group.
+ */
+ public ColocationGroup prunePartitions(ColocationGroup colocationGroup) {
+ assert tablePartitions == colocationGroup.assignments().size() : "Number of partitions does not match";
+
+ if (remainingPartitions == null) {
+ return colocationGroup;
+ }
+
+ Map<String, List<PartitionWithConsistencyToken>> partitionsPerNode = newHashMap(colocationGroup.nodeNames().size());
+ Set<String> newNodes = new HashSet<>();
+
+ for (String nodeName : colocationGroup.nodeNames()) {
+ List<PartitionWithConsistencyToken> partsWithConsistencyTokens = new ArrayList<>();
+
+ for (int p = 0; p < colocationGroup.assignments().size(); p++) {
+ NodeWithConsistencyToken nodeWithConsistencyToken = colocationGroup.assignments().get(p);
+ if (!remainingPartitions.contains(p)) {
+ continue;
+ }
+
+ if (Objects.equals(nodeName, nodeWithConsistencyToken.name())) {
+ long t = nodeWithConsistencyToken.enlistmentConsistencyToken();
+
+ partsWithConsistencyTokens.add(new PartitionWithConsistencyToken(p, t));
+ newNodes.add(nodeName);
+ }
+ }
+
+ if (!partsWithConsistencyTokens.isEmpty()) {
+ partitionsPerNode.put(nodeName, partsWithConsistencyTokens);
+ }
+ }
+
+ // Keep assignments intact, because they are used by DestinationFactory.
+ return new ColocationGroup(
+ colocationGroup.sourceIds(),
+ List.copyOf(newNodes),
+ colocationGroup.assignments(),
+ partitionsPerNode
+ );
+ }
+
+ @Nullable
+ private static IntSet computeRemainingPartitions(
Review Comment:
according to our [coding guidelines](https://google.github.io/styleguide/javaguide.html#s4.8.5-annotations) `Type-use annotations appear immediately before the annotated type`
```suggestion
private static @Nullable IntSet computeRemainingPartitions(
```
please fix it everywhere in the patch
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/pruning/PartitionPrunerImpl.java:
##########
@@ -0,0 +1,220 @@
+/*
+ * 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.sql.engine.prepare.pruning;
+
+import it.unimi.dsi.fastutil.longs.Long2ObjectArrayMap;
+import it.unimi.dsi.fastutil.longs.Long2ObjectMap;
+import it.unimi.dsi.fastutil.longs.Long2ObjectMap.Entry;
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.ignite.internal.sql.engine.exec.mapping.ColocationGroup;
+import org.apache.ignite.internal.sql.engine.exec.mapping.MappedFragment;
+import org.apache.ignite.internal.sql.engine.prepare.Fragment;
+import org.apache.ignite.internal.sql.engine.prepare.IgniteRelShuttle;
+import org.apache.ignite.internal.sql.engine.rel.IgniteReceiver;
+import org.apache.ignite.internal.sql.engine.rel.IgniteRel;
+import org.apache.ignite.internal.sql.engine.rel.IgniteSender;
+import org.apache.ignite.internal.sql.engine.schema.IgniteTable;
+import org.jetbrains.annotations.Nullable;
+
+/** Applies partition pruning. */
+public class PartitionPrunerImpl implements PartitionPruner {
+
+ /** Constructor. */
+ public PartitionPrunerImpl() {
Review Comment:
why do we need to specify default constructor explicitly?
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/pruning/PartitionPruningMetadata.java:
##########
@@ -53,6 +55,6 @@ public Long2ObjectMap<PartitionPruningColumns> data() {
/** {@inheritDoc} */
@Override
public String toString() {
- return S.toString(this);
+ return data.toString();
Review Comment:
the same, let's keep `S.toString(this)`
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/PartitionWithConsistencyToken.java:
##########
@@ -55,4 +57,29 @@ public int partId() {
public long enlistmentConsistencyToken() {
return enlistmentConsistencyToken;
}
+
+ /** {@inheritDoc} */
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ }
+ if (o == null || getClass() != o.getClass()) {
+ return false;
+ }
+ PartitionWithConsistencyToken that = (PartitionWithConsistencyToken) o;
+ return partId == that.partId && enlistmentConsistencyToken == that.enlistmentConsistencyToken;
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public int hashCode() {
+ return Objects.hash(partId, enlistmentConsistencyToken);
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public String toString() {
+ return partId + ":" + enlistmentConsistencyToken;
Review Comment:
see comments above
##########
modules/sql-engine/src/test/resources/mapping/test_partition_pruning.test:
##########
@@ -0,0 +1,53 @@
+# Pruning from a simple scan
+N1
+SELECT * FROM t1_n1n2n3 WHERE id = 0
Review Comment:
let's add one more test case to check pruning for collocated join
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/pruning/PartitionPruningPredicate.java:
##########
@@ -0,0 +1,201 @@
+/*
+ * 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.sql.engine.prepare.pruning;
+
+import static org.apache.ignite.internal.util.IgniteUtils.newHashMap;
+
+import it.unimi.dsi.fastutil.ints.Int2ObjectMap;
+import it.unimi.dsi.fastutil.ints.IntArraySet;
+import it.unimi.dsi.fastutil.ints.IntSet;
+import java.time.Instant;
+import java.time.LocalDate;
+import java.time.LocalDateTime;
+import java.time.LocalTime;
+import java.time.ZoneId;
+import java.util.ArrayList;
+import java.util.Calendar;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import org.apache.calcite.rex.RexDynamicParam;
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.util.ImmutableIntList;
+import org.apache.calcite.util.TimestampString;
+import org.apache.ignite.internal.sql.engine.exec.NodeWithConsistencyToken;
+import org.apache.ignite.internal.sql.engine.exec.PartitionWithConsistencyToken;
+import org.apache.ignite.internal.sql.engine.exec.mapping.ColocationGroup;
+import org.apache.ignite.internal.sql.engine.schema.IgniteTable;
+import org.apache.ignite.internal.sql.engine.schema.PartitionCalculator;
+import org.apache.ignite.internal.type.NativeType;
+import org.apache.ignite.internal.type.NativeTypeSpec;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Partition predicate encapsulates partition pruning logic.
+ */
+public final class PartitionPruningPredicate {
+
+ private static final ZoneId ZONE_ID_UTC = ZoneId.of("UTC");
+
+ private final int tablePartitions;
+
+ // TODO: https://issues.apache.org/jira/browse/IGNITE-21543 Remove after is resolved,
+ // remaining partitions should always be not null.
+ @Nullable
+ private final IntSet remainingPartitions;
+
+ /**
+ * Constructor.
+ *
+ * @param table Table.
+ * @param pruningColumns Columns.
+ * @param dynamicParameters Values of dynamic parameters.
+ */
+ public PartitionPruningPredicate(IgniteTable table, PartitionPruningColumns pruningColumns, Object[] dynamicParameters) {
+ this.tablePartitions = table.partitions();
+ this.remainingPartitions = computeRemainingPartitions(table, pruningColumns, dynamicParameters);
+ }
+
+ /**
+ * Applies partition pruning to the given colocation group. This group should have the same number of assignments as the source table.
+ *
+ * @param colocationGroup Colocation group.
+ *
+ * @return New colocation group.
+ */
+ public ColocationGroup prunePartitions(ColocationGroup colocationGroup) {
+ assert tablePartitions == colocationGroup.assignments().size() : "Number of partitions does not match";
+
+ if (remainingPartitions == null) {
+ return colocationGroup;
+ }
+
+ Map<String, List<PartitionWithConsistencyToken>> partitionsPerNode = newHashMap(colocationGroup.nodeNames().size());
+ Set<String> newNodes = new HashSet<>();
+
+ for (String nodeName : colocationGroup.nodeNames()) {
+ List<PartitionWithConsistencyToken> partsWithConsistencyTokens = new ArrayList<>();
+
+ for (int p = 0; p < colocationGroup.assignments().size(); p++) {
+ NodeWithConsistencyToken nodeWithConsistencyToken = colocationGroup.assignments().get(p);
+ if (!remainingPartitions.contains(p)) {
+ continue;
+ }
+
+ if (Objects.equals(nodeName, nodeWithConsistencyToken.name())) {
+ long t = nodeWithConsistencyToken.enlistmentConsistencyToken();
+
+ partsWithConsistencyTokens.add(new PartitionWithConsistencyToken(p, t));
+ newNodes.add(nodeName);
+ }
+ }
+
+ if (!partsWithConsistencyTokens.isEmpty()) {
+ partitionsPerNode.put(nodeName, partsWithConsistencyTokens);
+ }
+ }
+
+ // Keep assignments intact, because they are used by DestinationFactory.
+ return new ColocationGroup(
+ colocationGroup.sourceIds(),
+ List.copyOf(newNodes),
+ colocationGroup.assignments(),
+ partitionsPerNode
+ );
+ }
+
+ @Nullable
+ private static IntSet computeRemainingPartitions(
+ IgniteTable table,
+ PartitionPruningColumns pruningColumns,
+ Object[] dynamicParameters
+ ) {
+ ImmutableIntList keys = table.distribution().getKeys();
+ IntSet remainingPartitions = new IntArraySet(pruningColumns.columns().size());
+ PartitionCalculator partitionCalculator = table.partitionCalculator().get();
+
+ for (Int2ObjectMap<RexNode> columns : pruningColumns.columns()) {
+ for (int key : keys) {
+ RexNode node = columns.get(key);
+ NativeType physicalType = table.descriptor().columnDescriptor(key).physicalType();
+
+ // TODO: https://issues.apache.org/jira/browse/IGNITE-21543
+ // Remove after this issue makes it possible to have CAST('uuid_str' AS UUID) as value.
+ if (physicalType.spec() == NativeTypeSpec.UUID) {
Review Comment:
uuid still can be passed as dynamic param, right?
--
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "zstan (via GitHub)" <gi...@apache.org>.
zstan merged PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187
--
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1492472359
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/mapping/ColocationGroup.java:
##########
@@ -40,11 +42,27 @@ public class ColocationGroup implements Serializable {
private final List<NodeWithConsistencyToken> assignments;
+ private final Map<String, List<PartitionWithConsistencyToken>> partitionsPerNode;
+
/** Constructor. */
public ColocationGroup(List<Long> sourceIds, List<String> nodeNames, List<NodeWithConsistencyToken> assignments) {
this.sourceIds = Objects.requireNonNull(sourceIds, "sourceIds");
this.nodeNames = Objects.requireNonNull(nodeNames, "nodeNames");
this.assignments = Objects.requireNonNull(assignments, "assignments");
+ this.partitionsPerNode = null;
+ }
+
+ /** Constructor for colocation group with applied partition pruning. */
Review Comment:
`partitionsPerNode` is stored separately from `assignments` because `IgniteTrimExchange` creates a destination that expects the number of assignments as stored for a table (see DestinationFactory).
--
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1494636610
##########
modules/sql-engine/src/test/resources/mapping/test_partition_pruning.test:
##########
@@ -0,0 +1,53 @@
+# Pruning from a simple scan
+N1
+SELECT * FROM t1_n1n2n3 WHERE id = 0
Review Comment:
Done.
--
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1490679838
##########
modules/sql-engine/src/test/resources/mapping/test_partition_pruning.test:
##########
@@ -0,0 +1,50 @@
+# Pruning from a simple scan
+N1
+SELECT * FROM t1_n1n2n3 WHERE id = 0
+---
+Fragment#0 root
+ executionNodes: [N1]
+ remoteFragments: [1]
+ exchangeSourceNodes: {1=[N2]}
+ tree:
Review Comment:
Maybe we can include information regarding used partitions to this out?
--
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1491416347
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/NodeWithConsistencyToken.java:
##########
@@ -85,6 +84,6 @@ public int hashCode() {
/** {@inheritDoc} */
@Override
public String toString() {
- return S.toString(NodeWithConsistencyToken.class, this);
+ return name + ":" + enlistmentConsistencyToken;
Review Comment:
More concise representation, because `nodeName:1234567891011` is much more readable than `[NodeWithConsistencyToken [name=nodeName, enlistmentConsistencyToken=1234567891011] ]`.
The same was applied to `NodeWithConsistencyToken`.
--
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1491416347
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/NodeWithConsistencyToken.java:
##########
@@ -85,6 +84,6 @@ public int hashCode() {
/** {@inheritDoc} */
@Override
public String toString() {
- return S.toString(NodeWithConsistencyToken.class, this);
+ return name + ":" + enlistmentConsistencyToken;
Review Comment:
More concise representation, because `nodeName:1234567891011` is much more readable than `[NodeWithConsistencyToken [name=nodeName, enlistmentConsistencyToken=1234567891011] ]`.
--
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1492522078
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/mapping/MappedFragment.java:
##########
@@ -85,4 +110,45 @@ public Long2ObjectMap<ColocationGroup> groupsBySourceId() {
public @Nullable Long2ObjectMap<List<String>> sourcesByExchangeId() {
return sourcesByExchangeId;
}
+
+ /**
+ * Creates a fragment by replacing the given colocation groups.
+ *
+ * @param replacedGroups Groups to replace.
+ *
+ * @return New mapped fragment.
+ */
+ public MappedFragment replaceColocationGroups(Long2ObjectMap<ColocationGroup> replacedGroups) {
+ List<ColocationGroup> newGroups = new ArrayList<>();
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1494226611
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/pruning/PartitionPrunerImpl.java:
##########
@@ -0,0 +1,220 @@
+/*
+ * 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.sql.engine.prepare.pruning;
+
+import it.unimi.dsi.fastutil.longs.Long2ObjectArrayMap;
+import it.unimi.dsi.fastutil.longs.Long2ObjectMap;
+import it.unimi.dsi.fastutil.longs.Long2ObjectMap.Entry;
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.ignite.internal.sql.engine.exec.mapping.ColocationGroup;
+import org.apache.ignite.internal.sql.engine.exec.mapping.MappedFragment;
+import org.apache.ignite.internal.sql.engine.prepare.Fragment;
+import org.apache.ignite.internal.sql.engine.prepare.IgniteRelShuttle;
+import org.apache.ignite.internal.sql.engine.rel.IgniteReceiver;
+import org.apache.ignite.internal.sql.engine.rel.IgniteRel;
+import org.apache.ignite.internal.sql.engine.rel.IgniteSender;
+import org.apache.ignite.internal.sql.engine.schema.IgniteTable;
+import org.jetbrains.annotations.Nullable;
+
+/** Applies partition pruning. */
+public class PartitionPrunerImpl implements PartitionPruner {
+
+ /** Constructor. */
+ public PartitionPrunerImpl() {
+
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public List<MappedFragment> apply(
+ List<MappedFragment> mappedFragments,
+ Object[] dynamicParameters
+ ) {
+ List<MappedFragment> updatedFragments = new ArrayList<>(mappedFragments.size());
+ Long2ObjectMap<List<String>> newNodesByExchangeId = new Long2ObjectArrayMap<>();
+
+ // Partition pruning (PP). For each fragment:
+ //
+ // 1. Extract PP metadata from each fragment's root in the form of [colo_col1=<val>, ..] (see PartitionPruningMetadataExtractor)
+ //
+ // 2. If PP metadata exists then update fragment's colocation group
+ // to retain partition that are necessary to perform an operator (e.g. for a scan operator such
+ // partitions only include that ones that can contain data).
+ //
+ // Iterate over fragments again to update fragments that receive data from fragments updated at step 2.
+ // This is accomplished by updating `sourcesByExchangeId`.
+ //
+
+ PartitionPruningMetadataExtractor extractor = new PartitionPruningMetadataExtractor();
+
+ for (MappedFragment mappedFragment : mappedFragments) {
+ // Fragment that contains colocated operators has exactly one colocation group.
+ // Do not attempt to apply PP to other fragments.
Review Comment:
Removed. This is not important.
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/pruning/PartitionPrunerImpl.java:
##########
@@ -0,0 +1,220 @@
+/*
+ * 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.sql.engine.prepare.pruning;
+
+import it.unimi.dsi.fastutil.longs.Long2ObjectArrayMap;
+import it.unimi.dsi.fastutil.longs.Long2ObjectMap;
+import it.unimi.dsi.fastutil.longs.Long2ObjectMap.Entry;
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.ignite.internal.sql.engine.exec.mapping.ColocationGroup;
+import org.apache.ignite.internal.sql.engine.exec.mapping.MappedFragment;
+import org.apache.ignite.internal.sql.engine.prepare.Fragment;
+import org.apache.ignite.internal.sql.engine.prepare.IgniteRelShuttle;
+import org.apache.ignite.internal.sql.engine.rel.IgniteReceiver;
+import org.apache.ignite.internal.sql.engine.rel.IgniteRel;
+import org.apache.ignite.internal.sql.engine.rel.IgniteSender;
+import org.apache.ignite.internal.sql.engine.schema.IgniteTable;
+import org.jetbrains.annotations.Nullable;
+
+/** Applies partition pruning. */
+public class PartitionPrunerImpl implements PartitionPruner {
+
+ /** Constructor. */
+ public PartitionPrunerImpl() {
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1494358025
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/framework/TestBuilders.java:
##########
@@ -355,6 +358,9 @@ public interface TableBuilder extends TableBuilderBase<TableBuilder> {
/** Sets id for the table. The caller must guarantee that provided id is unique. */
TableBuilder tableId(int id);
+ /** Sets the number of partitions fot this table. default value is 1. */
Review Comment:
Thanks. 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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "AMashenkov (via GitHub)" <gi...@apache.org>.
AMashenkov commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1494435502
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/mapping/QuerySplitter.java:
##########
@@ -188,12 +191,13 @@ public IgniteRel visit(IgniteTableScan rel) {
IgniteTable table = rel.getTable().unwrap(IgniteTable.class);
assert table != null;
+ long sourceId = idGenerator.nextId();
if (curr.seenRelations.add(table.id())) {
- curr.tables.add(table);
+ curr.tables.put(sourceId, table);
Review Comment:
If so, then what is the purpose of using Long2ObjectMap?
Adding cost O(n), `get` costs also O(n). It is worst alternative to a hash-based map and a list.
--
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "zstan (via GitHub)" <gi...@apache.org>.
zstan commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1492408593
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/pruning/PartitionPrunerImpl.java:
##########
@@ -0,0 +1,190 @@
+/*
+ * 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.sql.engine.prepare.pruning;
+
+import it.unimi.dsi.fastutil.longs.Long2ObjectArrayMap;
+import it.unimi.dsi.fastutil.longs.Long2ObjectMap;
+import it.unimi.dsi.fastutil.longs.Long2ObjectMap.Entry;
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.ignite.internal.sql.engine.exec.mapping.ColocationGroup;
+import org.apache.ignite.internal.sql.engine.exec.mapping.MappedFragment;
+import org.apache.ignite.internal.sql.engine.prepare.Fragment;
+import org.apache.ignite.internal.sql.engine.prepare.IgniteRelShuttle;
+import org.apache.ignite.internal.sql.engine.rel.IgniteReceiver;
+import org.apache.ignite.internal.sql.engine.rel.IgniteRel;
+import org.apache.ignite.internal.sql.engine.rel.IgniteSender;
+import org.apache.ignite.internal.sql.engine.schema.IgniteTable;
+import org.jetbrains.annotations.Nullable;
+
+/** Applies partition pruning. */
+public class PartitionPrunerImpl implements PartitionPruner {
+
+ /** Constructor. */
+ public PartitionPrunerImpl() {
+
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public List<MappedFragment> apply(
+ List<MappedFragment> mappedFragments,
+ Object[] dynamicParameters
+ ) {
+ List<MappedFragment> updatedFragments = new ArrayList<>(mappedFragments.size());
+ Long2ObjectMap<List<String>> newNodesByExchangeId = new Long2ObjectArrayMap<>();
+
+ // Partition pruning (PP). For each fragment:
+ //
+ // 1. Extract PP metadata from eac in the form of [colo_col1=<val>, ..] (see PartitionPruningMetadataExtractor)
+ //
+ // 2. If PP metadata exists then update fragment's colocation group
+ // to retain partition that are necessary to perform an operator (e.g. for a scan operator such
+ // partitions only include that ones that can contain data).
+ //
+ // Iterate over fragments again to update fragments that receive data from fragments updated at step 2.
+ // This is accomplished by updating `sourcesByExchangeId`.
+ //
+
+ for (MappedFragment mappedFragment : mappedFragments) {
+ // Fragment that contains colocated operators has exactly one colocation group.
+ // Do not attempt to apply PP to other fragments.
+ if (mappedFragment.groups().size() != 1) {
+ updatedFragments.add(mappedFragment);
+ continue;
+ }
+
+ Fragment fragment = mappedFragment.fragment();
+ if (fragment.tables().isEmpty()) {
+ updatedFragments.add(mappedFragment);
+ continue;
+ }
+
+ PartitionPruningMetadataExtractor extractor = new PartitionPruningMetadataExtractor();
Review Comment:
seems you can move this initialization above **mappedFragments** loop and change a bit :
org.apache.ignite.internal.sql.engine.prepare.pruning.PartitionPruningMetadataExtractor#go(IgniteRel rel) {
result.clear(); <--
a bit help to GC ?
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/mapping/MappingServiceImpl.java:
##########
@@ -147,7 +152,7 @@ private CompletableFuture<List<MappedFragment>> map0(MultiStepPlan multiStepPlan
return val;
});
- return cacheValue.mappedFragments;
+ return cacheValue.mappedFragments.thenApply(mappedFragments -> applyPartitionPruning(mappedFragments, parameters));
Review Comment:
seems dynamic parameters need to be part of cache key ?
##########
.idea/codeStyles/Project.xml:
##########
@@ -42,7 +42,7 @@
</value>
</option>
</AndroidXmlCodeStyleSettings>
- <JSCodeStyleSettings>
+ <JSCodeStyleSettings version="0">
Review Comment:
seems you need to revert all this stuff ?
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/pruning/PartitionPruningPredicate.java:
##########
@@ -0,0 +1,200 @@
+/*
+ * 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.sql.engine.prepare.pruning;
+
+import it.unimi.dsi.fastutil.ints.Int2ObjectMap;
+import it.unimi.dsi.fastutil.ints.IntArraySet;
+import it.unimi.dsi.fastutil.ints.IntSet;
+import java.time.Instant;
+import java.time.LocalDate;
+import java.time.LocalDateTime;
+import java.time.LocalTime;
+import java.time.ZoneId;
+import java.util.ArrayList;
+import java.util.Calendar;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import org.apache.calcite.rex.RexDynamicParam;
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.util.ImmutableIntList;
+import org.apache.calcite.util.TimestampString;
+import org.apache.ignite.internal.sql.engine.exec.NodeWithConsistencyToken;
+import org.apache.ignite.internal.sql.engine.exec.PartitionWithConsistencyToken;
+import org.apache.ignite.internal.sql.engine.exec.mapping.ColocationGroup;
+import org.apache.ignite.internal.sql.engine.schema.IgniteTable;
+import org.apache.ignite.internal.sql.engine.schema.PartitionCalculator;
+import org.apache.ignite.internal.type.NativeType;
+import org.apache.ignite.internal.type.NativeTypeSpec;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Partition predicate encapsulates partition pruning logic.
+ */
+public final class PartitionPruningPredicate {
+
+ private static final ZoneId ZONE_ID_UTC = ZoneId.of("UTC");
+
+ private final int tablePartitions;
+
+ // TODO: https://issues.apache.org/jira/browse/IGNITE-21543 Remove after is resolved,
+ // remaining partitions should always be not null.
+ @Nullable
+ private final IntSet remainingPartitions;
+
+ /**
+ * Constructor.
+ *
+ * @param table Table.
+ * @param pruningColumns Columns.
+ * @param dynamicParameters Values of dynamic parameters.
+ */
+ public PartitionPruningPredicate(IgniteTable table, PartitionPruningColumns pruningColumns, Object[] dynamicParameters) {
+ this.tablePartitions = table.partitions();
+ this.remainingPartitions = computeRemainingPartitions(table, pruningColumns, dynamicParameters);
+ }
+
+ /**
+ * Applies partition pruning to the given colocation group. This group should have the same number of assignments as the source table.
+ *
+ * @param colocationGroup Colocation group.
+ *
+ * @return New colocation group.
+ */
+ public ColocationGroup prunePartitions(ColocationGroup colocationGroup) {
+ assert tablePartitions == colocationGroup.assignments().size() : "Number of partitions does not match";
+
+ if (remainingPartitions == null) {
+ return colocationGroup;
+ }
+
+ Map<String, List<PartitionWithConsistencyToken>> partitionsPerNode = new HashMap<>(colocationGroup.nodeNames().size());
Review Comment:
org.apache.ignite.internal.util.IgniteUtils#newHashMap
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/pruning/PartitionPrunerImpl.java:
##########
@@ -0,0 +1,190 @@
+/*
+ * 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.sql.engine.prepare.pruning;
+
+import it.unimi.dsi.fastutil.longs.Long2ObjectArrayMap;
+import it.unimi.dsi.fastutil.longs.Long2ObjectMap;
+import it.unimi.dsi.fastutil.longs.Long2ObjectMap.Entry;
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.ignite.internal.sql.engine.exec.mapping.ColocationGroup;
+import org.apache.ignite.internal.sql.engine.exec.mapping.MappedFragment;
+import org.apache.ignite.internal.sql.engine.prepare.Fragment;
+import org.apache.ignite.internal.sql.engine.prepare.IgniteRelShuttle;
+import org.apache.ignite.internal.sql.engine.rel.IgniteReceiver;
+import org.apache.ignite.internal.sql.engine.rel.IgniteRel;
+import org.apache.ignite.internal.sql.engine.rel.IgniteSender;
+import org.apache.ignite.internal.sql.engine.schema.IgniteTable;
+import org.jetbrains.annotations.Nullable;
+
+/** Applies partition pruning. */
+public class PartitionPrunerImpl implements PartitionPruner {
+
+ /** Constructor. */
+ public PartitionPrunerImpl() {
+
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public List<MappedFragment> apply(
+ List<MappedFragment> mappedFragments,
+ Object[] dynamicParameters
+ ) {
+ List<MappedFragment> updatedFragments = new ArrayList<>(mappedFragments.size());
+ Long2ObjectMap<List<String>> newNodesByExchangeId = new Long2ObjectArrayMap<>();
+
+ // Partition pruning (PP). For each fragment:
+ //
+ // 1. Extract PP metadata from eac in the form of [colo_col1=<val>, ..] (see PartitionPruningMetadataExtractor)
Review Comment:
what does "eac" mean?
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/pruning/PartitionPruningColumns.java:
##########
@@ -56,7 +55,7 @@ public List<Int2ObjectMap<RexNode>> columns() {
/** {@inheritDoc} */
@Override
public String toString() {
- return S.toString(this);
+ return columns.toString();
Review Comment:
if you need such a change seems annotation @IgniteToStringInclude need to be removed too ? Also i see that @TestOnly method can be moved directly to tests ? but it`s up to you ..
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/pruning/PartitionPrunerImpl.java:
##########
@@ -0,0 +1,190 @@
+/*
+ * 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.sql.engine.prepare.pruning;
+
+import it.unimi.dsi.fastutil.longs.Long2ObjectArrayMap;
+import it.unimi.dsi.fastutil.longs.Long2ObjectMap;
+import it.unimi.dsi.fastutil.longs.Long2ObjectMap.Entry;
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.ignite.internal.sql.engine.exec.mapping.ColocationGroup;
+import org.apache.ignite.internal.sql.engine.exec.mapping.MappedFragment;
+import org.apache.ignite.internal.sql.engine.prepare.Fragment;
+import org.apache.ignite.internal.sql.engine.prepare.IgniteRelShuttle;
+import org.apache.ignite.internal.sql.engine.rel.IgniteReceiver;
+import org.apache.ignite.internal.sql.engine.rel.IgniteRel;
+import org.apache.ignite.internal.sql.engine.rel.IgniteSender;
+import org.apache.ignite.internal.sql.engine.schema.IgniteTable;
+import org.jetbrains.annotations.Nullable;
+
+/** Applies partition pruning. */
+public class PartitionPrunerImpl implements PartitionPruner {
+
+ /** Constructor. */
+ public PartitionPrunerImpl() {
+
+ }
+
+ /** {@inheritDoc} */
+ @Override
+ public List<MappedFragment> apply(
Review Comment:
as i can see this class is not holding any values and all methods can be static ? for test cases it can be easily changed too, isn`t it ?
--
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1491416347
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/NodeWithConsistencyToken.java:
##########
@@ -85,6 +84,6 @@ public int hashCode() {
/** {@inheritDoc} */
@Override
public String toString() {
- return S.toString(NodeWithConsistencyToken.class, this);
+ return name + ":" + enlistmentConsistencyToken;
Review Comment:
More concise representation, because `nodeName: 123456789` is much more readable than `[NodeWithConsistencyToken [name=nodeName, enlistmentConsistencyToken=123456789] ]`.
--
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1492472359
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/mapping/ColocationGroup.java:
##########
@@ -40,11 +42,27 @@ public class ColocationGroup implements Serializable {
private final List<NodeWithConsistencyToken> assignments;
+ private final Map<String, List<PartitionWithConsistencyToken>> partitionsPerNode;
+
/** Constructor. */
public ColocationGroup(List<Long> sourceIds, List<String> nodeNames, List<NodeWithConsistencyToken> assignments) {
this.sourceIds = Objects.requireNonNull(sourceIds, "sourceIds");
this.nodeNames = Objects.requireNonNull(nodeNames, "nodeNames");
this.assignments = Objects.requireNonNull(assignments, "assignments");
+ this.partitionsPerNode = null;
+ }
+
+ /** Constructor for colocation group with applied partition pruning. */
Review Comment:
This is stored separately because `IgniteTrimExchange` creates a destination that expects the number of assignments as stored for a table (see DestinationFactory).
--
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1492530647
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/mapping/MappedFragment.java:
##########
@@ -66,10 +71,30 @@ public class MappedFragment {
this.target = target;
}
+ /** Constructor. */
+ private MappedFragment(
+ Fragment fragment,
+ List<ColocationGroup> groups, List<String> nodes,
+ Long2ObjectMap<ColocationGroup> groupsBySourceId,
+ @Nullable Long2ObjectMap<List<String>> sourcesByExchangeId,
+ @Nullable ColocationGroup target
+ ) {
+ this.fragment = fragment;
+ this.nodes = List.copyOf(nodes);
+ this.groupsBySourceId = groupsBySourceId;
+ this.sourcesByExchangeId = sourcesByExchangeId;
Review Comment:
I think we should find a better name for `sourcesByExchangeId ` fields. Because it is not obvious that it stores node name (yes, it stores string which sorta-kinda looks like node names) but still.
--
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1491404103
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/pruning/PartitionPruningPredicate.java:
##########
@@ -0,0 +1,197 @@
+/*
+ * 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.sql.engine.prepare.pruning;
+
+import it.unimi.dsi.fastutil.ints.Int2ObjectMap;
+import it.unimi.dsi.fastutil.ints.IntArraySet;
+import it.unimi.dsi.fastutil.ints.IntSet;
+import java.time.Instant;
+import java.time.LocalDate;
+import java.time.LocalDateTime;
+import java.time.LocalTime;
+import java.time.ZoneId;
+import java.util.ArrayList;
+import java.util.Calendar;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import org.apache.calcite.rex.RexDynamicParam;
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.util.ImmutableIntList;
+import org.apache.calcite.util.TimestampString;
+import org.apache.ignite.internal.sql.engine.exec.NodeWithConsistencyToken;
+import org.apache.ignite.internal.sql.engine.exec.PartitionWithConsistencyToken;
+import org.apache.ignite.internal.sql.engine.exec.mapping.ColocationGroup;
+import org.apache.ignite.internal.sql.engine.schema.IgniteTable;
+import org.apache.ignite.internal.sql.engine.schema.PartitionCalculator;
+import org.apache.ignite.internal.type.NativeType;
+import org.apache.ignite.internal.type.NativeTypeSpec;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Partition predicate encapsulates partition pruning logic.
+ */
+public final class PartitionPruningPredicate {
+
+ private static final ZoneId ZONE_ID_UTC = ZoneId.of("UTC");
+
+ private final int tablePartitions;
+
+ @Nullable
+ private final IntSet remainingPartitions;
+
+ /**
+ * Constructor.
+ *
+ * @param table Table.
+ * @param pruningColumns Columns.
+ * @param dynamicParameters Values of dynamic parameters.
+ */
+ public PartitionPruningPredicate(IgniteTable table, PartitionPruningColumns pruningColumns, Object[] dynamicParameters) {
+ this.tablePartitions = table.partitions();
+ this.remainingPartitions = computeRemainingPartitions(table, pruningColumns, dynamicParameters);
+ }
+
+ /**
+ * Applies partition pruning to the given colocation group. This group should have the same number of assignments as the source table.
+ *
+ * @param colocationGroup Colocation group.
+ *
+ * @return New colocation group.
+ */
+ public ColocationGroup prunePartitions(ColocationGroup colocationGroup) {
+ assert tablePartitions == colocationGroup.assignments().size() : "Number of partitions does not match";
+
+ if (remainingPartitions == null) {
+ return colocationGroup;
+ }
+
+ Map<String, List<PartitionWithConsistencyToken>> partitionsPerNode = new HashMap<>(colocationGroup.nodeNames().size());
+ Set<String> newNodes = new HashSet<>();
+
+ for (String nodeName : colocationGroup.nodeNames()) {
+ List<PartitionWithConsistencyToken> partsWithConsistencyTokens = new ArrayList<>();
+
+ for (int p = 0; p < colocationGroup.assignments().size(); p++) {
+ NodeWithConsistencyToken nodeWithConsistencyToken = colocationGroup.assignments().get(p);
+ if (!remainingPartitions.contains(p)) {
+ continue;
+ }
+
+ if (Objects.equals(nodeName, nodeWithConsistencyToken.name())) {
+ long t = nodeWithConsistencyToken.enlistmentConsistencyToken();
+
+ partsWithConsistencyTokens.add(new PartitionWithConsistencyToken(p, t));
+ newNodes.add(nodeName);
+ }
+ }
+
+ if (!partsWithConsistencyTokens.isEmpty()) {
+ partitionsPerNode.put(nodeName, partsWithConsistencyTokens);
+ }
+ }
+
+ // Keep assignments intact, because they are used by DestinationFactory.
+ return new ColocationGroup(
+ colocationGroup.sourceIds(),
+ List.copyOf(newNodes),
+ colocationGroup.assignments(),
+ partitionsPerNode
+ );
+ }
+
+ @Nullable
+ private static IntSet computeRemainingPartitions(
+ IgniteTable table,
+ PartitionPruningColumns pruningColumns,
+ Object[] dynamicParameters
+ ) {
+ ImmutableIntList keys = table.distribution().getKeys();
+ IntSet remainingPartitions = new IntArraySet(pruningColumns.columns().size());
+ PartitionCalculator partitionCalculator = table.partitionCalculator().get();
+
+ for (Int2ObjectMap<RexNode> columns : pruningColumns.columns()) {
+ for (int key : keys) {
+ RexNode node = columns.get(key);
+ NativeType physicalType = table.descriptor().columnDescriptor(key).physicalType();
+
+ // TODO: Custom data types in partition pruning.
+ if (physicalType.spec() == NativeTypeSpec.UUID) {
+ return null;
+ }
+
+ Object val = getNodeValue(physicalType, node, dynamicParameters);
+
+ partitionCalculator.append(val);
+ }
+
+ int p = partitionCalculator.partition();
+ remainingPartitions.add(p);
+ }
+
+ return remainingPartitions;
+ }
+
+ @Nullable
+ private static Object getNodeValue(NativeType physicalType, RexNode node, Object[] dynamicParameters) {
+ Object val;
+
+ if (node instanceof RexLiteral) {
+ val = getValueFromLiteral(physicalType, (RexLiteral) node);
+ } else if (node instanceof RexDynamicParam) {
+ RexDynamicParam dynamicParam = (RexDynamicParam) node;
+ val = dynamicParameters[dynamicParam.getIndex()];
+ } else {
+ throw new IllegalArgumentException("Unexpected column value node: " + node);
+ }
+
+ return val;
+ }
+
+ @Nullable
+ private static Object getValueFromLiteral(NativeType physicalType, RexLiteral lit) {
+ if (physicalType.spec() == NativeTypeSpec.DATE) {
Review Comment:
This code (and the whole `getNodeValue` ) can be rewritten by using expression compilation, but current implementation of `ExpressionFactory` uses `ExecutionContext` for no reason at all (it uses `TypeFactory` and `RowHandler`, if there are other dependencies I almost certain that they are also trivial).
It would be better to make `ExpressionFactory` independent of `ExecutionContext`.
--
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1494197597
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/pruning/PartitionPruningPredicate.java:
##########
@@ -0,0 +1,201 @@
+/*
+ * 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.sql.engine.prepare.pruning;
+
+import static org.apache.ignite.internal.util.IgniteUtils.newHashMap;
+
+import it.unimi.dsi.fastutil.ints.Int2ObjectMap;
+import it.unimi.dsi.fastutil.ints.IntArraySet;
+import it.unimi.dsi.fastutil.ints.IntSet;
+import java.time.Instant;
+import java.time.LocalDate;
+import java.time.LocalDateTime;
+import java.time.LocalTime;
+import java.time.ZoneId;
+import java.util.ArrayList;
+import java.util.Calendar;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import org.apache.calcite.rex.RexDynamicParam;
+import org.apache.calcite.rex.RexLiteral;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.util.ImmutableIntList;
+import org.apache.calcite.util.TimestampString;
+import org.apache.ignite.internal.sql.engine.exec.NodeWithConsistencyToken;
+import org.apache.ignite.internal.sql.engine.exec.PartitionWithConsistencyToken;
+import org.apache.ignite.internal.sql.engine.exec.mapping.ColocationGroup;
+import org.apache.ignite.internal.sql.engine.schema.IgniteTable;
+import org.apache.ignite.internal.sql.engine.schema.PartitionCalculator;
+import org.apache.ignite.internal.type.NativeType;
+import org.apache.ignite.internal.type.NativeTypeSpec;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * Partition predicate encapsulates partition pruning logic.
+ */
+public final class PartitionPruningPredicate {
+
+ private static final ZoneId ZONE_ID_UTC = ZoneId.of("UTC");
+
+ private final int tablePartitions;
+
+ // TODO: https://issues.apache.org/jira/browse/IGNITE-21543 Remove after is resolved,
+ // remaining partitions should always be not null.
+ @Nullable
+ private final IntSet remainingPartitions;
+
+ /**
+ * Constructor.
+ *
+ * @param table Table.
+ * @param pruningColumns Columns.
+ * @param dynamicParameters Values of dynamic parameters.
+ */
+ public PartitionPruningPredicate(IgniteTable table, PartitionPruningColumns pruningColumns, Object[] dynamicParameters) {
+ this.tablePartitions = table.partitions();
+ this.remainingPartitions = computeRemainingPartitions(table, pruningColumns, dynamicParameters);
+ }
+
+ /**
+ * Applies partition pruning to the given colocation group. This group should have the same number of assignments as the source table.
+ *
+ * @param colocationGroup Colocation group.
+ *
+ * @return New colocation group.
+ */
+ public ColocationGroup prunePartitions(ColocationGroup colocationGroup) {
+ assert tablePartitions == colocationGroup.assignments().size() : "Number of partitions does not match";
+
+ if (remainingPartitions == null) {
+ return colocationGroup;
+ }
+
+ Map<String, List<PartitionWithConsistencyToken>> partitionsPerNode = newHashMap(colocationGroup.nodeNames().size());
+ Set<String> newNodes = new HashSet<>();
+
+ for (String nodeName : colocationGroup.nodeNames()) {
+ List<PartitionWithConsistencyToken> partsWithConsistencyTokens = new ArrayList<>();
+
+ for (int p = 0; p < colocationGroup.assignments().size(); p++) {
+ NodeWithConsistencyToken nodeWithConsistencyToken = colocationGroup.assignments().get(p);
+ if (!remainingPartitions.contains(p)) {
+ continue;
+ }
+
+ if (Objects.equals(nodeName, nodeWithConsistencyToken.name())) {
+ long t = nodeWithConsistencyToken.enlistmentConsistencyToken();
+
+ partsWithConsistencyTokens.add(new PartitionWithConsistencyToken(p, t));
+ newNodes.add(nodeName);
+ }
+ }
+
+ if (!partsWithConsistencyTokens.isEmpty()) {
+ partitionsPerNode.put(nodeName, partsWithConsistencyTokens);
+ }
+ }
+
+ // Keep assignments intact, because they are used by DestinationFactory.
+ return new ColocationGroup(
+ colocationGroup.sourceIds(),
+ List.copyOf(newNodes),
+ colocationGroup.assignments(),
+ partitionsPerNode
+ );
+ }
+
+ @Nullable
+ private static IntSet computeRemainingPartitions(
+ IgniteTable table,
+ PartitionPruningColumns pruningColumns,
+ Object[] dynamicParameters
+ ) {
+ ImmutableIntList keys = table.distribution().getKeys();
+ IntSet remainingPartitions = new IntArraySet(pruningColumns.columns().size());
+ PartitionCalculator partitionCalculator = table.partitionCalculator().get();
+
+ for (Int2ObjectMap<RexNode> columns : pruningColumns.columns()) {
+ for (int key : keys) {
+ RexNode node = columns.get(key);
+ NativeType physicalType = table.descriptor().columnDescriptor(key).physicalType();
+
+ // TODO: https://issues.apache.org/jira/browse/IGNITE-21543
+ // Remove after this issue makes it possible to have CAST('uuid_str' AS UUID) as value.
+ if (physicalType.spec() == NativeTypeSpec.UUID) {
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "zstan (via GitHub)" <gi...@apache.org>.
zstan commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1494112071
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/framework/TestBuilders.java:
##########
@@ -355,6 +358,9 @@ public interface TableBuilder extends TableBuilderBase<TableBuilder> {
/** Sets id for the table. The caller must guarantee that provided id is unique. */
TableBuilder tableId(int id);
+ /** Sets the number of partitions fot this table. default value is 1. */
Review Comment:
```suggestion
/** Sets the number of partitions for this table. default value is 1. */
```
--
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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1499077279
##########
modules/sql-engine/src/test/resources/mapping/test_partition_pruning.test:
##########
@@ -0,0 +1,53 @@
+# Pruning from a simple scan
+N1
+SELECT * FROM t1_n1n2n3 WHERE id = 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
Re: [PR] IGNITE-21279: Sql. Partition pruning. Integrate static partition pruning into READ statements execution pipeline [ignite-3]
Posted by "lowka (via GitHub)" <gi...@apache.org>.
lowka commented on code in PR #3187:
URL: https://github.com/apache/ignite-3/pull/3187#discussion_r1494475146
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/mapping/QuerySplitter.java:
##########
@@ -188,12 +191,13 @@ public IgniteRel visit(IgniteTableScan rel) {
IgniteTable table = rel.getTable().unwrap(IgniteTable.class);
assert table != null;
+ long sourceId = idGenerator.nextId();
if (curr.seenRelations.add(table.id())) {
- curr.tables.add(table);
+ curr.tables.put(sourceId, table);
Review Comment:
>you missed the very first question. Should if clause be removed?
No, it should not. The same table/view within query have the same id.
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/mapping/QuerySplitter.java:
##########
@@ -188,12 +191,13 @@ public IgniteRel visit(IgniteTableScan rel) {
IgniteTable table = rel.getTable().unwrap(IgniteTable.class);
assert table != null;
+ long sourceId = idGenerator.nextId();
if (curr.seenRelations.add(table.id())) {
- curr.tables.add(table);
+ curr.tables.put(sourceId, table);
Review Comment:
>you missed the very first question. Should if clause be removed?
No, it should not. The same table/view within query have the same id.
--
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