You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2021/07/21 06:21:13 UTC
[GitHub] [druid] abhishekagarwal87 commented on a change in pull request #11434: Fix left join SQL queries with IS NOT NULL filter
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org