Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/24208 )
Change subject: IMPALA-14903: Calcite planner: Simplify code for string literals ...................................................................... IMPALA-14903: Calcite planner: Simplify code for string literals By default, Calcite always sets string literals to type "char" whereas Impala needs string literals to be type "string". The current code committed works around this by creating a cast around the column. This complicated the code. For example, A Values RelNode cannot have a cast, so a Project RelNode had to be created on top of a Values RelNode to handle the cast. The simplification involved two changes: 1) In ImpalaSqlValidatorImpl, deriveType sets the type to STRING at validation time 2) Calcite also creates string literals of type CHAR when doing the SqlNode to RelNode conversion, so it is also changed in RexBuilder while it is still in the analysis phase. Other changes included for this: - As mentioned above, a Project RelNode is no longer needed for casting on top of the Values node, so the code has been removed from CoerceNodes. Also, a ProjectMerge rule has been added at the end of optimization to clean up extra unneeded Project RelNodes. - While there are still some cases where the return type needs to be coerced, there are several cases where this is no longer needed. In CoerceOperandShuttle, the return type is no longer changed; the value from the Validator "inferReturnType" is now always used. - Because the "inferReturnType" is always used, a couple of minor changes were also needed to handle this. While this theoretically should be in its own commit, they are small enough that they are included here. The "abs", "first_value" and "last_value" functions are derived from Impala rather than Calcite now. Also, a method in ImpalaAggOperator was added to allow the use of "ignore nulls" with "first_value" and "last_value". - The RemoveUnraggedCharRexExecutor was created solely for the char casting so it has been removed. - We only want to modify the non-default type during the analysis phase. Once coercion happens, when the type is being determined. During optimization phase, it is possible that a cast of a tinyint to an integer gets simplified to an int literal. There is already code in ImpalaRexBuilder to handle this. However, the post analysis is now set immediately before coercion, allowing some pre-coercion Calcite code to have its literals cast in the right way. - IMPALA-15125 has been fixed with this commit. - A test was removed from test_rows_availability.py because a new plan is generated that is different from the original planner and does not cause the test to sleep. - Some minor changes to the junit tests. -- Some costs are slightly different for q8. The Project over the Values has been eliminated causing an extra predicate to be added. -- q47 and q57 have a "rank() is not null" predicate removed. This changed because the return type is no longer changed, and the nullability of the rank function is now set correctly, and no "is not null" clause is needed. Change-Id: Id8e61b2555afd81ef52f19431fdd1224d4039c00 Reviewed-on: http://gerrit.cloudera.org:8080/24208 Reviewed-by: Impala Public Jenkins <[email protected]> Tested-by: Impala Public Jenkins <[email protected]> --- M java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceNodes.java M java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceOperandShuttle.java M java/calcite-planner/src/main/java/org/apache/impala/calcite/functions/RexLiteralConverter.java M java/calcite-planner/src/main/java/org/apache/impala/calcite/operators/ImpalaAggOperator.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/operators/ImpalaRexBuilder.java M java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaProjectRel.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/rules/ImpalaCoreRules.java M java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaRexExecutor.java D java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/RemoveUnraggedCharCastRexExecutor.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/service/ImpalaSqlValidatorImpl.java M java/calcite-planner/src/main/java/org/apache/impala/calcite/type/ImpalaTypeCoercionImpl.java M testdata/workloads/functional-planner/queries/PlannerTest/calcite/analytic-rank-pushdown-calcite.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q01.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q02.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q03.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q04.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q05.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q06.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q07.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q08.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q09.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q10a.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q11.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q12.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q13.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q14a.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q14b.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q15.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q16.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q17.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q18.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q19.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q20.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q21.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q22.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q23a.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q23b.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q24a.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q24b.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q25.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q26.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q27.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q28.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q29.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q30.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q31.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q32.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q33.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q34.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q35a.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q36.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q37.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q38.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q39a.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q39b.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q40.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q41.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q42.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q43-verbose.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q43.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q44.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q45.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q46.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q47.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q48.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q49.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q50.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q51.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q52.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q53.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q54.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q55.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q56.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q57.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q58.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q59.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q60.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q61.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q62.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q63.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q64.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q65.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q66.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q67.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q68.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q69.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q70.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q71.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q72.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q73.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q74.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q75.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q76.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q77.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q78.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q79.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q80.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q81.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q82.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q83.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q84.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q85.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q86.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q87.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q88.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q89.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q90.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q91.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q92.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q93.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q94.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q95.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q96.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q97.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q98.test M testdata/workloads/functional-planner/queries/PlannerTest/calcite_tpcds/tpcds-q99.test M tests/query_test/test_rows_availability.py 122 files changed, 3,159 insertions(+), 4,646 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/24208 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Id8e61b2555afd81ef52f19431fdd1224d4039c00 Gerrit-Change-Number: 24208 Gerrit-PatchSet: 23 Gerrit-Owner: Steve Carlin <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Steve Carlin <[email protected]>
