Hello Joe McDonnell,

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/21335

to look at the new patch set (#13).

Change subject: IMPALA-13022: Added infrastructure for implicit casting of 
functions
......................................................................

IMPALA-13022: Added infrastructure for implicit casting of functions

Ideally, at validation time,  we would like Calcite to coerce function
parameters and generate the proper return types that match Impala
functions. Currently, Calcite comes close to hitting these needs, and
any function that passes Calcite validation will be a valid function.
However, while some of the types are close, we want the output of the
Calcite optimizer to contain methods where all parameter types and
return types exactly match what is expected by Impala at runtime.

The bulk of this commit is to address this issue. There are a couple of
other parts to this commit added that rely on proper types as well. This
will be described as well further down in the commit message.

The 'fixing' of the parameters and return types is done after
optimization and before the Calcite RelNodes are translated into Impala
RelNodes. The code responsible for this is under the coercenodes directory.

The entry point for fixing these nodes is the method CoerceNodes.coerceNodes()
which takes a RelNode input and produces a RelNode output. This method
will journey through the RelNodes bottom up because a RelNode must be created
with its inputs, so it makes sense to fix the inputs first.

One problem within Calcite is how it generates RexLiterals. For the literal
number 2, Calcite will generate an INTEGER type. However, the Impala output
for this is a TINYINT.  Another Calcite issue is for a string literal
such as 'hello'. Calcite will generate a CHAR(5) type whereas Impala
generates a string.

These inconsistencies cause Impala to crash in certain situations. For instance,
the query "select 'hello' union select 'goodbye'" generates a union with
2 input nodes. If we were to use the Calcite definitions, one would have
type of char(5) and the other would have type of char(7), which would cause
a crash.

Also, eventually when CTAS statements are implemented, we need the types to
match Impala's native type.

Most of the Calcite RelNode types need some sort of correction due to these
issues. Join, Filter and Project nodes may have expressions that need fixing.
The CoerceOperandShuttle class helps navigating through the RexNode function
structure for these. The Aggregate class may require an underlying Project
class where the explanation is detailed in the processAggNode method. The
Union node also will generate underlying Project nodes for casting. The
Values node creates a Project above itself since Calcite will not allow
generation of a RexLiteral of string type directly, so it needs to be cast.

In addition to these changes, other changes that relied on casting issues
have been added.  The or/and/case operator is now supported. 'Or' and 'and'
worked before, but not if there were more than 2 or/and conditions grouped
together. Case requires all return types to match and required some
special logic.  These functions required a little extra support since
they have variable arguments.

Change-Id: I13a349673f185463276ad7ddb83f0cfc2d73218c
---
A 
java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceNodes.java
A 
java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceOperandShuttle.java
A 
java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/AnalyzedCaseExpr.java
A 
java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/AnalyzedCastExpr.java
M 
java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/AnalyzedFunctionCallExpr.java
M 
java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/FunctionResolver.java
A 
java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/ImplicitTypeChecker.java
M 
java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/RexCallConverter.java
M 
java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/RexLiteralConverter.java
A 
java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaConvertletTable.java
A 
java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaCustomOperatorTable.java
M 
java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaOperator.java
M 
java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaOperatorTable.java
M 
java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaAggRel.java
M 
java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaJoinRel.java
M 
java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaPlanRel.java
M 
java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaValuesRel.java
M 
java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ParentPlanRelContext.java
M 
java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java
M 
java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteRelNodeConverter.java
M 
java/calcite-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeConverter.java
M 
java/calcite-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeSystemImpl.java
M testdata/workloads/functional-query/queries/QueryTest/calcite.test
23 files changed, 2,217 insertions(+), 74 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/35/21335/13
--
To view, visit http://gerrit.cloudera.org:8080/21335
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I13a349673f185463276ad7ddb83f0cfc2d73218c
Gerrit-Change-Number: 21335
Gerrit-PatchSet: 13
Gerrit-Owner: Steve Carlin <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>

Reply via email to