abhishekagarwal87 commented on a change in pull request #11434:
URL: https://github.com/apache/druid/pull/11434#discussion_r673680040



##########
File path: 
sql/src/main/java/org/apache/druid/sql/calcite/rule/DruidJoinRule.java
##########
@@ -243,15 +248,30 @@ static boolean canHandleCondition(final RexNode 
condition, final RelDataType lef
 
       if (isLeftExpression(operands.get(0), numLeftFields) && 
isRightInputRef(operands.get(1), numLeftFields)) {
         equalitySubConditions.add(Pair.of(operands.get(0), (RexInputRef) 
operands.get(1)));
+        rightColumns.add((RexInputRef) operands.get(1));
       } else if (isRightInputRef(operands.get(0), numLeftFields)
                  && isLeftExpression(operands.get(1), numLeftFields)) {
         equalitySubConditions.add(Pair.of(operands.get(1), (RexInputRef) 
operands.get(0)));
+        rightColumns.add((RexInputRef) operands.get(0));
       } else {
         // Cannot handle this condition.
         return Optional.empty();
       }
     }
 
+    if (right != null && 
!DruidJoinQueryRel.computeRightRequiresSubquery(DruidJoinQueryRel.getSomeDruidChild(right))
+        && right instanceof DruidQueryRel) {
+      DruidQueryRel druidQueryRel = (DruidQueryRel) right;
+      if (druidQueryRel.getDruidTable().getDataSource() instanceof 
LookupDataSource) {
+        long distinctRightColumns = 
rightColumns.stream().map(RexSlot::getIndex).distinct().count();
+        if (distinctRightColumns > 1) {
+          // it means that the join's right side is lookup and the join 
condition contains both columns of lookup.
+          // currently, the native engine doesn't support this.

Review comment:
       ### 
   ```suggestion
             // it means that the join's right side is lookup and the join 
condition contains both key and value columns of lookup.
             // currently, the lookup datasource in the native engine doesn't 
support using value column in the join condition.
   ```

##########
File path: 
sql/src/main/java/org/apache/druid/sql/calcite/rel/DruidJoinQueryRel.java
##########
@@ -316,6 +316,9 @@ public RelOptCost computeSelfCost(final RelOptPlanner 
planner, final RelMetadata
       cost = CostEstimates.COST_JOIN_SUBQUERY;
     } else {
       cost = partialQuery.estimateCost();
+      if (joinRel.getJoinType() == JoinRelType.INNER) {

Review comment:
       this behaviour should be controllable through a feature flag. 

##########
File path: 
sql/src/main/java/org/apache/druid/sql/calcite/rule/DruidJoinRule.java
##########
@@ -243,15 +248,30 @@ static boolean canHandleCondition(final RexNode 
condition, final RelDataType lef
 
       if (isLeftExpression(operands.get(0), numLeftFields) && 
isRightInputRef(operands.get(1), numLeftFields)) {
         equalitySubConditions.add(Pair.of(operands.get(0), (RexInputRef) 
operands.get(1)));
+        rightColumns.add((RexInputRef) operands.get(1));
       } else if (isRightInputRef(operands.get(0), numLeftFields)
                  && isLeftExpression(operands.get(1), numLeftFields)) {
         equalitySubConditions.add(Pair.of(operands.get(1), (RexInputRef) 
operands.get(0)));
+        rightColumns.add((RexInputRef) operands.get(0));
       } else {
         // Cannot handle this condition.
         return Optional.empty();
       }
     }
 
+    if (right != null && 
!DruidJoinQueryRel.computeRightRequiresSubquery(DruidJoinQueryRel.getSomeDruidChild(right))

Review comment:
       can you also add an explanation for checking 
`DruidJoinQueryRel.computeRightRequiresSubquery` - i.e. in if it requires a 
sub-query, even joining a lookup will use a querydatasource. 

##########
File path: 
sql/src/main/java/org/apache/druid/sql/calcite/rule/FilterJoinExcludePushToChildRule.java
##########
@@ -292,4 +299,43 @@ private static boolean classifyFilters(List<RexNode> 
filters,
     // Did anything change?
     return !filtersToRemove.isEmpty();
   }
+
+  private void removeRedundantIsNotNullFilters(List<RexNode> joinFilters, 
JoinRelType joinType)

Review comment:
       can you add a brief explanation of what is happening here? 




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



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to