[
https://issues.apache.org/jira/browse/CALCITE-7665?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18099599#comment-18099599
]
Yu Xu commented on CALCITE-7665:
--------------------------------
I have conducted some verification regarding this and would like to summarize
my conclusions:
Porting the aforementioned optimization to `RelBuilder`/`RexSimplify` would
entail significant refactoring; for instance, when `SqlToRelConverter` creates
`Filter` or `Project` nodes, it largely bypasses `RelBuilder`, opting instead
to call `RelFactories` or `LogicalXxx.create()` directly.
1. Points in the SqlToRelConverter code that must be modified:
convertWhere() current implementation:
final RelFactories.FilterFactory filterFactory =
RelFactories.DEFAULT_FILTER_FACTORY;
final RelNode filter =
filterFactory.createFilter(bb.root(), convertedWhere2, ImmutableSet.of());
To switch to using RelBuilder:
final RelNode filter = relBuilder
.push(bb.root())
.filter(convertedWhere2)
.build();
This ensures that `relBuilder.filter()` calls
`RexSimplify.simplifyFilterPredicates()`, converting `NOT(OR(=, =))` into
`SEARCH(Sarg)` or `AND(<>, <>)`.
The inner Filter of the correlated join also needs to be updated to
`relBuilder.push(p.r).filter(newCond).build()`; otherwise, the `NOT IN` within
the lateral join will not be simplified.
Other locations where `Filter`, `Join`, or `Project` are directly instantiated
also require modification.
2. Impact on current code
A large number of test expectations need to be rewritten; RelBuilder's
`simplify` does more than just the `NOT(OR) → AND` optimization—it also:
Constant folding/removal of redundant CASTs—such as `x = 1 OR x = 2` becoming
`SEARCH(Sarg)` or `TRUE AND x` simplifying to `x`—means that, all in all, once
`convertWhere()` switches to using `RelBuilder`, the query plans for all test
cases involving `WHERE` clauses in `SqlToRelConverterTest` are likely to
change. Conservatively estimated, hundreds of XML entries will need to be
updated.
Perhaps we could launch a separate refactoring initiative later to fully
address this matter, as that might offer better control; however, if the goal
is simply to resolve the issue in the current Jira ticket, submitting the
corresponding PR should suffice (requiring only < 10 lines of code in
`SqlToRelConverter`).
> Convert multi-value NOT IN to AND of NOT_EQUALS in SqlToRelConverter
> --------------------------------------------------------------------
>
> Key: CALCITE-7665
> URL: https://issues.apache.org/jira/browse/CALCITE-7665
> Project: Calcite
> Issue Type: Improvement
> Components: core
> Affects Versions: 1.42.0
> Reporter: Yu Xu
> Assignee: Yu Xu
> Priority: Major
> Labels: pull-request-available
> Fix For: 1.43.0
>
>
> Currently "deptno NOT IN (10, 20)" would convert to:
>
> {code:java}
> NOT(OR(=(deptno, 10), =(deptno, 20))) {code}
>
> 1.Asymmetry with IN the handling of single-valued and multi-valued IN has
> already become very straightforward:
>
> {code:java}
> deptno IN (10) -> =(deptno, 10)
> deptno IN (10, 20) -> OR(=(deptno, 10), =(deptno, 20)) {code}
>
> However, while you optimized `NOT IN` with a single value into `NOT_EQUALS`,
> the multi-value case remains as `NOT(OR(EQUALS))`, resulting in a lack of
> consistency between the two at the execution plan level.
> 2. Subsequent optimization rules may fail to match:
> Many rules in Calcite operate directly based on `SqlKind` or the operator
> type. For instance:
> Certain index pushdown and partition pruning optimizations specifically look
> for `NOT_EQUALS`.
> Constant propagation and range inference handle `<> 10` more naturally than
> `NOT(= 10)`.
> If expressions consistently appear in the form of `NOT(EQUALS)`, these rules
> might overlook them, thereby missing opportunities for further simplification.
> 3. An extra negation in the generated executable code
> When ultimately compiling the `RelNode` into Java code for execution:
> {code:java}
> `<>(a, b)` directly generates `a != b`.
> `NOT(=(a, b))` generates `!(a == b)`. {code}
> Although the overhead is minimal, for filter conditions evaluated frequently,
> it is best to eliminate any unnecessary operations.
>
> This issue can also be identified from existing tests, such as:
> {code:java}
> select empno from emp where not case when true then deptno in (10,20) when
> false then false else deptno in (30,40) end {code}
> current plan is：
> {code:java}
> LogicalProject(EMPNO=[$0])
> LogicalFilter(condition=[CASE(true, NOT(OR(=($7, 10), =($7, 20))),
> NOT(true))])
> LogicalTableScan(table=[[CATALOG, SALES, EMP]]) {code}
> it is better covert to：
>
> {code:java}
> LogicalProject(EMPNO=[$0])
> LogicalFilter(condition=[CASE(true, AND(<>($7, 10), <>($7, 20)),
> NOT(true))])
> LogicalTableScan(table=[[CATALOG, SALES, EMP]])
> {code}
>
>
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
