[CALCITE-739] Extend RexUtil.pullFactors to recognize additional common factors

[xuzifu666]

jira: https://issues.apache.org/jira/browse/CALCITE-739

Base on the jira, I added normalizeComparison(RexNode), which canonicalizes binary comparisons (=,!=,<,>,<=,>=) so that:

1. When both operands are RexInputRefs, the one with the smaller index appears on the left.
2. When one side is an input ref and the other is not (e.g., a literal or complex expression), the input ref appears on the left.
3. The method flips the operator using the existing RexUtil.invert when needed.

In addition applied normalizeComparison in pull, commonFactors, and removeFactor so that equivalent comparisons can be matched as common factors.

Note: I have not implemented the second rule mentioned in the Jira ticket in this PR. The reasons are the risk of exponential expansion resulting from CNF expansion and the overhead of additional implication checks on high-frequency call paths. Although this would handle more complex absorption scenarios, patterns like (a OR b) AND ((x AND a) OR (y AND b)) appear relatively infrequently in actual SQL optimization, whereas the proposed CNF expansion and implication checks would impact all code paths that call pullFactors.

[mihaibudiu]

How come there are no plan changes? Is this analysis used in Calcite at all?

[xuzifu666]

How come there are no plan changes? Is this analysis used in Calcite at all?

@mihaibudiu RexUtil.pullFactors is currently only used in tests, not in the main optimizer. So the fix corrects the utility method itself but does not change any production query plans.

But its value is more about correctness and future use. It could be adopted by RexSimplify or a rule. The same normalization idea could be applied where it actually matters, e.g., inside RexSimplify.simplifyAnds or a predicate-pulling rule, which would then produce plan changes.

I will investigate how to make improvements in RexSimplify with minimal cost.

[xuzifu666]

Apologies @mihaibudiu , I accidentally squashed the previous commit history with the current changes. Here is an overview of the current modifications:

RexSimplify.absorb now recognizes symmetric comparison forms during matching.
A new helper, equivalentComparison(a, b), treats two comparisons as equal when they differ only in operand order (a = b ≡ b = a, a < b ≡ b > a) — kind must be reversible and operands cross-equal. It replaces the digest-exact components.contains(other) check inside absorb. No expression is ever rewritten; the surviving term keeps its original form. I have actually added tests for RexSimplify; prior to this change, these tests would not pass.

[mihaibudiu]

If this code is never executed, it means it has very low coverage.

[mihaibudiu]

this only seems to work for comparisons of columns; this should be in the javadoc

[xuzifu666]

Done, the Javadoc of normalizeComparison now states that only comparisons involving an input ref are normalized, and that other operand kinds (e.g. two CASTs) are returned unchanged, matching the instanceof RexInputRef logic in the implementation.

[xuzifu666]

If this code is never executed, it means it has very low coverage.

IMO the code should been executed and covered, in both senses:

1. absorb runs on every AND/OR simplification, so the new predicate is on the hot path, not a rare branch.
2. the new symmetric-match branch is exercised directly by testSimplifyComparisonSymmetry: the absorption cases fail under the old code and pass with it, proving the branch is reached and load-bearing.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
92.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud