# [jira] [Comment Edited] (FLINK-14030) Nonequivalent conversion happens in Table planner

```
[
] ```
```
Leonard Xu edited comment on FLINK-14030 at 9/12/19 8:37 AM:
-------------------------------------------------------------

The root cause is that Calcite's simplification logic of isNull expression will
convert  from

*"f(operand0, operand1) IS NULL"* to

*"operand0 IS NULL OR operand1 IS NULL"* when the *Policy* of  RexNode‘s
*SqlKind* is *ANY* 。

{code:java}
//org.apache.calcite.rex.RexSimplify.java

private RexNode simplifyIsNull(RexNode a) {
// Simplify the argument first,
// call ourselves recursively to see whether we can make more progress.
// For example, given
// "(CASE WHEN FALSE THEN 1 ELSE 2) IS NULL" we first simplify the
// argument to "2", and only then we can simplify "2 IS NULL" to "FALSE".
a = simplify(a, UNKNOWN);
if (!a.getType().isNullable() && isSafeExpression(a)) {
return rexBuilder.makeLiteral(false);
}
if (RexUtil.isNull(a)) {
return rexBuilder.makeLiteral(true);
}
if (a.getKind() == SqlKind.CAST) {
return null;
}
switch (Strong.policy(a.getKind())) {
case NOT_NULL:
return rexBuilder.makeLiteral(false);
case ANY:
// "f" is a strong operator, so "f(operand0, operand1) IS NULL" simplifies
// to "operand0 IS NULL OR operand1 IS NULL"
final List<RexNode> operands = new ArrayList<>();
for (RexNode operand : ((RexCall) a).getOperands()) {
final RexNode simplified = simplifyIsNull(operand);
if (simplified == null) {
rexBuilder.makeCall(SqlStdOperatorTable.IS_NULL, operand));
} else {
}
}
return RexUtil.composeDisjunction(rexBuilder, operands, false);
case AS_IS:
default:
return null;
}
}{code}

Unfortunately, most of calculating SqlKinds are assigned *Policy.ANY*  at
present.

{code:java}
//org.apache.calcite.plan.Strong.java
public static Policy policy(SqlKind kind) {
return MAP.getOrDefault(kind, Policy.AS_IS);
}

....

map.put(SqlKind.PLUS, Policy.ANY);
map.put(SqlKind.PLUS_PREFIX, Policy.ANY);
map.put(SqlKind.MINUS, Policy.ANY);
map.put(SqlKind.MINUS_PREFIX, Policy.ANY);
map.put(SqlKind.TIMES, Policy.ANY);
map.put(SqlKind.DIVIDE, Policy.ANY);

* that operator evaluates to null. */
public enum Policy {
/** This kind of expression is never null. No need to look at its arguments,
* if it has any. */
NOT_NULL,

/** This kind of expression has its own particular rules about whether it
* is null. */
CUSTOM,

/** This kind of expression is null if and only if at least one of its
* arguments is null. */
ANY,

/** This kind of expression may be null. There is no way to rewrite. */
AS_IS,
}{code}

Both Flink SQL and Flink table API will call  this simplification logic. It
seems difficult to fix this issue elegantly since this simplification is
treated  as normal behavior in Calcite.

Maybe we should find a better way to fix this later. Do you have any suggestion?

[~lzljs3620320] [~godfreyhe]

thanks.

was (Author: leonard xu):
The root cause is that Calcite's simplification logic of isNull expression will
convert  from

*"f(operand0, operand1) IS NULL"* to

*"operand0 IS NULL OR operand1 IS NULL"* when the *Policy* of  RexNode‘s *__
SqlKind* is *ANY* 。

{code:java}
//org.apache.calcite.rex.RexSimplify.java

private RexNode simplifyIsNull(RexNode a) {
// Simplify the argument first,
// call ourselves recursively to see whether we can make more progress.
// For example, given
// "(CASE WHEN FALSE THEN 1 ELSE 2) IS NULL" we first simplify the
// argument to "2", and only then we can simplify "2 IS NULL" to "FALSE".
a = simplify(a, UNKNOWN);
if (!a.getType().isNullable() && isSafeExpression(a)) {
return rexBuilder.makeLiteral(false);
}
if (RexUtil.isNull(a)) {
return rexBuilder.makeLiteral(true);
}
if (a.getKind() == SqlKind.CAST) {
return null;
}
switch (Strong.policy(a.getKind())) {
case NOT_NULL:
return rexBuilder.makeLiteral(false);
case ANY:
// "f" is a strong operator, so "f(operand0, operand1) IS NULL" simplifies
// to "operand0 IS NULL OR operand1 IS NULL"
final List<RexNode> operands = new ArrayList<>();
for (RexNode operand : ((RexCall) a).getOperands()) {
final RexNode simplified = simplifyIsNull(operand);
if (simplified == null) {
rexBuilder.makeCall(SqlStdOperatorTable.IS_NULL, operand));
} else {
}
}
return RexUtil.composeDisjunction(rexBuilder, operands, false);
case AS_IS:
default:
return null;
}
}{code}

Unfortunately, most of calculating SqlKinds are assigned *Policy.ANY*  at
present.

{code:java}
//org.apache.calcite.plan.Strong.java
public static Policy policy(SqlKind kind) {
return MAP.getOrDefault(kind, Policy.AS_IS);
}

....

map.put(SqlKind.PLUS, Policy.ANY);
map.put(SqlKind.PLUS_PREFIX, Policy.ANY);
map.put(SqlKind.MINUS, Policy.ANY);
map.put(SqlKind.MINUS_PREFIX, Policy.ANY);
map.put(SqlKind.TIMES, Policy.ANY);
map.put(SqlKind.DIVIDE, Policy.ANY);

* that operator evaluates to null. */
public enum Policy {
/** This kind of expression is never null. No need to look at its arguments,
* if it has any. */
NOT_NULL,

/** This kind of expression has its own particular rules about whether it
* is null. */
CUSTOM,

/** This kind of expression is null if and only if at least one of its
* arguments is null. */
ANY,

/** This kind of expression may be null. There is no way to rewrite. */
AS_IS,
}{code}

Both Flink SQL and Flink table API will call  this simplification logic. It
seems difficult to fix this issue elegantly since this simplification is
treated  as normal behavior in Calcite.

Maybe we should find a better way to fix this later. Do you have any suggestion?

[~lzljs3620320] [~godfreyhe]

thanks.

> Nonequivalent conversion happens in Table planner
> --------------------------------------------------
>
>          Issue Type: Bug
>          Components: Table SQL / Planner
>    Affects Versions: 1.9.0
>            Reporter: Leonard Xu
>            Priority: Critical
>             Fix For: 1.9.1
>
>
> *testAllApis()* unit tests will run fail because planner make a conversion
>  from *[ifThenElse(isNull(plus(f0, f1)), 'null', 'not null')]*
>  to *[CASE(OR(IS NULL(\$0), IS NULL(\$1)), _UTF-16LE'null', _UTF-16LE'not
> null')]*
>  which is not a equivalence conversion. The result of expression 'f0 + 'f1
> should be null
>  when the result overflows even if its two operands both are not null.
> It's easy to reproduce as following:
>  testAllApis(
>  'f0 + 'f1,
>  "f1 + f1",
>  "f1 + f1",
>  "null")// the result should be null because overflow
> override def testData: Row =
> { val testData = new Row(2) testData.setField(0,
> BigDecimal("1e10").bigDecimal) testData.setField(1,
> BigDecimal("0").bigDecimal) testData }
> override def typeInfo: RowTypeInfo =
> { new RowTypeInfo( /* 0 */ fromLogicalTypeToTypeInfo(DECIMAL(38, 10)), /* 1
> */ fromLogicalTypeToTypeInfo(DECIMAL(38, 28)) ) }
>

--
This message was sent by Atlassian Jira
(v8.3.2#803003)
```