[GitHub] [ignite] korlov42 commented on a diff in pull request #10424: IGNITE-18341 SQL Calcite: Introduce correlated distribution

[GitBox <gi...@apache.org>]

korlov42 commented on code in PR #10424:
URL: https://github.com/apache/ignite/pull/10424#discussion_r1043571089


##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/trait/IgniteDistributions.java:
##########
@@ -113,6 +114,15 @@ public static IgniteDistribution hash(List<Integer> keys, DistributionFunction f
         return canonize(new DistributionTrait(ImmutableIntList.copyOf(keys), function));
     }
 
+    /**
+     * @param corrId Target distribution correlation id.
+     * @param target Target distribution.
+     * @return Distribution by correlate.
+     */
+    public static IgniteDistribution correlated(CorrelationId corrId, IgniteDistribution target) {

Review Comment:
   Could you please add more details to the javadoc? What does correlated distribution  mean? When should and should not it be used? 



##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/trait/DistributionFunction.java:
##########
@@ -316,4 +327,59 @@ public AffinityDistribution(int cacheId, Object identity) {
             return "affinity[identity=" + identity + ", cacheId=" + cacheId + ']';
         }
     }
+
+    /**
+     * Correlated distribution, used to bypass set of nodes on the right hand of CNLJ and to be restored to
+     * original hash distribution (with remapped keys) by the filter node.
+     */
+    public static final class CorrelatedDistribution extends DistributionFunction {
+        /** */
+        private final CorrelationId corrId;
+
+        /** */
+        private final IgniteDistribution target;
+
+        /** */
+        private CorrelatedDistribution(CorrelationId corrId, IgniteDistribution target) {
+            this.corrId = corrId;
+            this.target = target;
+
+            assert target.getType() == RelDistribution.Type.HASH_DISTRIBUTED : target.getType();
+        }
+
+        /** {@inheritDoc} */
+        @Override public RelDistribution.Type type() {
+            return RelDistribution.Type.RANDOM_DISTRIBUTED;
+        }
+
+        /** {@inheritDoc} */
+        @Override public <Row> Destination<Row> destination(
+            ExecutionContext<Row> ctx,
+            AffinityService affSrvc,
+            ColocationGroup target,
+            ImmutableIntList keys
+        ) {
+            throw new AssertionError("Correlated distribution should be converted to delegate before using");
+        }
+
+        /** */
+        public CorrelationId correlationId() {
+            return corrId;
+        }
+
+        /** */
+        public IgniteDistribution target() {
+            return target;
+        }
+
+        /** {@inheritDoc} */
+        @Override public boolean correlated() {
+            return true;
+        }
+
+        /** {@inheritDoc} */
+        @Override protected String name0() {
+            return "correlated[corrId=" + corrId + ", target=" + target + ']';

Review Comment:
   does it make sense to choose another name? Right now it clashes with CorrelationTrait



-- 
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