[
https://issues.apache.org/jira/browse/CALCITE-5907?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17776061#comment-17776061
]
Zoltan Haindrich commented on CALCITE-5907:
-------------------------------------------
I wonder how valid an `=` is if it has a `varchar` on one side and a `boolean`
on the other...
I had to disable paranoid mode to be able to run such cases...
If your compare operator supports things like that - I think instead of
allowing that it might be better to only remove such casts as a last step
before execution - and let calcite see the cast while its working with it.
I was looking into this a bit; some notes:
* we have the desired simplify via simplifyList ... simplifyComparision ;
which restricts this optimization to take it only into consideration when the
type of op0 is
[boolean|https://github.com/apache/calcite/blob/5151168e9a9035595939c2ae0f21a06984229209/core/src/main/java/org/apache/calcite/rex/RexSimplify.java#L540]
** this can be tricked into a similar bug by giving {{true = x}}
* in the meantime there is also a duplicate of a similar logic in
[here|https://github.com/apache/calcite/blob/5151168e9a9035595939c2ae0f21a06984229209/core/src/main/java/org/apache/calcite/rex/RexSimplify.java#L1641]
- which doesn't take the type of the column into account at all
I'm a bit skeptical about that we have anything to fix here....we could remove
the duplicate logic for sure; and tighten the check at the other - but I would
suspect that an issue like this could hit back from a different direction
later....
some testcases I was taking a look at:
{code}
// works correctly ; no simplify
@Test void testSimplify1() {
simplify=simplify.withParanoid(false);
checkSimplifyUnchanged(
eq(vVarchar(), trueLiteral));
}
// triggers a similar issue
@Test void testSimplify3() {
simplify=simplify.withParanoid(false);
checkSimplifyUnchanged(
eq(trueLiteral,vVarchar() ));
}
// triggers the reported issue
@Test void testSimplify2() {
simplify=simplify.withParanoid(false);
checkSimplifyUnchanged(
and(
eq(vVarchar(1), vVarchar(2)),
eq(vVarchar(3), trueLiteral)));
}
@Test void testSimplifyValidUsage() {
checkSimplify(
and(
eq(vVarchar(1), vVarchar(2)),
eq(cast(vVarchar(3),tBool()), trueLiteral))
,"AND(=(?0.varchar1, ?0.varchar2), CAST(?0.varchar3):BOOLEAN NOT NULL)");
}
{code}
> Unexpected boolean expression simplification for And expression
> ---------------------------------------------------------------
>
> Key: CALCITE-5907
> URL: https://issues.apache.org/jira/browse/CALCITE-5907
> Project: Calcite
> Issue Type: Bug
> Components: core
> Affects Versions: 1.35.0
> Reporter: Yunhong Zheng
> Priority: Major
>
> As FLINK-27402 shown. If we have a table MyTable(a Int, b Boolean, c
> String). Calcite will not simplify this case ( c is Varchar type while
> SqlLiteral is boolean):
> {code:java}
> SELECT * FROM MyTable WHERE c = true;{code}
> As the logical plan is :
> {code:java}
> LogicalSink(table=[*anonymous_collect$1*], fields=[a, b, c])
> +- LogicalProject(inputs=[0..2])
> +- LogicalFilter(condition=[=($2, true)])
> +- LogicalTableScan(table=[[default_catalog, default_database,
> MyTable]]){code}
> However, Calcite will simplify this case while simplifyAnd :
> {code:java}
> SELECT * FROM MyTable WHERE b = true and c = true;{code}
> As the logical plan is shown below: 'b = true' and 'c = true' both were
> simplified to 'b' and 'c':
> {code:java}
> LogicalSink(table=[*anonymous_collect$1*], fields=[a, b, c])
> +- LogicalProject(inputs=[0..2])
> +- LogicalFilter(condition=[AND($1, $2)])
> +- LogicalTableScan(table=[[default_catalog, default_database,
> MyTable]]){code}
> This may cause error because of filter condition is a Varchar type literal.
>
> After reading Calcite's code, I found that. The logic of
> RexSimplify.implify() and RexSimplify.implifyAnd() is different, where the
> logic of RexSimplify.implifyAnd() is problematic:
> {code:java}
> // Simplify BOOLEAN expressions if possible
> while (term.getKind() == SqlKind.EQUALS) {
> RexCall call = (RexCall) term;
> if (call.getOperands().get(0).isAlwaysTrue()) {
> term = call.getOperands().get(1);
> terms.set(i, term);
> continue;
> } else if (call.getOperands().get(1).isAlwaysTrue()) {
> term = call.getOperands().get(0);
> terms.set(i, term);
> continue;
> }
> break;
> } {code}
> The above code cannot make such a simple judgment, as there may not be an
> implicit conversion to ensure that the types on both sides of the condition
> are consistent.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
