fix: optimize_projections failure after mark joins created by EXISTS OR EXISTS#21265
fix: optimize_projections failure after mark joins created by EXISTS OR EXISTS#21265buraksenn wants to merge 4 commits intoapache:mainfrom
optimize_projections failure after mark joins created by EXISTS OR EXISTS#21265Conversation
follow up on apache#17119
| let result = optimizer.optimize(plan, &config, observe); | ||
| assert!( | ||
| result.is_ok(), | ||
| "Full optimizer should not fail with schema mismatch: {:?}", |
There was a problem hiding this comment.
Seems a bit fragile to assume the assert fails with a schema mismatch, if it fails.
There was a problem hiding this comment.
I've added a test for just make sure full optimizer runs successfully as a regression test instead of this. Moreover, I've modified the other test to use assert_optimized_plan_equal and check final plan.
| // Verify no double projection — the projection we add to strip mark | ||
| // columns should be merged by optimize_projections, not left stacked. | ||
| assert!( | ||
| !plan_str.contains("Projection: a.a, a.b, a.c\n Projection:"), |
There was a problem hiding this comment.
Testing for an expected plan via string containment seems a bit unfortunate. Can't we check for the plan shape we expect, e.g., via assert_optimized_plan_equal!?
There was a problem hiding this comment.
I've added a test for just make sure full optimizer runs successfully as a regression test instead of this. Moreover, I've modified the other test to use
assert_optimized_plan_equaland check final plan.
replied above. Thanks it makes sense
2 participants
Which issue does this PR close?
Rationale for this change
Issue has details but main problem is mark columns from LeftMark joins leak into parent join schemas, causing
optimize_projectionsoptimizer to fail.What changes are included in this PR?
Add a projection after embedded subquery decorrelation to strip mark columns, following the same pattern as
scalar_subquery_to_join. I've seen this projection is merged in the final plan.Are these changes tested?
Added test case for reported failure
Are there any user-facing changes?
No.