xuzifu666 commented on code in PR #5295:
URL: https://github.com/apache/calcite/pull/5295#discussion_r4113880154
##########
core/src/main/java/org/apache/calcite/rex/RexSimplify.java:
##########
@@ -2013,11 +2013,13 @@ private <C extends Comparable<C>> RexNode
simplifyAnd2ForUnknownAsFalse(
// Simplify BOOLEAN expressions if possible
while (term.getKind() == SqlKind.EQUALS) {
RexCall call = (RexCall) term;
- if (call.getOperands().get(0).isAlwaysTrue()) {
+ if (call.getOperands().get(0).isAlwaysTrue()
+ && call.getOperands().get(1).getType().getSqlTypeName() ==
SqlTypeName.BOOLEAN) {
Review Comment:
Let me share my understanding of this part—though I might be mistaken: I
agree that a well-typed Rex tree should never contain =(varchar, bool), and
that SqlValidator is the right place to enforce comparability. However, that
invariant is not enforced at the Rex layer itself: `RexBuilder.makeCall(EQUALS,
...)` performs no operand-family check (`EQUALS`' return-type inference is
unconditionally `BOOLEAN`), so downstream systems that construct expressions
programmatically (e.g. Flink, the reporter of
https://issues.apache.org/jira/browse/FLINK-27402) can and do produce such
nodes.
The bug here is not the input's existence, but that the simplifier turned a
valid `BOOLEAN` condition into a non-`BOOLEAN` one. A rewrite rule must never
produce output less valid than its input; leaving the equality untouched is the
minimal, safe behavior. Throwing from a simplification hot path would turn a
silent robustness gap into planner crashes for rels that currently plan and
execute fine, and conflates rewriting with validation, Calcite keeps those
separate (cf. `RexChecker`/`RexUtil.verify`).
--
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]
