[CALCITE-5132] Scalar IN subquery returns UNKNOWN instead of FALSE when key is partially NULL#4814
[CALCITE-5132] Scalar IN subquery returns UNKNOWN instead of FALSE when key is partially NULL#4814xiedeyantu wants to merge 2 commits intoapache:mainfrom
Conversation
|
This PR is not yet ready for review. I need to continue checking. |
|
…en key is partially NULL
|
I think it is ready for review. |
| // Now the left join | ||
| builder.join(JoinRelType.LEFT, builder.and(conditions), variablesSet); | ||
|
|
||
| // for multi-column IN with nullable RHS columns, the global ck < c check is |
There was a problem hiding this comment.
The comment here needs some context: the actual comparison ck < c is much lower.
Perhaps you can first explain how the expansion is handled for scalar values.
| // (empno, deptno) IN (select empno, deptno from ...) | ||
| // | ||
| // where one RHS row is (7521, null): | ||
| // empno=7521: AND(TRUE, UNKNOWN) = UNKNOWN → result should be null |
There was a problem hiding this comment.
I also don't understand what this AND is
| nrAlias = "nr" + subQueryIndex; | ||
| } | ||
| // Only needed for multi-column, non-literal IN with null-safety logic. | ||
| // Single-column cases are handled correctly by the existing ck < c check. |
There was a problem hiding this comment.
What happens if the single column has a ROW type?
There was a problem hiding this comment.
I think the row type still falls under the category of single columns. This scenario involves the form (x, y) IN ((...), (...)).
There was a problem hiding this comment.
It would be nice to have a test to confirm that the implementation is correct.
Frankly I don't see a difference between two columns and a ROW type.
If the ROW type works correctly, maybe the two implementations can be unified?
There was a problem hiding this comment.
Okay, I need to confirm this issue again.
There was a problem hiding this comment.
I checked the form of row in, and it is indeed very similar to the current form. Before removing this patch, the row in form also failed to produce the correct result, but after this patch, it executes correctly.
|
2 participants
See CALCITE-5132