[CALCITE-7646] CorrelateProjectExtractor does not handle nested field accesses cor0.field0.field1#5086
[CALCITE-7646] CorrelateProjectExtractor does not handle nested field accesses cor0.field0.field1#5086mihaibudiu wants to merge 1 commit into
CorrelateProjectExtractor does not handle nested field accesses cor0.field0.field1#5086Conversation
|
@zabetak according to git blame you wrote most of this code, so I took the liberty to request your review. |
… accesses cor0.field0.field1 Signed-off-by: Mihai Budiu <mbudiu@feldera.com>
|
I think the CI failure is transient, the tests pass locally. |
There shouldn't be any problems. I re-trigger sonar CI to double-check. |
|
The fact that I requested a review from @zabetak should not deter anyone else from reviewing this PR. I think it's a genuine improvement which will help our decorrelator handle more queries. |
|
Would it also be possible to add a few SQL-based test cases to QuidemTest? |
|
Sure, I will do that, but the results should not change, just the nature of the plan |
xuzifu666
left a comment
xuzifu666
left a comment
There was a problem hiding this comment.
In my view the overall implementation is ok and meets the objectives. Apart from suggesting the addition of QuidemTest, I have left two comments for your reference.
| // Find the correlated expressions from the right side that can be moved to the left | ||
| Set<RexNode> callsWithCorrelationInRight = | ||
| findCorrelationDependentCalls(correlate.getCorrelationId(), right); | ||
| // Only direct field accesses on the correlation variable, such as |
There was a problem hiding this comment.
These comments should be left-aligned.
|
|
||
| /** Test case for <a href="https://issues.apache.org/jira/browse/CALCITE-7646">[CALCITE-7646] | ||
| * CorrelateProjectExtractor does not handle nested field accesses cor0.field0.field1</a>. */ | ||
| @Test void testNestedCorrelationFieldAccessInFilter() { |
There was a problem hiding this comment.
The new logic returns null from visitCall for neutral subexpressions. The Javadoc example CASE(IS NOT NULL($cor0.PATH), $cor0.PATH, ARRAY(NULL)) seems not covered by any test. can we add a test to verify that constant/dynamic-parameter operands do not wrongly disqualify the enclosing correlated call?
3 participants
Jira Link
CALCITE-7646
Changes Proposed
These changes make the helper class slightly more capable, enabling more programs to be decorrelated.