Re: [PR] IGNITE-26138 Sql. Partition awareness. Prepare partition pruning metadata after planning [ignite-3]

[via GitHub]

lowka commented on code in PR #6374:
URL: https://github.com/apache/ignite-3/pull/6374#discussion_r2269369914


##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/mapping/MappingServiceImpl.java:
##########
@@ -360,14 +362,21 @@ private static ExecutionTarget 
buildTargetForSystemView(ExecutionTargetFactory f
                 : factory.allOf(nodes);
     }
 
-    private List<MappedFragment> applyPartitionPruning(List<MappedFragment> 
mappedFragments, MappingParameters parameters) {
-        return partitionPruner.apply(mappedFragments, 
parameters.dynamicParameters());
+    private List<MappedFragment> applyPartitionPruning(
+            List<MappedFragment> mappedFragments, 
+            MappingParameters parameters, 
+            @Nullable PartitionPruningMetadata partitionPruningMetadata
+    ) {
+        if (partitionPruningMetadata == null) {
+            return mappedFragments;
+        }
+        return partitionPruner.apply(mappedFragments, 
parameters.dynamicParameters(), partitionPruningMetadata);
     }
 
     private FragmentsTemplate getOrCreateTemplate(MultiStepPlan plan) {
         // QuerySplitter is deterministic, thus we can cache result in order 
to reuse it next time
         return templatesCache.get(plan.id(), key -> {
-            IdGenerator idGenerator = new IdGenerator(0);
+            IdGenerator idGenerator = new IdGenerator(plan.numSources());

Review Comment:
   There is a piece of code in the FragmentMapper.MapperVisitor that causes 
issues when source ids and fragment ids collide:
   
   ```
   
org.apache.ignite.internal.sql.engine.exec.mapping.FragmentMapper.MapperVisitor#visit(org.apache.ignite.internal.sql.engine.rel.IgniteReceiver):
   
           @Override
           public Mapping visit(IgniteReceiver rel) {
               return mapComputableSource(rel.exchangeId());
           }
   
           private Mapping mapComputableSource(long sourceId) {
               ExecutionTarget target = 
context.targetFactory().someOf(context.nodes());
   
               return newMapping(sourceId, target);
           }
   ```



-- 
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-unsubscr...@ignite.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org