amansinha100 commented on a change in pull request #1598: DRILL-6880: For
Hash-Join hash-table build - treat null keys as an equal match
URL: https://github.com/apache/drill/pull/1598#discussion_r246194734
##########
File path:
exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/common/ChainedHashTable.java
##########
@@ -253,28 +277,29 @@ private void
setupIsKeyMatchInternal(ClassGenerator<HashTable> cg, MappingSet in
JConditional jc;
- // codegen for nullable columns if nulls are not equal
- if (comparators.get(i) == Comparator.EQUALS
- && left.isOptional() && right.isOptional()) {
- jc = cg.getEvalBlock()._if(left.getIsSet().eq(JExpr.lit(0)).
+ if ( work != setupWork.DO_BUILD ) { // BUILD runs this logic in a
separate method - areBothKeysNull()
+ // codegen for the special case when both columns are null (i.e.,
return early with midPointResult)
+ if (comparators.get(i) == Comparator.EQUALS
+ && left.isOptional() && right.isOptional()) {
+ jc = cg.getEvalBlock()._if(left.getIsSet().eq(JExpr.lit(0)).
cand(right.getIsSet().eq(JExpr.lit(0))));
- jc._then()._return(JExpr.FALSE);
+ jc._then()._return(midPointResult);
+ }
}
+ if ( ! checkIfBothNulls ) { // generate comparison code (at least one of
the two columns' values is non-null)
+ final LogicalExpression f =
FunctionGenerationHelper.getOrderingComparatorNullsHigh(left, right,
context.getFunctionRegistry());
- final LogicalExpression f =
- FunctionGenerationHelper
- .getOrderingComparatorNullsHigh(left, right,
context.getFunctionRegistry());
+ HoldingContainer out = cg.addExpr(f,
ClassGenerator.BlkCreateMode.FALSE);
- HoldingContainer out = cg.addExpr(f, ClassGenerator.BlkCreateMode.FALSE);
+ // check if two values are not equal (comparator result != 0)
+ jc = cg.getEvalBlock()._if(out.getValue().ne(JExpr.lit(0)));
- // check if two values are not equal (comparator result != 0)
- jc = cg.getEvalBlock()._if(out.getValue().ne(JExpr.lit(0)));
-
- jc._then()._return(JExpr.FALSE);
+ jc._then()._return(midPointResult);
+ }
}
// All key expressions compared equal, so return TRUE
- cg.getEvalBlock()._return(JExpr.TRUE);
+ cg.getEvalBlock()._return(finalResult);
Review comment:
The comment above this line says return TRUE but it should be changed since
`finalResult` can be False.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
