hsyuan commented on issue #1459: [CALCITE-3347] IndexOutOfBoundsException in 
FixNullabilityShuttle when using FilterIntoJoinRule
URL: https://github.com/apache/calcite/pull/1459#issuecomment-537776037
 
 
   I need to reconsider this. The predicate is just NOT_ENUMERABLE, it doesn't 
check whether it is semi/anti or not. We can update the predicate to exclude 
semi joins.
   ```
   join.getConvention() != EnumerableConvention.INSTANCE || 
!joinType.projectsRight()
   ```
   which means when it is enumerable and not semi joins, we will pull up the 
filter conditions. (because enumerable hash / merge joins currently don't 
support non-equi join conditions)
   So the index check would become unnecessary.
   
   But the code is really counter intuitive:
   ```
   if (!predicate.apply(join, joinType, exp)) {
   ```
   I would rather remove the not `!` and change the predicate to the opposite, 
but given that this may change the public API to some extent, I am not sure the 
risk of change that may be posed to down stream, although I doubt if there is 
anyone actually customizes the rule.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to