vlsi commented on PR #2611: URL: https://github.com/apache/calcite/pull/2611#issuecomment-6341268661
Thank you for the contribution, and sorry it waited this long for a decision. I'm closing this PR because the default configuration no longer produces the plan from CALCITE-4887. Since Calcite 1.34.0, `SqlToRelConverter.Config.expand` defaults to `false` (CALCITE-3870, https://github.com/apache/calcite/commit/7e8bc2dba49297c0fbe8f8f4cbcdb0a616871caf), and setting it to `true` is deprecated. With the default, the query from `testFilterSubQueryRemove` keeps the `IN` as a `RexSubQuery`, and `CoreRules.FILTER_SUB_QUERY_TO_CORRELATE` turns it into one left join, with no `COUNT()` cross join: ```text LogicalAggregate(group=[{}], EXPR$0=[COUNT()]) LogicalProject($f0=[1]) LogicalProject("K0"=[$0], "C1"=[$1], "F1"."A0"=[$2], "F2"."A0"=[$3], "F0"."C0"=[$4], "F1"."C0"=[$5], "F0"."C1"=[$6], "F1"."C2"=[$7], "F2"."C3"=[$8], EMPNO=[$9], ENAME=[$10], JOB=[$11], MGR=[$12], HIREDATE=[$13], SAL=[$14], COMM=[$15], DEPTNO=[$16], SLACKER=[$17]) LogicalFilter(condition=[OR(=($4, 1), IS NOT NULL($19))]) LogicalJoin(condition=[=($4, $18)], joinType=[left]) LogicalJoin(condition=[=($4, $16)], joinType=[left]) LogicalTableScan(table=[[CATALOG, STRUCT, T]]) LogicalTableScan(table=[[CATALOG, SALES, EMP]]) LogicalAggregate(group=[{0}], i=[LITERAL_AGG(true)]) LogicalProject(DEPTNO=[$7]) LogicalFilter(condition=[=($7, 1)]) LogicalTableScan(table=[[CATALOG, SALES, EMP]]) ``` The cross join still appears with `expand = true`, which is the path this PR changes. On that path the change also produces a wrong filter: in the expected plan the PR adds to `RelOptRulesTest.xml`, the cross join is gone, but the condition still reads `CASE(=($9, 0:BIGINT), … <($10, $9) …)`, and `$9` and `$10` are now `EMPNO` and `ENAME` rather than the two counts. The change also skips the count join for `NOT IN`: the PR changes the expected result of `select * from "scott".emp where empno not in (null, 7782)` in `sub-query.iq` from 0 rows to 13, and the correct result is 0 rows. The count join was added in CALCITE-2071 to get these cases right. If you rely on `expand = true` and need the fix there, feel free to open a new PR against `main`. It should skip the count join only where an unknown `IN` result can't change the filter (under `AND` or `OR` in `WHERE`, not under `NOT` or `NOT IN`), check the `IN` there with a left join and `IS NOT NULL` on the indicator, as `SubQueryRemoveRule` does on `main`, and keep the existing `NOT IN` cases in `sub-query.iq` passing unchanged. -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
