Alex Behm 534999382d IMPALA-4574: Do not treat UUID() like a constant expr. ...

A recent change (IMPALA-1788) lead UUID() to be constant folded,
and therefore, produce the same value for every invocation across
rows. Similar issues might also occur due to the BE optimizing
UUID() during codegen of scalar-fn-call.h/cc.

The fix is to not treat UUID() like a constant expr in both
the FE and BE.

Discussion:
The fix in this patch is rather blunt, but minimally invasive to
reduce the risk of adding new bugs. Ideally, the constness of an
Expr should be determined in one place and the FE and BE should agree
on which Exprs are constant. I considered the following alternatives
but concluded they were too risky:
1. Pass a flag from FE to BE for ever Expr indicating its constness.
   This simple solution would populate a thrift field with the
   result of Expr.isConstant() for every Expr in an Expr tree.
   There are several issues. Calling isConstant() for every Expr
   in an Expr tree is rather expensive due to repeated traversals
   of the tree. That could be mitigated by populating an isConstant
   flag during Expr.analyze() to avoid re-computing the constness
   repeatedly. This requires changes to analyze(), clone(), reset(),
   and possibly other places for many Exprs. There is potential
   for missing a place and adding a new bug.
2. The above solution could be limited to only FunctionCallExpr.
   However, the BE expr type FUNCTION_CALL which maps to
   scalar-fn-call.h/cc is created from various FE Exprs, not just
   FunctionCallExpr. So adding a flag only to scalar-fn-call.h/cc
   would be confusing because it would only sometimes be set
   in a meaningful way. This seems more confusing than the current
   straightforward solution.

Testing: Added FE and EE tests.

Change-Id: If2499f5f6ecdcb098623202c8e6dc2d02727194a
Reviewed-on: http://gerrit.cloudera.org:8080/5324
Reviewed-by: Alex Behm <alex.behm@cloudera.com>
Tested-by: Internal Jenkins

2016-12-06 22:03:01 +00:00

64 KiB

Raw Blame History

View Raw