[CALCITE-3128] Joining two tables producing only NULLs will return 0 … #1279
Conversation
linq4j/src/main/java/org/apache/calcite/linq4j/EnumerableDefaults.java
Outdated
Show resolved
Hide resolved
| @@ -1113,7 +1121,14 @@ public boolean moveNext() { | |||
| final TSource outer = outers.current(); | |||
| final Enumerable<TInner> innerEnumerable; | |||
| if (outer == null) { | |||
| innerEnumerable = null; | |||
There was a problem hiding this comment.
Since the innerLookup has only one entry (maybe EmptyList or some constant numbers in Linq4jTest.ONE_SELECTOR, etc.), can we just remove the branch (outer==null) to avoid introducing the variable isConditionAlwaysTrue? The logic in else branch can handle this case as the outerKey is also EmptyList.
There was a problem hiding this comment.
We can remove the branch outer==null only if
- the
outerKeySelector.apply(outer)can handle null key, - and we also should match the the
outerKeytype to know if this is a always true match,
These 2 conditions are too dependent on the implementation and can not be guaranteed.
| // when the join condition is always true, the innerLookup will always | ||
| // have one entry whose key is an EmptyList and value is | ||
| // the whole inner enumerable. | ||
| innerEnumerable = innerLookup.values().iterator().next(); |
There was a problem hiding this comment.
Do we need to add Assertion here for double check?
There was a problem hiding this comment.
I think we don't need to, cause a always true condition will drain all the inner enumerables definitely into one entry
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableHashJoin.java
Show resolved
Hide resolved
| joinType.generatesNullsOnRight())))).toBlock()); | ||
| joinType.generatesNullsOnRight())) | ||
| .append( | ||
| Expressions.constant( |
There was a problem hiding this comment.
Minor detail. We could simplify this expression with Expressions.list and four append:
Expressions.list(
A, B, C, D)
.append(E)
.append(F)
.append(G)
.append(H)
by just moving the append parameters directly inside the Expressions.list (and get rid of all the append calls):
Expressions.list(
A, B, C, D, E, F, G, H)
danny0405
force-pushed
the
master
branch
2 times, most recently
from
80f411d
to
ca27fe9
Compare
vlsi
force-pushed
the
master
branch
2 times, most recently
from
49cb002
to
8768a23
Compare
…rows