[ 
https://issues.apache.org/jira/browse/CALCITE-3368?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16949867#comment-16949867
 ] 

Julian Hyde commented on CALCITE-3368:
--------------------------------------

What do we expect to be the result of 1073741824 + 1073741824 if we are dealing 
with INTEGER columns? (2^30  + 2^30  is 2^31, which is too large for a 32 bit 
signed INTEGER .) You seem to be expecting that the calculation will return 
NULL, but I think our Java implementation will evaluate to -2147483648 (-2 ^ 
31).

I don't recall what the SQL standard says we should do.

> PLUS, MUNUS and TIMES should be unsafe when simplifying ‘expression IS NULL’
> ----------------------------------------------------------------------------
>
>                 Key: CALCITE-3368
>                 URL: https://issues.apache.org/jira/browse/CALCITE-3368
>             Project: Calcite
>          Issue Type: Bug
>          Components: core
>    Affects Versions: 1.21.0
>            Reporter: Leonard Xu
>            Assignee: Leonard Xu
>            Priority: Major
>              Labels: pull-request-available
>          Time Spent: 1h 10m
>  Remaining Estimate: 0h
>
> 'is null' expression in SQL may be optimized incorrectly in the underlying 
> implementation.
>  
> When I write a Fink SQL to test overflow just like 
> {code:java}
> select 
>    case when (f0 + f1) is null then 'null' else 'not null' end
> from testTable
> {code}
> , I found expression '(f0 + f1) is null ' has been optimized by Calcite, and 
> the optimization may be incorrect.
>  
> The underlying implementation is that Calcite's simplification logic of 
> isNull expression in SQL will convert  from
> *"f(operand0, operand1) IS NULL"* to 
> *"operand0 IS NULL OR operand1 IS NULL"*  if the Policy of  RexNode‘s SqlKind 
> is ANY。
> This simplification  leads to the  expression will not calculate  the real 
> value of  *f(operand0, operand1)* (eg.. '(f0 + f1)' in my case ),but  '(f0 + 
> f1)' maybe overflows after operation. 
> {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) {
>  operands.add(
>  rexBuilder.makeCall(SqlStdOperatorTable.IS_NULL, operand));
>  } else {
>  operands.add(simplified);
>  }
>  }
>  return RexUtil.composeDisjunction(rexBuilder, operands, false);
>  case AS_IS:
>  default:
>  return null;
>  }
> }{code}
> And 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}
>  
> It may be an obvious nonequivalent simplification in SQL. And this issue come 
> from Flink (FLINK-14030).
> [~danny0405], Could you have a look at this?



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to