korlov42 commented on code in PR #7703:
URL: https://github.com/apache/ignite-3/pull/7703#discussion_r2938571974


##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/pruning/ModifyNodeVisitor.java:
##########
@@ -73,7 +85,19 @@
 class ModifyNodeVisitor implements IgniteRelVisitor<List<List<RexNode>>> {
     private static final IgniteLogger LOG = 
Loggers.forClass(ModifyNodeVisitor.class);
 
-    private ModifyNodeVisitor() {
+    /**
+     * A placeholder used to designate that there is no value for a column. If 
PP metadata collection algorithm encounters this placeholder
+     * in a colocation column for a source, then there can be no PP metadata 
for this source. Since this is a marker to terminate 
+     * algorithm, the type chosen for this expression does not matter.
+     */
+    static final RexNode VALUE_NOT_ASSIGNED = new RexInputRef(0, 
Commons.typeFactory().createSqlType(SqlTypeName.NULL));
+
+    private final PartitionPruningMetadataExtractor extractor;
+
+    private Pair<IgniteTable, Operation> modifiedTable;

Review Comment:
   let's split this pair and make fields final



##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/pruning/ModifyNodeVisitor.java:
##########
@@ -332,4 +378,113 @@ public RexNode visitInputRef(RexInputRef inputRef) {
 
         return null;
     }
+
+    private @Nullable List<List<RexNode>> extractValuesFromScan(
+            IgniteRel rel,
+            long sourceId,
+            IgniteTable table,
+            @Nullable ImmutableIntList requiredColumns,
+            @Nullable List<RexNode> projects
+    ) {
+
+        PartitionPruningColumns metadata = extractor.result.get(sourceId);
+        // Do not propagate metadata if tables are different for now.
+        IgniteTable tableUnderModification = modifiedTable.getFirst();
+        if (metadata == null || tableUnderModification.id() != table.id()) {
+            return null;
+        }
+
+        // Only handle column order-preserving projections for now.
+        boolean preserveOrder = requiredColumns != null && 
projectionPreservesColumnOrder(requiredColumns);

Review Comment:
   if required columns is null then we won't extract anything, right?



##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/pruning/ModifyNodeVisitor.java:
##########
@@ -332,4 +378,113 @@ public RexNode visitInputRef(RexInputRef inputRef) {
 
         return null;
     }
+
+    private @Nullable List<List<RexNode>> extractValuesFromScan(
+            IgniteRel rel,
+            long sourceId,
+            IgniteTable table,
+            @Nullable ImmutableIntList requiredColumns,
+            @Nullable List<RexNode> projects
+    ) {
+
+        PartitionPruningColumns metadata = extractor.result.get(sourceId);
+        // Do not propagate metadata if tables are different for now.
+        IgniteTable tableUnderModification = modifiedTable.getFirst();
+        if (metadata == null || tableUnderModification.id() != table.id()) {
+            return null;
+        }
+
+        // Only handle column order-preserving projections for now.
+        boolean preserveOrder = requiredColumns != null && 
projectionPreservesColumnOrder(requiredColumns);
+        if (!preserveOrder) {
+            return null;
+        }
+
+        if (!nullOrEmpty(projects)) {
+            assert requiredColumns != null : "No required columns: " + rel;
+
+            List<RexNode> expectedProjection = 
createProjectionFromScan(modifiedTable.getSecond(), projects, requiredColumns);
+            RelDataType rowType = 
table.getRowType(rel.getCluster().getTypeFactory(), requiredColumns);
+
+            if (!RexUtils.isIdentity(expectedProjection, rowType)) {

Review Comment:
   this limitation is too strict. We are interested in colocation columns only 
and we don't care about the rest. Moreover, all the test which were added for 
INSERT scenario doesn't make any sense, because you can't insert two identical 
keys. Such queries always end up in PK violation 



##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/prepare/pruning/ModifyNodeVisitor.java:
##########
@@ -332,4 +378,113 @@ public RexNode visitInputRef(RexInputRef inputRef) {
 
         return null;
     }
+
+    private @Nullable List<List<RexNode>> extractValuesFromScan(
+            IgniteRel rel,
+            long sourceId,
+            IgniteTable table,
+            @Nullable ImmutableIntList requiredColumns,
+            @Nullable List<RexNode> projects
+    ) {
+
+        PartitionPruningColumns metadata = extractor.result.get(sourceId);
+        // Do not propagate metadata if tables are different for now.
+        IgniteTable tableUnderModification = modifiedTable.getFirst();
+        if (metadata == null || tableUnderModification.id() != table.id()) {
+            return null;
+        }
+
+        // Only handle column order-preserving projections for now.
+        boolean preserveOrder = requiredColumns != null && 
projectionPreservesColumnOrder(requiredColumns);
+        if (!preserveOrder) {
+            return null;
+        }
+
+        if (!nullOrEmpty(projects)) {
+            assert requiredColumns != null : "No required columns: " + rel;
+
+            List<RexNode> expectedProjection = 
createProjectionFromScan(modifiedTable.getSecond(), projects, requiredColumns);
+            RelDataType rowType = 
table.getRowType(rel.getCluster().getTypeFactory(), requiredColumns);
+
+            if (!RexUtils.isIdentity(expectedProjection, rowType)) {
+                return null;
+            }
+        }
+
+        return metadataToValues(table, metadata);
+    }
+
+    private static boolean projectionPreservesColumnOrder(ImmutableIntList 
requiredColumns) {

Review Comment:
   name of the method is misleading. Currently, it checks not only order is 
preserved, but also that `requiredColumns` is prefix with no gaps



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to