You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Andrew Sherman (Code Review)" <ge...@cloudera.org> on 2018/10/18 00:37:01 UTC

[Impala-ASF-CR] IMPALA-5821: Add query with implicit casts to extended explain output.

Andrew Sherman has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11719


Change subject: IMPALA-5821: Add query with implicit casts to extended explain output.
......................................................................

IMPALA-5821: Add query with implicit casts to extended explain output.

If explain_level is at 'extended' level or higher, then print the
analyzed query as text in the explain header to aid in diagnosis. This
query text is generated from the analyzed statement tree, so it includes
query rewriting. In addition implicit casts are printed, and numeric
literals are printed with a cast so that their type is visible.

If SHOW_IMPLICIT_CASTS is passed to toSql() then
- in CastExpr print the implicit cast
- in NumericLiteral print the literal with a cast to show the type

Add a PlannerTestOption directive that will force the query text showing
implicit casts to be included in the PLAN section of a .test file.

The toSql() method can be called on a ParseNode tree to return
the sql corresponding ot the tree. In the past toSQl() has been
enhanced to print rewritten sql by partially overloading toSql() [with
toSql(boolean)]. This current change requires changing toSQl() in
many places as NumericLiteral can appear at different points in ia
parse tree. To avoid many new fragile overloads of toSql() I added
toSql(ToSqlOptions), where ToSqlOptions is an enum which controls the
form of the Sql that is returned. This changes many files but is safer
and means that any future options to toSql() can be added painlessly.

If a query in a .test file produces a diff when run by PlannerTest,
then print the name of the .test file in the output.

Documentation of this change will be done as IMPALA-7718

EXAMPLE OUTPUT:

[localhost:21000] default> set explain_level=extended;
EXPLAIN_LEVEL set to extended
[localhost:21000] default> explain select * from functional_kudu.alltypestiny where bigint_col < 1000 / 100;
Query: explain select * from functional_kudu.alltypestiny where bigint_col < 1000 / 100
Query with implicit casts: SELECT * FROM functional_kudu.alltypestiny WHERE CAST(bigint_col AS DOUBLE) < CAST(10 AS DOUBLE)
Max Per-Host Resource Reservation: Memory=0B Threads=2
Per-Host Resource Estimates: Memory=10MB
Codegen disabled by planner
""
F00:PLAN FRAGMENT [UNPARTITIONED] hosts=1 instances=1
|  Per-Host Resources: mem-estimate=0B mem-reservation=0B thread-reservation=2
(etc.)

Note that the initial "Query:" that is printed originates from
impala-shell (suppressed with --quiet), but for neatness I put the new
"Query with implicit casts:" output at the top of the header.

TESTING:

All end-to-end tests pass.
Added a new test in ExprRewriterTest which prints sql with implict casts
for some interesting queries.
The output of some Planner Tests in .test files has been updated to
include the sql with implict casts that is printed when explain_level is
at at least 'extended' level.

QUESTIONS FOR REVIEWERS:

Is "Query with implicit casts:" a good description?
Is the EXPLAIN header the right place for this output?

Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
---
M fe/src/main/java/org/apache/impala/analysis/AdminFnStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableAddDropRangePartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java
M fe/src/main/java/org/apache/impala/analysis/AnalyticWindow.java
M fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java
M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
M fe/src/main/java/org/apache/impala/analysis/BetweenPredicate.java
M fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java
M fe/src/main/java/org/apache/impala/analysis/BoolLiteral.java
M fe/src/main/java/org/apache/impala/analysis/CaseExpr.java
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
M fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java
M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateDataSrcStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateDropRoleStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
M fe/src/main/java/org/apache/impala/analysis/DescribeDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDataSrcStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropFunctionStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/ExistsPredicate.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/ExtractFromExpr.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/FunctionArgs.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/analysis/GrantRevokePrivStmt.java
M fe/src/main/java/org/apache/impala/analysis/GrantRevokeRoleStmt.java
M fe/src/main/java/org/apache/impala/analysis/HdfsCachingOp.java
M fe/src/main/java/org/apache/impala/analysis/InPredicate.java
M fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/IsNotEmptyPredicate.java
M fe/src/main/java/org/apache/impala/analysis/IsNullPredicate.java
M fe/src/main/java/org/apache/impala/analysis/KuduPartitionExpr.java
M fe/src/main/java/org/apache/impala/analysis/KuduPartitionParam.java
M fe/src/main/java/org/apache/impala/analysis/LikePredicate.java
M fe/src/main/java/org/apache/impala/analysis/LimitElement.java
M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/NullLiteral.java
M fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java
M fe/src/main/java/org/apache/impala/analysis/OrderByElement.java
M fe/src/main/java/org/apache/impala/analysis/ParseNode.java
M fe/src/main/java/org/apache/impala/analysis/PartitionDef.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSet.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java
M fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
M fe/src/main/java/org/apache/impala/analysis/RangePartition.java
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectListItem.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/SetStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowCreateFunctionStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowCreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowDataSrcsStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowDbsStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowFilesStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowFunctionsStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowGrantPrincipalStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowRolesStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowTablesStmt.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/analysis/StatementBase.java
M fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java
M fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
M fe/src/main/java/org/apache/impala/analysis/Subquery.java
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
M fe/src/main/java/org/apache/impala/analysis/TableSampleClause.java
M fe/src/main/java/org/apache/impala/analysis/TimestampArithmeticExpr.java
M fe/src/main/java/org/apache/impala/analysis/TimestampLiteral.java
A fe/src/main/java/org/apache/impala/analysis/ToSqlOptions.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/analysis/TruncateStmt.java
M fe/src/main/java/org/apache/impala/analysis/TupleIsNullPredicate.java
M fe/src/main/java/org/apache/impala/analysis/TypeDef.java
M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
M fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java
M fe/src/main/java/org/apache/impala/analysis/UseStmt.java
M fe/src/main/java/org/apache/impala/analysis/ValidTupleIdExpr.java
M fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java
M fe/src/main/java/org/apache/impala/analysis/WithClause.java
M fe/src/main/java/org/apache/impala/planner/JoinBuildSink.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/max-row-size.test
M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
M testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level2.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level3.test
M testdata/workloads/functional-query/queries/QueryTest/stats-extrapolation.test
111 files changed, 847 insertions(+), 307 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/19/11719/1
-- 
To view, visit http://gerrit.cloudera.org:8080/11719
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
Gerrit-Change-Number: 11719
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>

[Impala-ASF-CR] IMPALA-5821: Add query with implicit casts to extended explain output.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11719 )

Change subject: IMPALA-5821: Add query with implicit casts to extended explain output.
......................................................................


Patch Set 7: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/11719
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
Gerrit-Change-Number: 11719
Gerrit-PatchSet: 7
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Nov 2018 23:31:30 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5821: Add query with implicit casts to extended explain output.

Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11719 )

Change subject: IMPALA-5821: Add query with implicit casts to extended explain output.
......................................................................


Patch Set 4:

(15 comments)

overall good cleanup. mostly minor comments from my end.

http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/main/java/org/apache/impala/analysis/ParseNode.java
File fe/src/main/java/org/apache/impala/analysis/ParseNode.java:

http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/main/java/org/apache/impala/analysis/ParseNode.java@32
PS4, Line 32: sql
nit: pls use consistent case (SQL, sql, Sql)


http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/main/java/org/apache/impala/analysis/ParseNode.java@32
PS4, Line 32: @param
pls omit these javadoc annotations.


http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/main/java/org/apache/impala/analysis/ParseNode.java@39
PS4, Line 39:    * This should return the same result as calling toSql(ToSqlOptions.DEFAULT).
would be good to do this via a default interface method. pls add a todo for when we fully move to Java 8.


http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/main/java/org/apache/impala/analysis/ToSqlOptions.java
File fe/src/main/java/org/apache/impala/analysis/ToSqlOptions.java:

http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/main/java/org/apache/impala/analysis/ToSqlOptions.java@25
PS4, Line 25: default, normal
"normal" makes sense if you know what is currently done. perhaps "original query without rewrites"?


http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/main/java/org/apache/impala/analysis/ToSqlOptions.java@38
PS4, Line 38:   // This does have the consequence that the sql with implict casts may not pssibly fail
nit: spelling


http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/main/java/org/apache/impala/analysis/ToSqlOptions.java@38
PS4, Line 38: may not pssibly fail
            :   // to parse if resubmitted as
I found this difficult to understand. Did you mean that Sql produced in this mode may not be valid Sql?

I'd prefer the comment be merged with the comment block on L35.


http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/main/java/org/apache/impala/analysis/TypeDef.java
File fe/src/main/java/org/apache/impala/analysis/TypeDef.java:

http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/main/java/org/apache/impala/analysis/TypeDef.java@165
PS4, Line 165:     return parsedType_.toSql();
pass down here?


http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/main/java/org/apache/impala/common/PrintUtils.java
File fe/src/main/java/org/apache/impala/common/PrintUtils.java:

http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/main/java/org/apache/impala/common/PrintUtils.java@110
PS4, Line 110: 32
what's this?


http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/main/java/org/apache/impala/common/PrintUtils.java@115
PS4, Line 115: exiting
sp. existing?


http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
File fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java:

http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java@424
PS4, Line 424: @pa
remove


http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java@451
PS4, Line 451:     // AnalyzesOk(stmt.toSql(true), ctx);
rm?


http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
File fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java:

http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java@750
PS4, Line 750: @para
remove the comment annotations.


http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java@801
PS4, Line 801: This would also be included if both
more direct: Equivalent to enabling INCLUDE_EXPLAIN_HEADER and EXTENDED_EXPLAIN.


http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/test/java/org/apache/impala/util/PrintUtilsTest.java
File fe/src/test/java/org/apache/impala/util/PrintUtilsTest.java:

http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/test/java/org/apache/impala/util/PrintUtilsTest.java@128
PS4, Line 128: @pa
rm


http://gerrit.cloudera.org:8080/#/c/11719/4/testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test
File testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test:

http://gerrit.cloudera.org:8080/#/c/11719/4/testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test@9
PS4, Line 9: -- +straight_join
           : * 
were you expecting the hint to print this way?



-- 
To view, visit http://gerrit.cloudera.org:8080/11719
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
Gerrit-Change-Number: 11719
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 31 Oct 2018 21:30:33 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5821: Add query with implicit casts to extended explain output.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11719 )

Change subject: IMPALA-5821: Add query with implicit casts to extended explain output.
......................................................................


Patch Set 8:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/1367/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/11719
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
Gerrit-Change-Number: 11719
Gerrit-PatchSet: 8
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 14 Nov 2018 22:04:34 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5821: Add query with implicit casts to extended explain output.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11719 )

Change subject: IMPALA-5821: Add query with implicit casts to extended explain output.
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/1181/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/11719
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
Gerrit-Change-Number: 11719
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Fri, 26 Oct 2018 22:30:44 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5821: Add query with implicit casts to extended explain output.

Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11719 )

Change subject: IMPALA-5821: Add query with implicit casts to extended explain output.
......................................................................


Patch Set 5:

(4 comments)

While here, do you have thoughts on how to maintain this going forward? Main concern is that a "toSql()" (default options) will be used and we'll forget to look for casts, if expected. I see that "toSql()" is used in many places other than this use-case so I'd lean towards follow-up changes to require the option and see if we can replace the other "toSql()" calls with something more explicit for their use. I'm fine with a todo/jira.

http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/main/java/org/apache/impala/common/PrintUtils.java
File fe/src/main/java/org/apache/impala/common/PrintUtils.java:

http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/main/java/org/apache/impala/common/PrintUtils.java@110
PS4, Line 110: 32
> Just some padding for if lines expand. Maybe this is too ugly.
ok, just mention it in a brief comment. I prefer such inline constants to have some explanation. It seems to be an optimization so I'm also ok with removing it to simplify.


http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
File fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java:

http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java@424
PS4, Line 424: col
> I don't really understand what is wrong with param? 
nothing's wrong with it. my comments are motivated to keep things consistent.


http://gerrit.cloudera.org:8080/#/c/11719/5/fe/src/test/java/org/apache/impala/util/PrintUtilsTest.java
File fe/src/test/java/org/apache/impala/util/PrintUtilsTest.java:

http://gerrit.cloudera.org:8080/#/c/11719/5/fe/src/test/java/org/apache/impala/util/PrintUtilsTest.java@89
PS5, Line 89: *
nit: looks like there's still an extra '*' here.


http://gerrit.cloudera.org:8080/#/c/11719/4/testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test
File testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test:

http://gerrit.cloudera.org:8080/#/c/11719/4/testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test@9
PS4, Line 9: -- +straight_join
           : * 
> Yes. Though I welcome your opinion.
that's fine by me



-- 
To view, visit http://gerrit.cloudera.org:8080/11719
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
Gerrit-Change-Number: 11719
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 01 Nov 2018 16:21:52 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5821: Add query with implicit casts to extended explain output.

Posted by "Thomas Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Marshall has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11719 )

Change subject: IMPALA-5821: Add query with implicit casts to extended explain output.
......................................................................

IMPALA-5821: Add query with implicit casts to extended explain output.

If explain_level is at 'extended' level or higher, then enhance the
output from the explain command. (1) Show the analyzed sql in the
explain header, this is the rewritten sql, which includes implicit
casts, and literals are printed with a cast so that their type is
visible. (2) When predicates are shown in the plan these are shown in
the same format.

The toSql() method can be called on a ParseNode tree to return
the sql corresponding ot the tree. In the past toSQl() has been
enhanced to print rewritten sql by partially overloading toSql() [with
toSql(boolean)]. This current change requires changing toSQl() in
many places as NumericLiteral can appear at different points in ia
parse tree. To avoid many new fragile overloads of toSql() I added
toSql(ToSqlOptions), where ToSqlOptions is an enum which controls the
form of the Sql that is returned. This changes many files but is safer
and means that any future options to toSql() can be added painlessly.

If SHOW_IMPLICIT_CASTS is passed to toSql() then
- in CastExpr print the implicit cast
- in NumericLiteral print the literal with a cast to show the type

Add a PlannerTestOption directive that will force the query text showing
implicit casts to be included in the PLAN section of a .test file.

The analyzed query text is wrapped at 80 characters. Note that the
analyzed query cannot always be executed as queries rewritten to use
LEFT SEMI JOIN are not legal sql. In addition some space characters may
be removed from the query for prettier display.

Documentation of this change will be done as IMPALA-7718

EXAMPLE OUTPUT:

[localhost:21000] default> set explain_level=2;
EXPLAIN_LEVEL set to 2
[localhost:21000] default> explain select * from functional_kudu.alltypestiny where bigint_col < 1000 / 100;
Query: explain select * from functional_kudu.alltypestiny where bigint_col < 1000 / 100
Max Per-Host Resource Reservation: Memory=0B Threads=2
Per-Host Resource Estimates: Memory=10MB
Codegen disabled by planner
Analyzed query: SELECT * FROM functional_kudu.alltypestiny WHERE CAST(bigint_col
AS DOUBLE) < CAST(10 AS DOUBLE)
""
F00:PLAN FRAGMENT [UNPARTITIONED] hosts=1 instances=1
|  Per-Host Resources: mem-estimate=4.88MB mem-reservation=0B thread-reservation=2
PLAN-ROOT SINK
|  mem-estimate=0B mem-reservation=0B thread-reservation=0
|
00:SCAN KUDU [functional_kudu.alltypestiny]
   predicates: CAST(bigint_col AS DOUBLE) < CAST(10 AS DOUBLE)
   mem-estimate=4.88MB mem-reservation=0B thread-reservation=1
   tuple-ids=0 row-size=97B cardinality=1
   in pipelines: 00(GETNEXT)
Fetched 16 row(s) in 0.03s

TESTING:

All end-to-end tests pass.
Added a new test in ExprRewriterTest which prints sql with implict casts
for some interesting queries.
Add a unit test for the code which wraps text at 60 characters.
The output of some Planner Tests in .test files has been updated to
include the Analyzed sql that is printed when explain_level is
at at least 'extended' level.

Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
Reviewed-on: http://gerrit.cloudera.org:8080/11719
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Thomas Marshall <th...@cmu.edu>
---
M fe/src/main/java/org/apache/impala/analysis/AdminFnStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableAddDropRangePartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java
M fe/src/main/java/org/apache/impala/analysis/AnalyticWindow.java
M fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java
M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
M fe/src/main/java/org/apache/impala/analysis/BetweenPredicate.java
M fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java
M fe/src/main/java/org/apache/impala/analysis/BoolLiteral.java
M fe/src/main/java/org/apache/impala/analysis/CaseExpr.java
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
M fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java
M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateDataSrcStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateDropRoleStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
M fe/src/main/java/org/apache/impala/analysis/DescribeDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDataSrcStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropFunctionStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/ExistsPredicate.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/ExtractFromExpr.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/FunctionArgs.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/analysis/GrantRevokePrivStmt.java
M fe/src/main/java/org/apache/impala/analysis/GrantRevokeRoleStmt.java
M fe/src/main/java/org/apache/impala/analysis/HdfsCachingOp.java
M fe/src/main/java/org/apache/impala/analysis/InPredicate.java
M fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/IsNotEmptyPredicate.java
M fe/src/main/java/org/apache/impala/analysis/IsNullPredicate.java
M fe/src/main/java/org/apache/impala/analysis/KuduPartitionExpr.java
M fe/src/main/java/org/apache/impala/analysis/KuduPartitionParam.java
M fe/src/main/java/org/apache/impala/analysis/LikePredicate.java
M fe/src/main/java/org/apache/impala/analysis/LimitElement.java
M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/NullLiteral.java
M fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java
M fe/src/main/java/org/apache/impala/analysis/OrderByElement.java
M fe/src/main/java/org/apache/impala/analysis/ParseNode.java
M fe/src/main/java/org/apache/impala/analysis/PartitionDef.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSet.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java
M fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
M fe/src/main/java/org/apache/impala/analysis/RangePartition.java
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectListItem.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/SetStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowCreateFunctionStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowCreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowDataSrcsStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowDbsStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowFilesStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowFunctionsStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowGrantPrincipalStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowRolesStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowTablesStmt.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/analysis/StatementBase.java
M fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java
M fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
M fe/src/main/java/org/apache/impala/analysis/Subquery.java
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
M fe/src/main/java/org/apache/impala/analysis/TableSampleClause.java
M fe/src/main/java/org/apache/impala/analysis/TimestampArithmeticExpr.java
M fe/src/main/java/org/apache/impala/analysis/TimestampLiteral.java
A fe/src/main/java/org/apache/impala/analysis/ToSqlOptions.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/analysis/TruncateStmt.java
M fe/src/main/java/org/apache/impala/analysis/TupleIsNullPredicate.java
M fe/src/main/java/org/apache/impala/analysis/TypeDef.java
M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
M fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java
M fe/src/main/java/org/apache/impala/analysis/UseStmt.java
M fe/src/main/java/org/apache/impala/analysis/ValidTupleIdExpr.java
M fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java
M fe/src/main/java/org/apache/impala/analysis/WithClause.java
M fe/src/main/java/org/apache/impala/common/PrintUtils.java
M fe/src/main/java/org/apache/impala/planner/AggregationNode.java
M fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java
M fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java
M fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java
M fe/src/main/java/org/apache/impala/planner/HashJoinNode.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/JoinBuildSink.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/planner/NestedLoopJoinNode.java
M fe/src/main/java/org/apache/impala/planner/PlanNode.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/planner/SelectNode.java
M fe/src/main/java/org/apache/impala/planner/SubplanNode.java
M fe/src/main/java/org/apache/impala/planner/UnionNode.java
M fe/src/main/java/org/apache/impala/planner/UnnestNode.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
M fe/src/test/java/org/apache/impala/util/PrintUtilsTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/fk-pk-join-detection.test
M testdata/workloads/functional-planner/queries/PlannerTest/kudu-selectivity.test
M testdata/workloads/functional-planner/queries/PlannerTest/max-row-size.test
M testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering-disabled.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test
M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
M testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test
M testdata/workloads/functional-planner/queries/PlannerTest/tablesample.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level2.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level3.test
M testdata/workloads/functional-query/queries/QueryTest/parquet-stats.test
M testdata/workloads/functional-query/queries/QueryTest/stats-extrapolation.test
133 files changed, 1,341 insertions(+), 460 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Thomas Marshall: Looks good to me, approved

-- 
To view, visit http://gerrit.cloudera.org:8080/11719
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
Gerrit-Change-Number: 11719
Gerrit-PatchSet: 10
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>

[Impala-ASF-CR] IMPALA-5821: Add query with implicit casts to extended explain output.

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/11719 )

Change subject: IMPALA-5821: Add query with implicit casts to extended explain output.
......................................................................


Patch Set 1:

(4 comments)

Nice change, I really missed this feature!

http://gerrit.cloudera.org:8080/#/c/11719/1/fe/src/main/java/org/apache/impala/analysis/ParseNode.java
File fe/src/main/java/org/apache/impala/analysis/ParseNode.java:

http://gerrit.cloudera.org:8080/#/c/11719/1/fe/src/main/java/org/apache/impala/analysis/ParseNode.java@30
PS1, Line 30:   /**
            :    * Returns the SQL string corresponding to this node.
            :    */
Can you explain the relation of the two toSql() functions in the comment? If I understand correctly, then toSql() should always call toSql(ToSqlOptions options) with default option.

At the first glance I thought that the parameterless toSql should be removed to avoid possible mistakes, but then I realized the huge number of places where it is called, so now I agree with this solution.


http://gerrit.cloudera.org:8080/#/c/11719/1/fe/src/main/java/org/apache/impala/analysis/ToSqlOptions.java
File fe/src/main/java/org/apache/impala/analysis/ToSqlOptions.java:

http://gerrit.cloudera.org:8080/#/c/11719/1/fe/src/main/java/org/apache/impala/analysis/ToSqlOptions.java@1
PS1, Line 1: package org.apache.impala.analysis;
Apache license is missing (also noticed by the jenkins job).


http://gerrit.cloudera.org:8080/#/c/11719/1/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
File fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java:

http://gerrit.cloudera.org:8080/#/c/11719/1/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java@344
PS1, Line 344: TestToSqlWithImplicitCasts
Can you add a test (or maybe a few) to check that nested structures like sub queries and expression trees propagate the option?


http://gerrit.cloudera.org:8080/#/c/11719/1/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
File fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java:

http://gerrit.cloudera.org:8080/#/c/11719/1/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java@546
PS1, Line 546:         errorLog.append("\nTest file: " + testFile + ".test"
             :             + "\n");
nit: fits to one line



-- 
To view, visit http://gerrit.cloudera.org:8080/11719
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
Gerrit-Change-Number: 11719
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Oct 2018 16:59:52 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5821: Add query with implicit casts to extended explain output.

Posted by "Thomas Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/11719 )

Change subject: IMPALA-5821: Add query with implicit casts to extended explain output.
......................................................................


Patch Set 2:

(3 comments)

> > It seems good to me, but I would ping Greg Rahn in the Jira,
 > maybe
 > > he has some ideas.
 > 
 > I have pinged him
 > 
 > > I have one issue with the current output: as I saw in the .test
 > > files, queries are printed in one line by default, which can make
 > > complex queries very difficult to read. It would be much nicer to
 > > break them at logical points, but I have no idea how to do it
 > > easily.
 > 
 > I agree that a clever printing would be nice, but that sounds
 > tricky. I could wrap at say the last space before 80 columns with
 > only small effort, but this will make the header harder to parse in
 > tests. Should I try this?

Doing the simple thing (wrapping at 80) is probably fine.

http://gerrit.cloudera.org:8080/#/c/11719/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11719/2//COMMIT_MSG@32
PS2, Line 32: If a query in a .test file produces a diff when run by PlannerTest,
            : then print the name of the .test file in the output.
Could you submit this as a standalone review?

If I understand correctly, it doesn't really have anything to do with the rest of the review, and its nice to keep changes relatively focused, esp. since this is already such a large patch.


http://gerrit.cloudera.org:8080/#/c/11719/2//COMMIT_MSG@67
PS2, Line 67: Is "Query with implicit casts:" a good description?
Any reason not to also show the rewritten sql here and call this something like "Analyzed query"


http://gerrit.cloudera.org:8080/#/c/11719/2/testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
File testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test:

http://gerrit.cloudera.org:8080/#/c/11719/2/testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test@59
PS2, Line 59: c_custkey > 10
not sure how difficult this would be, but it would be nice to include the implicit casts here too



-- 
To view, visit http://gerrit.cloudera.org:8080/11719
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
Gerrit-Change-Number: 11719
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Fri, 19 Oct 2018 22:58:18 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5821: Add query with implicit casts to extended explain output.

Posted by "Andrew Sherman (Code Review)" <ge...@cloudera.org>.
Andrew Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/11719 )

Change subject: IMPALA-5821: Add query with implicit casts to extended explain output.
......................................................................


Patch Set 2:

> It seems good to me, but I would ping Greg Rahn in the Jira, maybe
 > he has some ideas.
 
I have pinged him
 
 > I have one issue with the current output: as I saw in the .test
 > files, queries are printed in one line by default, which can make
 > complex queries very difficult to read. It would be much nicer to
 > break them at logical points, but I have no idea how to do it
 > easily.

I agree that a clever printing would be nice, but that sounds tricky. I could wrap at say the last space before 80 columns with only small effort, but this will make the header harder to parse in tests. Should I try this?


-- 
To view, visit http://gerrit.cloudera.org:8080/11719
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
Gerrit-Change-Number: 11719
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Oct 2018 17:04:34 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5821: Add query with implicit casts to extended explain output.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11719 )

Change subject: IMPALA-5821: Add query with implicit casts to extended explain output.
......................................................................


Patch Set 9: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/11719
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
Gerrit-Change-Number: 11719
Gerrit-PatchSet: 9
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 15 Nov 2018 21:31:32 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5821: Add query with implicit casts to extended explain output.

Posted by "Andrew Sherman (Code Review)" <ge...@cloudera.org>.
Andrew Sherman has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/11719 )

Change subject: IMPALA-5821: Add query with implicit casts to extended explain output.
......................................................................

IMPALA-5821: Add query with implicit casts to extended explain output.

If explain_level is at 'extended' level or higher, then print the
analyzed query as text in the explain header to aid in diagnosis. This
query text is generated from the analyzed statement tree, so it includes
query rewriting. In addition implicit casts are printed, and numeric
literals are printed with a cast so that their type is visible.

If SHOW_IMPLICIT_CASTS is passed to toSql() then
- in CastExpr print the implicit cast
- in NumericLiteral print the literal with a cast to show the type

Add a PlannerTestOption directive that will force the query text showing
implicit casts to be included in the PLAN section of a .test file.

The toSql() method can be called on a ParseNode tree to return
the sql corresponding ot the tree. In the past toSQl() has been
enhanced to print rewritten sql by partially overloading toSql() [with
toSql(boolean)]. This current change requires changing toSQl() in
many places as NumericLiteral can appear at different points in ia
parse tree. To avoid many new fragile overloads of toSql() I added
toSql(ToSqlOptions), where ToSqlOptions is an enum which controls the
form of the Sql that is returned. This changes many files but is safer
and means that any future options to toSql() can be added painlessly.

If a query in a .test file produces a diff when run by PlannerTest,
then print the name of the .test file in the output.

Documentation of this change will be done as IMPALA-7718

EXAMPLE OUTPUT:

[localhost:21000] default> set explain_level=extended;
EXPLAIN_LEVEL set to extended
[localhost:21000] default> explain select * from functional_kudu.alltypestiny where bigint_col < 1000 / 100;
Query: explain select * from functional_kudu.alltypestiny where bigint_col < 1000 / 100
Query with implicit casts: SELECT * FROM functional_kudu.alltypestiny WHERE CAST(bigint_col AS DOUBLE) < CAST(10 AS DOUBLE)
Max Per-Host Resource Reservation: Memory=0B Threads=2
Per-Host Resource Estimates: Memory=10MB
Codegen disabled by planner
""
F00:PLAN FRAGMENT [UNPARTITIONED] hosts=1 instances=1
|  Per-Host Resources: mem-estimate=0B mem-reservation=0B thread-reservation=2
(etc.)

Note that the initial "Query:" that is printed originates from
impala-shell (suppressed with --quiet), but for neatness I put the new
"Query with implicit casts:" output at the top of the header.

TESTING:

All end-to-end tests pass.
Added a new test in ExprRewriterTest which prints sql with implict casts
for some interesting queries.
The output of some Planner Tests in .test files has been updated to
include the sql with implict casts that is printed when explain_level is
at at least 'extended' level.

QUESTIONS FOR REVIEWERS:

Is "Query with implicit casts:" a good description?
Is the EXPLAIN header the right place for this output?

Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
---
M fe/src/main/java/org/apache/impala/analysis/AdminFnStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableAddDropRangePartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java
M fe/src/main/java/org/apache/impala/analysis/AnalyticWindow.java
M fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java
M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
M fe/src/main/java/org/apache/impala/analysis/BetweenPredicate.java
M fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java
M fe/src/main/java/org/apache/impala/analysis/BoolLiteral.java
M fe/src/main/java/org/apache/impala/analysis/CaseExpr.java
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
M fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java
M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateDataSrcStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateDropRoleStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
M fe/src/main/java/org/apache/impala/analysis/DescribeDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDataSrcStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropFunctionStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/ExistsPredicate.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/ExtractFromExpr.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/FunctionArgs.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/analysis/GrantRevokePrivStmt.java
M fe/src/main/java/org/apache/impala/analysis/GrantRevokeRoleStmt.java
M fe/src/main/java/org/apache/impala/analysis/HdfsCachingOp.java
M fe/src/main/java/org/apache/impala/analysis/InPredicate.java
M fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/IsNotEmptyPredicate.java
M fe/src/main/java/org/apache/impala/analysis/IsNullPredicate.java
M fe/src/main/java/org/apache/impala/analysis/KuduPartitionExpr.java
M fe/src/main/java/org/apache/impala/analysis/KuduPartitionParam.java
M fe/src/main/java/org/apache/impala/analysis/LikePredicate.java
M fe/src/main/java/org/apache/impala/analysis/LimitElement.java
M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/NullLiteral.java
M fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java
M fe/src/main/java/org/apache/impala/analysis/OrderByElement.java
M fe/src/main/java/org/apache/impala/analysis/ParseNode.java
M fe/src/main/java/org/apache/impala/analysis/PartitionDef.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSet.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java
M fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
M fe/src/main/java/org/apache/impala/analysis/RangePartition.java
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectListItem.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/SetStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowCreateFunctionStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowCreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowDataSrcsStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowDbsStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowFilesStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowFunctionsStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowGrantPrincipalStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowRolesStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowTablesStmt.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/analysis/StatementBase.java
M fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java
M fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
M fe/src/main/java/org/apache/impala/analysis/Subquery.java
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
M fe/src/main/java/org/apache/impala/analysis/TableSampleClause.java
M fe/src/main/java/org/apache/impala/analysis/TimestampArithmeticExpr.java
M fe/src/main/java/org/apache/impala/analysis/TimestampLiteral.java
A fe/src/main/java/org/apache/impala/analysis/ToSqlOptions.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/analysis/TruncateStmt.java
M fe/src/main/java/org/apache/impala/analysis/TupleIsNullPredicate.java
M fe/src/main/java/org/apache/impala/analysis/TypeDef.java
M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
M fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java
M fe/src/main/java/org/apache/impala/analysis/UseStmt.java
M fe/src/main/java/org/apache/impala/analysis/ValidTupleIdExpr.java
M fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java
M fe/src/main/java/org/apache/impala/analysis/WithClause.java
M fe/src/main/java/org/apache/impala/planner/JoinBuildSink.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/max-row-size.test
M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
M testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level2.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level3.test
M testdata/workloads/functional-query/queries/QueryTest/stats-extrapolation.test
111 files changed, 864 insertions(+), 307 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/19/11719/2
-- 
To view, visit http://gerrit.cloudera.org:8080/11719
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
Gerrit-Change-Number: 11719
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-5821: Add query with implicit casts to extended explain output.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11719 )

Change subject: IMPALA-5821: Add query with implicit casts to extended explain output.
......................................................................


Patch Set 6:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/1251/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/11719
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
Gerrit-Change-Number: 11719
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 01 Nov 2018 21:19:46 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5821: Add query with implicit casts to extended explain output.

Posted by "Thomas Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/11719 )

Change subject: IMPALA-5821: Add query with implicit casts to extended explain output.
......................................................................


Patch Set 9: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/11719
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
Gerrit-Change-Number: 11719
Gerrit-PatchSet: 9
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 15 Nov 2018 21:32:05 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5821: Add query with implicit casts to extended explain output.

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/11719 )

Change subject: IMPALA-5821: Add query with implicit casts to extended explain output.
......................................................................


Patch Set 4:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/11719/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11719/4//COMMIT_MSG@51
PS4, Line 51: ""
This quote seems strange here - is it included in the output?


http://gerrit.cloudera.org:8080/#/c/11719/4//COMMIT_MSG@69
PS4, Line 69: 80
Isn't it 60?


http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
File fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java:

http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java@772
PS4, Line 772:           if (line.contains("Analyzed query:")) {
             :             builder.append(line).append("\n");
             :             inImplictCasts = true;
             :           } else if (inImplictCasts) {
             :             // Keep copying the query with implicit casts.
             :             // This works because this is the last thing in the header.
             :             builder.append(line).append("\n");
             :           }
optional: this could be expressed in a more compact way:

inImplictCasts |= line.contains("Analyzed query:");
if (inImplictCasts) builder.append(line).append("\n");


http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/test/java/org/apache/impala/util/PrintUtilsTest.java
File fe/src/test/java/org/apache/impala/util/PrintUtilsTest.java:

http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/test/java/org/apache/impala/util/PrintUtilsTest.java@83
PS4, Line 83: wrap length for testWrapText() - less than 80 to make test layout nicer
nit: Capital W + . at the end.


http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/test/java/org/apache/impala/util/PrintUtilsTest.java@89
PS4, Line 89: *
nit: extra *


http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/test/java/org/apache/impala/util/PrintUtilsTest.java@127
PS4, Line 127:    * Check that code that has been wrapped is correctly formatted
nit: missing .



-- 
To view, visit http://gerrit.cloudera.org:8080/11719
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
Gerrit-Change-Number: 11719
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 31 Oct 2018 15:31:26 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5821: Add query with implicit casts to extended explain output.

Posted by "Andrew Sherman (Code Review)" <ge...@cloudera.org>.
Andrew Sherman has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/11719 )

Change subject: IMPALA-5821: Add query with implicit casts to extended explain output.
......................................................................

IMPALA-5821: Add query with implicit casts to extended explain output.

If explain_level is at 'extended' level or higher, then enhance the
output from the explain command. (1) Show the analyzed sql in the
explain header, this is the rewritten sql, which includes implicit
casts, and literals are printed with a cast so that their type is
visible. (2) When predicates are shown in the plan these are shown in
the same format.

The toSql() method can be called on a ParseNode tree to return
the sql corresponding ot the tree. In the past toSQl() has been
enhanced to print rewritten sql by partially overloading toSql() [with
toSql(boolean)]. This current change requires changing toSQl() in
many places as NumericLiteral can appear at different points in ia
parse tree. To avoid many new fragile overloads of toSql() I added
toSql(ToSqlOptions), where ToSqlOptions is an enum which controls the
form of the Sql that is returned. This changes many files but is safer
and means that any future options to toSql() can be added painlessly.

If SHOW_IMPLICIT_CASTS is passed to toSql() then
- in CastExpr print the implicit cast
- in NumericLiteral print the literal with a cast to show the type

Add a PlannerTestOption directive that will force the query text showing
implicit casts to be included in the PLAN section of a .test file.

The analyzed query text is wrapped at 80 characters. Note that the
analyzed query cannot always be executed as queries rewritten to use
LEFT SEMI JOIN are not legal sql. In addition some space characters may
be removed from the query for prettier display.

Documentation of this change will be done as IMPALA-7718

EXAMPLE OUTPUT:

[localhost:21000] default> set explain_level=2;
EXPLAIN_LEVEL set to 2
[localhost:21000] default> explain select * from functional_kudu.alltypestiny where bigint_col < 1000 / 100;
Query: explain select * from functional_kudu.alltypestiny where bigint_col < 1000 / 100
Max Per-Host Resource Reservation: Memory=0B Threads=2
Per-Host Resource Estimates: Memory=10MB
Codegen disabled by planner
Analyzed query: SELECT * FROM functional_kudu.alltypestiny WHERE CAST(bigint_col
AS DOUBLE) < CAST(10 AS DOUBLE)
""
F00:PLAN FRAGMENT [UNPARTITIONED] hosts=1 instances=1
|  Per-Host Resources: mem-estimate=4.88MB mem-reservation=0B thread-reservation=2
PLAN-ROOT SINK
|  mem-estimate=0B mem-reservation=0B thread-reservation=0
|
00:SCAN KUDU [functional_kudu.alltypestiny]
   predicates: CAST(bigint_col AS DOUBLE) < CAST(10 AS DOUBLE)
   mem-estimate=4.88MB mem-reservation=0B thread-reservation=1
   tuple-ids=0 row-size=97B cardinality=1
   in pipelines: 00(GETNEXT)
Fetched 16 row(s) in 0.03s

TESTING:

All end-to-end tests pass.
Added a new test in ExprRewriterTest which prints sql with implict casts
for some interesting queries.
Add a unit test for the code which wraps text at 80 characters.
The output of some Planner Tests in .test files has been updated to
include the Analyzed sql that is printed when explain_level is
at at least 'extended' level.

Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
---
M fe/src/main/java/org/apache/impala/analysis/AdminFnStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableAddDropRangePartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java
M fe/src/main/java/org/apache/impala/analysis/AnalyticWindow.java
M fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java
M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
M fe/src/main/java/org/apache/impala/analysis/BetweenPredicate.java
M fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java
M fe/src/main/java/org/apache/impala/analysis/BoolLiteral.java
M fe/src/main/java/org/apache/impala/analysis/CaseExpr.java
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
M fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java
M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateDataSrcStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateDropRoleStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
M fe/src/main/java/org/apache/impala/analysis/DescribeDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDataSrcStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropFunctionStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/ExistsPredicate.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/ExtractFromExpr.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/FunctionArgs.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/analysis/GrantRevokePrivStmt.java
M fe/src/main/java/org/apache/impala/analysis/GrantRevokeRoleStmt.java
M fe/src/main/java/org/apache/impala/analysis/HdfsCachingOp.java
M fe/src/main/java/org/apache/impala/analysis/InPredicate.java
M fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/IsNotEmptyPredicate.java
M fe/src/main/java/org/apache/impala/analysis/IsNullPredicate.java
M fe/src/main/java/org/apache/impala/analysis/KuduPartitionExpr.java
M fe/src/main/java/org/apache/impala/analysis/KuduPartitionParam.java
M fe/src/main/java/org/apache/impala/analysis/LikePredicate.java
M fe/src/main/java/org/apache/impala/analysis/LimitElement.java
M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/NullLiteral.java
M fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java
M fe/src/main/java/org/apache/impala/analysis/OrderByElement.java
M fe/src/main/java/org/apache/impala/analysis/ParseNode.java
M fe/src/main/java/org/apache/impala/analysis/PartitionDef.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSet.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java
M fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
M fe/src/main/java/org/apache/impala/analysis/RangePartition.java
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectListItem.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/SetStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowCreateFunctionStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowCreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowDataSrcsStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowDbsStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowFilesStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowFunctionsStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowGrantPrincipalStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowRolesStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowTablesStmt.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/analysis/StatementBase.java
M fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java
M fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
M fe/src/main/java/org/apache/impala/analysis/Subquery.java
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
M fe/src/main/java/org/apache/impala/analysis/TableSampleClause.java
M fe/src/main/java/org/apache/impala/analysis/TimestampArithmeticExpr.java
M fe/src/main/java/org/apache/impala/analysis/TimestampLiteral.java
A fe/src/main/java/org/apache/impala/analysis/ToSqlOptions.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/analysis/TruncateStmt.java
M fe/src/main/java/org/apache/impala/analysis/TupleIsNullPredicate.java
M fe/src/main/java/org/apache/impala/analysis/TypeDef.java
M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
M fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java
M fe/src/main/java/org/apache/impala/analysis/UseStmt.java
M fe/src/main/java/org/apache/impala/analysis/ValidTupleIdExpr.java
M fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java
M fe/src/main/java/org/apache/impala/analysis/WithClause.java
M fe/src/main/java/org/apache/impala/common/PrintUtils.java
M fe/src/main/java/org/apache/impala/planner/AggregationNode.java
M fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java
M fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java
M fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java
M fe/src/main/java/org/apache/impala/planner/HashJoinNode.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/JoinBuildSink.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/planner/NestedLoopJoinNode.java
M fe/src/main/java/org/apache/impala/planner/PlanNode.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/planner/SelectNode.java
M fe/src/main/java/org/apache/impala/planner/SubplanNode.java
M fe/src/main/java/org/apache/impala/planner/UnionNode.java
M fe/src/main/java/org/apache/impala/planner/UnnestNode.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
M fe/src/test/java/org/apache/impala/util/PrintUtilsTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/fk-pk-join-detection.test
M testdata/workloads/functional-planner/queries/PlannerTest/kudu-selectivity.test
M testdata/workloads/functional-planner/queries/PlannerTest/max-row-size.test
M testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering-disabled.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test
M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
M testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test
M testdata/workloads/functional-planner/queries/PlannerTest/tablesample.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level2.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level3.test
M testdata/workloads/functional-query/queries/QueryTest/parquet-stats.test
M testdata/workloads/functional-query/queries/QueryTest/stats-extrapolation.test
133 files changed, 1,350 insertions(+), 464 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/19/11719/4
-- 
To view, visit http://gerrit.cloudera.org:8080/11719
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
Gerrit-Change-Number: 11719
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>

[Impala-ASF-CR] IMPALA-5821: Add query with implicit casts to extended explain output.

Posted by "Andrew Sherman (Code Review)" <ge...@cloudera.org>.
Andrew Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/11719 )

Change subject: IMPALA-5821: Add query with implicit casts to extended explain output.
......................................................................


Patch Set 4:

(1 comment)

I agree that maintenance is a concern. 
1) for ParseNode I could remove toSql() from the interface and force all callers to use toSql(DEFAULT). This is easy in terms of effort as IntelliJ will do it for me, but results in a  lot of code changes, especially as many exception messages call toSql(). I did consider doing this originally but wanted to make this change as clear as possible. I can do this quickly in a follow-up jira. There are few more associated classes in analysis which would also be changed.
2) Do you want me to look at changes to toSql() that are not part of the ParseNode interface (or associated classes)? 
There is org.apache.impala.catalog.Type which has toSql() {return toSql(1)} where the argument controls how deeply we show nested Types.
There is org.apache.impala.analysis.TableName.toSql() and org.apache.impala.analysis.PlanHint.toSql()
Right now I would propose leaving these as is, but they could also be changed if we wanted to never have a naked toSql() call.

http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/main/java/org/apache/impala/common/PrintUtils.java
File fe/src/main/java/org/apache/impala/common/PrintUtils.java:

http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/main/java/org/apache/impala/common/PrintUtils.java@110
PS4, Line 110: 32
> ok, just mention it in a brief comment. I prefer such inline constants to h
I removed it.



-- 
To view, visit http://gerrit.cloudera.org:8080/11719
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
Gerrit-Change-Number: 11719
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 01 Nov 2018 20:32:36 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5821: Add query with implicit casts to extended explain output.

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/11719 )

Change subject: IMPALA-5821: Add query with implicit casts to extended explain output.
......................................................................


Patch Set 2:

> (4 comments)
 > 
 > Thanks for the review. What do you think about:
 > Is "Query with implicit casts:" a good description?
 > Is the EXPLAIN header the right place for this output?

It seems good to me, but I would ping Greg Rahn in the Jira, maybe he has some ideas.

I have one issue with the current output: as I saw in the .test files, queries are printed in one line by default, which can make complex queries very difficult to read. It would be much nicer to break them at logical points, but I have no idea how to do it easily.


-- 
To view, visit http://gerrit.cloudera.org:8080/11719
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
Gerrit-Change-Number: 11719
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Oct 2018 22:19:38 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5821: Add query with implicit casts to extended explain output.

Posted by "Andrew Sherman (Code Review)" <ge...@cloudera.org>.
Andrew Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/11719 )

Change subject: IMPALA-5821: Add query with implicit casts to extended explain output.
......................................................................


Patch Set 4:

(21 comments)

Thanks for review comments

http://gerrit.cloudera.org:8080/#/c/11719/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11719/4//COMMIT_MSG@51
PS4, Line 51: ""
> This quote seems strange here - is it included in the output?
This is what you get when you run 'impala-shell.sh -B'


http://gerrit.cloudera.org:8080/#/c/11719/4//COMMIT_MSG@69
PS4, Line 69: 80
> Isn't it 60?
yes


http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/main/java/org/apache/impala/analysis/ParseNode.java
File fe/src/main/java/org/apache/impala/analysis/ParseNode.java:

http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/main/java/org/apache/impala/analysis/ParseNode.java@32
PS4, Line 32: sql
> nit: pls use consistent case (SQL, sql, Sql)
Thanks


http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/main/java/org/apache/impala/analysis/ParseNode.java@32
PS4, Line 32: @param
> pls omit these javadoc annotations.
Done


http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/main/java/org/apache/impala/analysis/ParseNode.java@39
PS4, Line 39:    * This should return the same result as calling toSql(ToSqlOptions.DEFAULT).
> would be good to do this via a default interface method. pls add a todo for
Clever idea


http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/main/java/org/apache/impala/analysis/ToSqlOptions.java
File fe/src/main/java/org/apache/impala/analysis/ToSqlOptions.java:

http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/main/java/org/apache/impala/analysis/ToSqlOptions.java@25
PS4, Line 25: default, normal
> "normal" makes sense if you know what is currently done. perhaps "original 
Done


http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/main/java/org/apache/impala/analysis/ToSqlOptions.java@38
PS4, Line 38:   // This does have the consequence that the sql with implict casts may not pssibly fail
> nit: spelling
Done


http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/main/java/org/apache/impala/analysis/ToSqlOptions.java@38
PS4, Line 38: may not pssibly fail
            :   // to parse if resubmitted as
> I found this difficult to understand. Did you mean that Sql produced in thi
Yes, rewritten SQL is already not always valid SQL.
EXISTS queries that are rewritten as semi-joins are not legal SQL.
I clarified the comment slightly


http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/main/java/org/apache/impala/analysis/TypeDef.java
File fe/src/main/java/org/apache/impala/analysis/TypeDef.java:

http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/main/java/org/apache/impala/analysis/TypeDef.java@165
PS4, Line 165:     return parsedType_.toSql();
> pass down here?
It looks like it should, but the parsedType is not a ParseNode so I did not propagate ToSqlOptions to there.


http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/main/java/org/apache/impala/common/PrintUtils.java
File fe/src/main/java/org/apache/impala/common/PrintUtils.java:

http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/main/java/org/apache/impala/common/PrintUtils.java@110
PS4, Line 110: 32
> what's this?
Just some padding for if lines expand. Maybe this is too ugly.


http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/main/java/org/apache/impala/common/PrintUtils.java@115
PS4, Line 115: exiting
> sp. existing?
Done


http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
File fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java:

http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java@424
PS4, Line 424: @pa
> remove
I don't really understand what is wrong with param? 
But done.


http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java@451
PS4, Line 451:     // AnalyzesOk(stmt.toSql(true), ctx);
> rm?
Done


http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
File fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java:

http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java@750
PS4, Line 750: @para
> remove the comment annotations.
Done


http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java@772
PS4, Line 772:           if (line.contains("Analyzed query:")) {
             :             builder.append(line).append("\n");
             :             inImplictCasts = true;
             :           } else if (inImplictCasts) {
             :             // Keep copying the query with implicit casts.
             :             // This works because this is the last thing in the header.
             :             builder.append(line).append("\n");
             :           }
> optional: this could be expressed in a more compact way:
thanks


http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java@801
PS4, Line 801: This would also be included if both
> more direct: Equivalent to enabling INCLUDE_EXPLAIN_HEADER and EXTENDED_EXP
Thanks


http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/test/java/org/apache/impala/util/PrintUtilsTest.java
File fe/src/test/java/org/apache/impala/util/PrintUtilsTest.java:

http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/test/java/org/apache/impala/util/PrintUtilsTest.java@83
PS4, Line 83: wrap length for testWrapText() - less than 80 to make test layout nicer
> nit: Capital W + . at the end.
Done


http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/test/java/org/apache/impala/util/PrintUtilsTest.java@89
PS4, Line 89: *
> nit: extra *
Done


http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/test/java/org/apache/impala/util/PrintUtilsTest.java@127
PS4, Line 127:    * Check that code that has been wrapped is correctly formatted
> nit: missing .
Done


http://gerrit.cloudera.org:8080/#/c/11719/4/fe/src/test/java/org/apache/impala/util/PrintUtilsTest.java@128
PS4, Line 128: @pa
> rm
Done


http://gerrit.cloudera.org:8080/#/c/11719/4/testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test
File testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test:

http://gerrit.cloudera.org:8080/#/c/11719/4/testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test@9
PS4, Line 9: -- +straight_join
           : * 
> were you expecting the hint to print this way?
Yes. Though I welcome your opinion.
The newlines were already inserted by the toSql() code and I thought it clearer to maintain them.



-- 
To view, visit http://gerrit.cloudera.org:8080/11719
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
Gerrit-Change-Number: 11719
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 01 Nov 2018 00:37:50 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5821: Add query with implicit casts to extended explain output.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11719 )

Change subject: IMPALA-5821: Add query with implicit casts to extended explain output.
......................................................................


Patch Set 5:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/1243/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/11719
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
Gerrit-Change-Number: 11719
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 01 Nov 2018 01:18:10 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5821: Add query with implicit casts to extended explain output.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11719 )

Change subject: IMPALA-5821: Add query with implicit casts to extended explain output.
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/1215/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/11719
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
Gerrit-Change-Number: 11719
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Tue, 30 Oct 2018 21:09:16 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5821: Add query with implicit casts to extended explain output.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11719 )

Change subject: IMPALA-5821: Add query with implicit casts to extended explain output.
......................................................................


Patch Set 1:

Build Failed 

https://jenkins.impala.io/job/gerrit-code-review-checks/1082/ : Initial code review checks failed. See linked job for details on the failure.


-- 
To view, visit http://gerrit.cloudera.org:8080/11719
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
Gerrit-Change-Number: 11719
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Oct 2018 01:11:26 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5821: Add query with implicit casts to extended explain output.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11719 )

Change subject: IMPALA-5821: Add query with implicit casts to extended explain output.
......................................................................


Patch Set 9:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3464/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/11719
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
Gerrit-Change-Number: 11719
Gerrit-PatchSet: 9
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 15 Nov 2018 17:35:19 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5821: Add query with implicit casts to extended explain output.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11719 )

Change subject: IMPALA-5821: Add query with implicit casts to extended explain output.
......................................................................


Patch Set 7: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/11719
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
Gerrit-Change-Number: 11719
Gerrit-PatchSet: 7
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Nov 2018 03:28:27 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5821: Add query with implicit casts to extended explain output.

Posted by "Thomas Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/11719 )

Change subject: IMPALA-5821: Add query with implicit casts to extended explain output.
......................................................................


Patch Set 6: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/11719
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
Gerrit-Change-Number: 11719
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Nov 2018 23:31:16 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5821: Add query with implicit casts to extended explain output.

Posted by "Andrew Sherman (Code Review)" <ge...@cloudera.org>.
Andrew Sherman has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/11719 )

Change subject: IMPALA-5821: Add query with implicit casts to extended explain output.
......................................................................

IMPALA-5821: Add query with implicit casts to extended explain output.

If explain_level is at 'extended' level or higher, then enhance the
output from the explain command. (1) Show the analyzed sql in the
explain header, this is the rewritten sql, which includes implicit
casts, and literals are printed with a cast so that their type is
visible. (2) When predicates are shown in the plan these are shown in
the same format.

The toSql() method can be called on a ParseNode tree to return
the sql corresponding ot the tree. In the past toSQl() has been
enhanced to print rewritten sql by partially overloading toSql() [with
toSql(boolean)]. This current change requires changing toSQl() in
many places as NumericLiteral can appear at different points in ia
parse tree. To avoid many new fragile overloads of toSql() I added
toSql(ToSqlOptions), where ToSqlOptions is an enum which controls the
form of the Sql that is returned. This changes many files but is safer
and means that any future options to toSql() can be added painlessly.

If SHOW_IMPLICIT_CASTS is passed to toSql() then
- in CastExpr print the implicit cast
- in NumericLiteral print the literal with a cast to show the type

Add a PlannerTestOption directive that will force the query text showing
implicit casts to be included in the PLAN section of a .test file.

The analyzed query text is wrapped at 80 characters. Note that the
analyzed query cannot always be executed as queries rewritten to use
LEFT SEMI JOIN are not legal sql. In addition some space characters may
be removed from the query for prettier display.

Documentation of this change will be done as IMPALA-7718

EXAMPLE OUTPUT:

[localhost:21000] default> set explain_level=2;
EXPLAIN_LEVEL set to 2
[localhost:21000] default> explain select * from functional_kudu.alltypestiny where bigint_col < 1000 / 100;
Query: explain select * from functional_kudu.alltypestiny where bigint_col < 1000 / 100
Max Per-Host Resource Reservation: Memory=0B Threads=2
Per-Host Resource Estimates: Memory=10MB
Codegen disabled by planner
Analyzed query: SELECT * FROM functional_kudu.alltypestiny WHERE CAST(bigint_col
AS DOUBLE) < CAST(10 AS DOUBLE)
""
F00:PLAN FRAGMENT [UNPARTITIONED] hosts=1 instances=1
|  Per-Host Resources: mem-estimate=4.88MB mem-reservation=0B thread-reservation=2
PLAN-ROOT SINK
|  mem-estimate=0B mem-reservation=0B thread-reservation=0
|
00:SCAN KUDU [functional_kudu.alltypestiny]
   predicates: CAST(bigint_col AS DOUBLE) < CAST(10 AS DOUBLE)
   mem-estimate=4.88MB mem-reservation=0B thread-reservation=1
   tuple-ids=0 row-size=97B cardinality=1
   in pipelines: 00(GETNEXT)
Fetched 16 row(s) in 0.03s

TESTING:

All end-to-end tests pass.
Added a new test in ExprRewriterTest which prints sql with implict casts
for some interesting queries.
Add a unit test for the code which wraps text at 60 characters.
The output of some Planner Tests in .test files has been updated to
include the Analyzed sql that is printed when explain_level is
at at least 'extended' level.

Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
---
M fe/src/main/java/org/apache/impala/analysis/AdminFnStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableAddDropRangePartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java
M fe/src/main/java/org/apache/impala/analysis/AnalyticWindow.java
M fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java
M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
M fe/src/main/java/org/apache/impala/analysis/BetweenPredicate.java
M fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java
M fe/src/main/java/org/apache/impala/analysis/BoolLiteral.java
M fe/src/main/java/org/apache/impala/analysis/CaseExpr.java
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
M fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java
M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateDataSrcStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateDropRoleStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
M fe/src/main/java/org/apache/impala/analysis/DescribeDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDataSrcStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropFunctionStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/ExistsPredicate.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/ExtractFromExpr.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/FunctionArgs.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/analysis/GrantRevokePrivStmt.java
M fe/src/main/java/org/apache/impala/analysis/GrantRevokeRoleStmt.java
M fe/src/main/java/org/apache/impala/analysis/HdfsCachingOp.java
M fe/src/main/java/org/apache/impala/analysis/InPredicate.java
M fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/IsNotEmptyPredicate.java
M fe/src/main/java/org/apache/impala/analysis/IsNullPredicate.java
M fe/src/main/java/org/apache/impala/analysis/KuduPartitionExpr.java
M fe/src/main/java/org/apache/impala/analysis/KuduPartitionParam.java
M fe/src/main/java/org/apache/impala/analysis/LikePredicate.java
M fe/src/main/java/org/apache/impala/analysis/LimitElement.java
M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/NullLiteral.java
M fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java
M fe/src/main/java/org/apache/impala/analysis/OrderByElement.java
M fe/src/main/java/org/apache/impala/analysis/ParseNode.java
M fe/src/main/java/org/apache/impala/analysis/PartitionDef.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSet.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java
M fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
M fe/src/main/java/org/apache/impala/analysis/RangePartition.java
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectListItem.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/SetStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowCreateFunctionStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowCreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowDataSrcsStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowDbsStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowFilesStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowFunctionsStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowGrantPrincipalStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowRolesStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowTablesStmt.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/analysis/StatementBase.java
M fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java
M fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
M fe/src/main/java/org/apache/impala/analysis/Subquery.java
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
M fe/src/main/java/org/apache/impala/analysis/TableSampleClause.java
M fe/src/main/java/org/apache/impala/analysis/TimestampArithmeticExpr.java
M fe/src/main/java/org/apache/impala/analysis/TimestampLiteral.java
A fe/src/main/java/org/apache/impala/analysis/ToSqlOptions.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/analysis/TruncateStmt.java
M fe/src/main/java/org/apache/impala/analysis/TupleIsNullPredicate.java
M fe/src/main/java/org/apache/impala/analysis/TypeDef.java
M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
M fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java
M fe/src/main/java/org/apache/impala/analysis/UseStmt.java
M fe/src/main/java/org/apache/impala/analysis/ValidTupleIdExpr.java
M fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java
M fe/src/main/java/org/apache/impala/analysis/WithClause.java
M fe/src/main/java/org/apache/impala/common/PrintUtils.java
M fe/src/main/java/org/apache/impala/planner/AggregationNode.java
M fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java
M fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java
M fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java
M fe/src/main/java/org/apache/impala/planner/HashJoinNode.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/JoinBuildSink.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/planner/NestedLoopJoinNode.java
M fe/src/main/java/org/apache/impala/planner/PlanNode.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/planner/SelectNode.java
M fe/src/main/java/org/apache/impala/planner/SubplanNode.java
M fe/src/main/java/org/apache/impala/planner/UnionNode.java
M fe/src/main/java/org/apache/impala/planner/UnnestNode.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
M fe/src/test/java/org/apache/impala/util/PrintUtilsTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/fk-pk-join-detection.test
M testdata/workloads/functional-planner/queries/PlannerTest/kudu-selectivity.test
M testdata/workloads/functional-planner/queries/PlannerTest/max-row-size.test
M testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering-disabled.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test
M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
M testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test
M testdata/workloads/functional-planner/queries/PlannerTest/tablesample.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level2.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level3.test
M testdata/workloads/functional-query/queries/QueryTest/parquet-stats.test
M testdata/workloads/functional-query/queries/QueryTest/stats-extrapolation.test
133 files changed, 1,344 insertions(+), 464 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/19/11719/5
-- 
To view, visit http://gerrit.cloudera.org:8080/11719
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
Gerrit-Change-Number: 11719
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>

[Impala-ASF-CR] IMPALA-5821: Add query with implicit casts to extended explain output.

Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/11719 )

Change subject: IMPALA-5821: Add query with implicit casts to extended explain output.
......................................................................


Patch Set 6: Code-Review+2

lgtm from my end. will let others on the thread have another look. 

for maintenance, I'd go with a follow-up to remove ParseNode's toSql(). then for the cases that were default, I'd look if they're really "toSql" or some other type of to-string method. for the non-parseNode types, "toSql" is fine... the differing type guards against the maintenance issue.


-- 
To view, visit http://gerrit.cloudera.org:8080/11719
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
Gerrit-Change-Number: 11719
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Thu, 01 Nov 2018 22:27:53 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5821: Add query with implicit casts to extended explain output.

Posted by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org>.
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/11719 )

Change subject: IMPALA-5821: Add query with implicit casts to extended explain output.
......................................................................


Patch Set 2:

> > It seems good to me, but I would ping Greg Rahn in the Jira,
 > maybe
 > > he has some ideas.
 > 
 > I have pinged him
 > 
 > > I have one issue with the current output: as I saw in the .test
 > > files, queries are printed in one line by default, which can make
 > > complex queries very difficult to read. It would be much nicer to
 > > break them at logical points, but I have no idea how to do it
 > > easily.
 > 
 > I agree that a clever printing would be nice, but that sounds
 > tricky. I could wrap at say the last space before 80 columns with
 > only small effort, but this will make the header harder to parse in
 > tests. Should I try this?

About good places to start new line: the enum could have a property whether to add new lines, and some ParseNodes could add new lines if it is true, e.g before joins and unions.

Another idea (of the "if you already touch it, why not rewrite the whole thing" type, so it is optional): if there is an object passed to toSql(), maybe it could be some kind of ToSqlBuilder instead of ToSqlOption. It could contain the implicit cast related information and could be also used as a string builder. The current implementation uses string builders in most nodes, but I think that toSql() can still be O(n^2) in some cases.


-- 
To view, visit http://gerrit.cloudera.org:8080/11719
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
Gerrit-Change-Number: 11719
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 19 Oct 2018 20:22:41 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5821: Add query with implicit casts to extended explain output.

Posted by "Andrew Sherman (Code Review)" <ge...@cloudera.org>.
Andrew Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/11719 )

Change subject: IMPALA-5821: Add query with implicit casts to extended explain output.
......................................................................


Patch Set 1:

(4 comments)

Thanks for the review. What do you think about:
Is "Query with implicit casts:" a good description?
Is the EXPLAIN header the right place for this output?

http://gerrit.cloudera.org:8080/#/c/11719/1/fe/src/main/java/org/apache/impala/analysis/ParseNode.java
File fe/src/main/java/org/apache/impala/analysis/ParseNode.java:

http://gerrit.cloudera.org:8080/#/c/11719/1/fe/src/main/java/org/apache/impala/analysis/ParseNode.java@30
PS1, Line 30:   /**
            :    * Returns the SQL string corresponding to this node.
            :    */
> Can you explain the relation of the two toSql() functions in the comment? I
Thanks fro thinking about this.  This is a case where default parameters would help.


http://gerrit.cloudera.org:8080/#/c/11719/1/fe/src/main/java/org/apache/impala/analysis/ToSqlOptions.java
File fe/src/main/java/org/apache/impala/analysis/ToSqlOptions.java:

http://gerrit.cloudera.org:8080/#/c/11719/1/fe/src/main/java/org/apache/impala/analysis/ToSqlOptions.java@1
PS1, Line 1: package org.apache.impala.analysis;
> Apache license is missing (also noticed by the jenkins job).
Done


http://gerrit.cloudera.org:8080/#/c/11719/1/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
File fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java:

http://gerrit.cloudera.org:8080/#/c/11719/1/fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java@344
PS1, Line 344: TestToSqlWithImplicitCasts
> Can you add a test (or maybe a few) to check that nested structures like su
good idea


http://gerrit.cloudera.org:8080/#/c/11719/1/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
File fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java:

http://gerrit.cloudera.org:8080/#/c/11719/1/fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java@546
PS1, Line 546:         errorLog.append("\nTest file: " + testFile + ".test"
             :             + "\n");
> nit: fits to one line
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/11719
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
Gerrit-Change-Number: 11719
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Oct 2018 22:08:31 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5821: Add query with implicit casts to extended explain output.

Posted by "Andrew Sherman (Code Review)" <ge...@cloudera.org>.
Andrew Sherman has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/11719 )

Change subject: IMPALA-5821: Add query with implicit casts to extended explain output.
......................................................................

IMPALA-5821: Add query with implicit casts to extended explain output.

If explain_level is at 'extended' level or higher, then enhance the
output from the explain command. (1) Show the analyzed sql in the
explain header, this is the rewritten sql, which includes implicit
casts, and literals are printed with a cast so that their type is
visible. (2) When predicates are shown in the plan these are shown in
the same format.

The toSql() method can be called on a ParseNode tree to return
the sql corresponding ot the tree. In the past toSQl() has been
enhanced to print rewritten sql by partially overloading toSql() [with
toSql(boolean)]. This current change requires changing toSQl() in
many places as NumericLiteral can appear at different points in ia
parse tree. To avoid many new fragile overloads of toSql() I added
toSql(ToSqlOptions), where ToSqlOptions is an enum which controls the
form of the Sql that is returned. This changes many files but is safer
and means that any future options to toSql() can be added painlessly.

If SHOW_IMPLICIT_CASTS is passed to toSql() then
- in CastExpr print the implicit cast
- in NumericLiteral print the literal with a cast to show the type

Add a PlannerTestOption directive that will force the query text showing
implicit casts to be included in the PLAN section of a .test file.

The analyzed query text is wrapped at 80 characters. Note that the
analyzed query cannot always be executed as queries rewritten to use
LEFT SEMI JOIN are not legal sql. In addition some space characters may
be removed from the query for prettier display.

Documentation of this change will be done as IMPALA-7718

EXAMPLE OUTPUT:

[localhost:21000] default> set explain_level=2;
EXPLAIN_LEVEL set to 2
[localhost:21000] default> explain select * from functional_kudu.alltypestiny where bigint_col < 1000 / 100;
Query: explain select * from functional_kudu.alltypestiny where bigint_col < 1000 / 100
Max Per-Host Resource Reservation: Memory=0B Threads=2
Per-Host Resource Estimates: Memory=10MB
Codegen disabled by planner
Analyzed query: SELECT * FROM functional_kudu.alltypestiny WHERE CAST(bigint_col
AS DOUBLE) < CAST(10 AS DOUBLE)
""
F00:PLAN FRAGMENT [UNPARTITIONED] hosts=1 instances=1
|  Per-Host Resources: mem-estimate=4.88MB mem-reservation=0B thread-reservation=2
PLAN-ROOT SINK
|  mem-estimate=0B mem-reservation=0B thread-reservation=0
|
00:SCAN KUDU [functional_kudu.alltypestiny]
   predicates: CAST(bigint_col AS DOUBLE) < CAST(10 AS DOUBLE)
   mem-estimate=4.88MB mem-reservation=0B thread-reservation=1
   tuple-ids=0 row-size=97B cardinality=1
   in pipelines: 00(GETNEXT)
Fetched 16 row(s) in 0.03s

TESTING:

All end-to-end tests pass.
Added a new test in ExprRewriterTest which prints sql with implict casts
for some interesting queries.
Add a unit test for the code which wraps text at 80 characters.
The output of some Planner Tests in .test files has been updated to
include the Analyzed sql that is printed when explain_level is
at at least 'extended' level.

Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
---
M fe/src/main/java/org/apache/impala/analysis/AdminFnStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableAddDropRangePartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java
M fe/src/main/java/org/apache/impala/analysis/AnalyticWindow.java
M fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java
M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
M fe/src/main/java/org/apache/impala/analysis/BetweenPredicate.java
M fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java
M fe/src/main/java/org/apache/impala/analysis/BoolLiteral.java
M fe/src/main/java/org/apache/impala/analysis/CaseExpr.java
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
M fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java
M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateDataSrcStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateDropRoleStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
M fe/src/main/java/org/apache/impala/analysis/DescribeDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDataSrcStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropFunctionStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/ExistsPredicate.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/ExtractFromExpr.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/FunctionArgs.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/analysis/GrantRevokePrivStmt.java
M fe/src/main/java/org/apache/impala/analysis/GrantRevokeRoleStmt.java
M fe/src/main/java/org/apache/impala/analysis/HdfsCachingOp.java
M fe/src/main/java/org/apache/impala/analysis/InPredicate.java
M fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/IsNotEmptyPredicate.java
M fe/src/main/java/org/apache/impala/analysis/IsNullPredicate.java
M fe/src/main/java/org/apache/impala/analysis/KuduPartitionExpr.java
M fe/src/main/java/org/apache/impala/analysis/KuduPartitionParam.java
M fe/src/main/java/org/apache/impala/analysis/LikePredicate.java
M fe/src/main/java/org/apache/impala/analysis/LimitElement.java
M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/NullLiteral.java
M fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java
M fe/src/main/java/org/apache/impala/analysis/OrderByElement.java
M fe/src/main/java/org/apache/impala/analysis/ParseNode.java
M fe/src/main/java/org/apache/impala/analysis/PartitionDef.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSet.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java
M fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
M fe/src/main/java/org/apache/impala/analysis/RangePartition.java
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectListItem.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/SetStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowCreateFunctionStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowCreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowDataSrcsStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowDbsStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowFilesStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowFunctionsStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowGrantPrincipalStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowRolesStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowTablesStmt.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/analysis/StatementBase.java
M fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java
M fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
M fe/src/main/java/org/apache/impala/analysis/Subquery.java
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
M fe/src/main/java/org/apache/impala/analysis/TableSampleClause.java
M fe/src/main/java/org/apache/impala/analysis/TimestampArithmeticExpr.java
M fe/src/main/java/org/apache/impala/analysis/TimestampLiteral.java
A fe/src/main/java/org/apache/impala/analysis/ToSqlOptions.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/analysis/TruncateStmt.java
M fe/src/main/java/org/apache/impala/analysis/TupleIsNullPredicate.java
M fe/src/main/java/org/apache/impala/analysis/TypeDef.java
M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
M fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java
M fe/src/main/java/org/apache/impala/analysis/UseStmt.java
M fe/src/main/java/org/apache/impala/analysis/ValidTupleIdExpr.java
M fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java
M fe/src/main/java/org/apache/impala/analysis/WithClause.java
M fe/src/main/java/org/apache/impala/common/PrintUtils.java
M fe/src/main/java/org/apache/impala/planner/AggregationNode.java
M fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java
M fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java
M fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java
M fe/src/main/java/org/apache/impala/planner/HashJoinNode.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/JoinBuildSink.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/planner/NestedLoopJoinNode.java
M fe/src/main/java/org/apache/impala/planner/PlanNode.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/planner/SelectNode.java
M fe/src/main/java/org/apache/impala/planner/SubplanNode.java
M fe/src/main/java/org/apache/impala/planner/UnionNode.java
M fe/src/main/java/org/apache/impala/planner/UnnestNode.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
M fe/src/test/java/org/apache/impala/util/PrintUtilsTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/fk-pk-join-detection.test
M testdata/workloads/functional-planner/queries/PlannerTest/kudu-selectivity.test
M testdata/workloads/functional-planner/queries/PlannerTest/max-row-size.test
M testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering-disabled.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test
M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
M testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test
M testdata/workloads/functional-planner/queries/PlannerTest/tablesample.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level2.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level3.test
M testdata/workloads/functional-query/queries/QueryTest/parquet-stats.test
M testdata/workloads/functional-query/queries/QueryTest/stats-extrapolation.test
133 files changed, 1,346 insertions(+), 464 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/19/11719/3
-- 
To view, visit http://gerrit.cloudera.org:8080/11719
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
Gerrit-Change-Number: 11719
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>

[Impala-ASF-CR] IMPALA-5821: Add query with implicit casts to extended explain output.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11719 )

Change subject: IMPALA-5821: Add query with implicit casts to extended explain output.
......................................................................


Patch Set 7:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3435/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/11719
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
Gerrit-Change-Number: 11719
Gerrit-PatchSet: 7
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Nov 2018 23:31:31 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5821: Add query with implicit casts to extended explain output.

Posted by "Andrew Sherman (Code Review)" <ge...@cloudera.org>.
Andrew Sherman has uploaded a new patch set (#8). ( http://gerrit.cloudera.org:8080/11719 )

Change subject: IMPALA-5821: Add query with implicit casts to extended explain output.
......................................................................

IMPALA-5821: Add query with implicit casts to extended explain output.

If explain_level is at 'extended' level or higher, then enhance the
output from the explain command. (1) Show the analyzed sql in the
explain header, this is the rewritten sql, which includes implicit
casts, and literals are printed with a cast so that their type is
visible. (2) When predicates are shown in the plan these are shown in
the same format.

The toSql() method can be called on a ParseNode tree to return
the sql corresponding ot the tree. In the past toSQl() has been
enhanced to print rewritten sql by partially overloading toSql() [with
toSql(boolean)]. This current change requires changing toSQl() in
many places as NumericLiteral can appear at different points in ia
parse tree. To avoid many new fragile overloads of toSql() I added
toSql(ToSqlOptions), where ToSqlOptions is an enum which controls the
form of the Sql that is returned. This changes many files but is safer
and means that any future options to toSql() can be added painlessly.

If SHOW_IMPLICIT_CASTS is passed to toSql() then
- in CastExpr print the implicit cast
- in NumericLiteral print the literal with a cast to show the type

Add a PlannerTestOption directive that will force the query text showing
implicit casts to be included in the PLAN section of a .test file.

The analyzed query text is wrapped at 80 characters. Note that the
analyzed query cannot always be executed as queries rewritten to use
LEFT SEMI JOIN are not legal sql. In addition some space characters may
be removed from the query for prettier display.

Documentation of this change will be done as IMPALA-7718

EXAMPLE OUTPUT:

[localhost:21000] default> set explain_level=2;
EXPLAIN_LEVEL set to 2
[localhost:21000] default> explain select * from functional_kudu.alltypestiny where bigint_col < 1000 / 100;
Query: explain select * from functional_kudu.alltypestiny where bigint_col < 1000 / 100
Max Per-Host Resource Reservation: Memory=0B Threads=2
Per-Host Resource Estimates: Memory=10MB
Codegen disabled by planner
Analyzed query: SELECT * FROM functional_kudu.alltypestiny WHERE CAST(bigint_col
AS DOUBLE) < CAST(10 AS DOUBLE)
""
F00:PLAN FRAGMENT [UNPARTITIONED] hosts=1 instances=1
|  Per-Host Resources: mem-estimate=4.88MB mem-reservation=0B thread-reservation=2
PLAN-ROOT SINK
|  mem-estimate=0B mem-reservation=0B thread-reservation=0
|
00:SCAN KUDU [functional_kudu.alltypestiny]
   predicates: CAST(bigint_col AS DOUBLE) < CAST(10 AS DOUBLE)
   mem-estimate=4.88MB mem-reservation=0B thread-reservation=1
   tuple-ids=0 row-size=97B cardinality=1
   in pipelines: 00(GETNEXT)
Fetched 16 row(s) in 0.03s

TESTING:

All end-to-end tests pass.
Added a new test in ExprRewriterTest which prints sql with implict casts
for some interesting queries.
Add a unit test for the code which wraps text at 60 characters.
The output of some Planner Tests in .test files has been updated to
include the Analyzed sql that is printed when explain_level is
at at least 'extended' level.

Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
---
M fe/src/main/java/org/apache/impala/analysis/AdminFnStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableAddDropRangePartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java
M fe/src/main/java/org/apache/impala/analysis/AnalyticWindow.java
M fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java
M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
M fe/src/main/java/org/apache/impala/analysis/BetweenPredicate.java
M fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java
M fe/src/main/java/org/apache/impala/analysis/BoolLiteral.java
M fe/src/main/java/org/apache/impala/analysis/CaseExpr.java
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
M fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java
M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateDataSrcStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateDropRoleStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
M fe/src/main/java/org/apache/impala/analysis/DescribeDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDataSrcStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropFunctionStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/ExistsPredicate.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/ExtractFromExpr.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/FunctionArgs.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/analysis/GrantRevokePrivStmt.java
M fe/src/main/java/org/apache/impala/analysis/GrantRevokeRoleStmt.java
M fe/src/main/java/org/apache/impala/analysis/HdfsCachingOp.java
M fe/src/main/java/org/apache/impala/analysis/InPredicate.java
M fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/IsNotEmptyPredicate.java
M fe/src/main/java/org/apache/impala/analysis/IsNullPredicate.java
M fe/src/main/java/org/apache/impala/analysis/KuduPartitionExpr.java
M fe/src/main/java/org/apache/impala/analysis/KuduPartitionParam.java
M fe/src/main/java/org/apache/impala/analysis/LikePredicate.java
M fe/src/main/java/org/apache/impala/analysis/LimitElement.java
M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/NullLiteral.java
M fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java
M fe/src/main/java/org/apache/impala/analysis/OrderByElement.java
M fe/src/main/java/org/apache/impala/analysis/ParseNode.java
M fe/src/main/java/org/apache/impala/analysis/PartitionDef.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSet.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java
M fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
M fe/src/main/java/org/apache/impala/analysis/RangePartition.java
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectListItem.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/SetStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowCreateFunctionStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowCreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowDataSrcsStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowDbsStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowFilesStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowFunctionsStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowGrantPrincipalStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowRolesStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowTablesStmt.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/analysis/StatementBase.java
M fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java
M fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
M fe/src/main/java/org/apache/impala/analysis/Subquery.java
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
M fe/src/main/java/org/apache/impala/analysis/TableSampleClause.java
M fe/src/main/java/org/apache/impala/analysis/TimestampArithmeticExpr.java
M fe/src/main/java/org/apache/impala/analysis/TimestampLiteral.java
A fe/src/main/java/org/apache/impala/analysis/ToSqlOptions.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/analysis/TruncateStmt.java
M fe/src/main/java/org/apache/impala/analysis/TupleIsNullPredicate.java
M fe/src/main/java/org/apache/impala/analysis/TypeDef.java
M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
M fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java
M fe/src/main/java/org/apache/impala/analysis/UseStmt.java
M fe/src/main/java/org/apache/impala/analysis/ValidTupleIdExpr.java
M fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java
M fe/src/main/java/org/apache/impala/analysis/WithClause.java
M fe/src/main/java/org/apache/impala/common/PrintUtils.java
M fe/src/main/java/org/apache/impala/planner/AggregationNode.java
M fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java
M fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java
M fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java
M fe/src/main/java/org/apache/impala/planner/HashJoinNode.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/JoinBuildSink.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/planner/NestedLoopJoinNode.java
M fe/src/main/java/org/apache/impala/planner/PlanNode.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/planner/SelectNode.java
M fe/src/main/java/org/apache/impala/planner/SubplanNode.java
M fe/src/main/java/org/apache/impala/planner/UnionNode.java
M fe/src/main/java/org/apache/impala/planner/UnnestNode.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
M fe/src/test/java/org/apache/impala/util/PrintUtilsTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/fk-pk-join-detection.test
M testdata/workloads/functional-planner/queries/PlannerTest/kudu-selectivity.test
M testdata/workloads/functional-planner/queries/PlannerTest/max-row-size.test
M testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering-disabled.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test
M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
M testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test
M testdata/workloads/functional-planner/queries/PlannerTest/tablesample.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level2.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level3.test
M testdata/workloads/functional-query/queries/QueryTest/parquet-stats.test
M testdata/workloads/functional-query/queries/QueryTest/stats-extrapolation.test
133 files changed, 1,341 insertions(+), 460 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/19/11719/8
-- 
To view, visit http://gerrit.cloudera.org:8080/11719
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
Gerrit-Change-Number: 11719
Gerrit-PatchSet: 8
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>

[Impala-ASF-CR] IMPALA-5821: Add query with implicit casts to extended explain output.

Posted by "Andrew Sherman (Code Review)" <ge...@cloudera.org>.
Andrew Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/11719 )

Change subject: IMPALA-5821: Add query with implicit casts to extended explain output.
......................................................................


Patch Set 4:

I added a new patch because rebase was required


-- 
To view, visit http://gerrit.cloudera.org:8080/11719
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
Gerrit-Change-Number: 11719
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Comment-Date: Tue, 30 Oct 2018 20:34:54 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5821: Add query with implicit casts to extended explain output.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11719 )

Change subject: IMPALA-5821: Add query with implicit casts to extended explain output.
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/1092/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/11719
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
Gerrit-Change-Number: 11719
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 18 Oct 2018 17:28:15 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5821: Add query with implicit casts to extended explain output.

Posted by "Andrew Sherman (Code Review)" <ge...@cloudera.org>.
Andrew Sherman has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/11719 )

Change subject: IMPALA-5821: Add query with implicit casts to extended explain output.
......................................................................

IMPALA-5821: Add query with implicit casts to extended explain output.

If explain_level is at 'extended' level or higher, then enhance the
output from the explain command. (1) Show the analyzed sql in the
explain header, this is the rewritten sql, which includes implicit
casts, and literals are printed with a cast so that their type is
visible. (2) When predicates are shown in the plan these are shown in
the same format.

The toSql() method can be called on a ParseNode tree to return
the sql corresponding ot the tree. In the past toSQl() has been
enhanced to print rewritten sql by partially overloading toSql() [with
toSql(boolean)]. This current change requires changing toSQl() in
many places as NumericLiteral can appear at different points in ia
parse tree. To avoid many new fragile overloads of toSql() I added
toSql(ToSqlOptions), where ToSqlOptions is an enum which controls the
form of the Sql that is returned. This changes many files but is safer
and means that any future options to toSql() can be added painlessly.

If SHOW_IMPLICIT_CASTS is passed to toSql() then
- in CastExpr print the implicit cast
- in NumericLiteral print the literal with a cast to show the type

Add a PlannerTestOption directive that will force the query text showing
implicit casts to be included in the PLAN section of a .test file.

The analyzed query text is wrapped at 80 characters. Note that the
analyzed query cannot always be executed as queries rewritten to use
LEFT SEMI JOIN are not legal sql. In addition some space characters may
be removed from the query for prettier display.

Documentation of this change will be done as IMPALA-7718

EXAMPLE OUTPUT:

[localhost:21000] default> set explain_level=2;
EXPLAIN_LEVEL set to 2
[localhost:21000] default> explain select * from functional_kudu.alltypestiny where bigint_col < 1000 / 100;
Query: explain select * from functional_kudu.alltypestiny where bigint_col < 1000 / 100
Max Per-Host Resource Reservation: Memory=0B Threads=2
Per-Host Resource Estimates: Memory=10MB
Codegen disabled by planner
Analyzed query: SELECT * FROM functional_kudu.alltypestiny WHERE CAST(bigint_col
AS DOUBLE) < CAST(10 AS DOUBLE)
""
F00:PLAN FRAGMENT [UNPARTITIONED] hosts=1 instances=1
|  Per-Host Resources: mem-estimate=4.88MB mem-reservation=0B thread-reservation=2
PLAN-ROOT SINK
|  mem-estimate=0B mem-reservation=0B thread-reservation=0
|
00:SCAN KUDU [functional_kudu.alltypestiny]
   predicates: CAST(bigint_col AS DOUBLE) < CAST(10 AS DOUBLE)
   mem-estimate=4.88MB mem-reservation=0B thread-reservation=1
   tuple-ids=0 row-size=97B cardinality=1
   in pipelines: 00(GETNEXT)
Fetched 16 row(s) in 0.03s

TESTING:

All end-to-end tests pass.
Added a new test in ExprRewriterTest which prints sql with implict casts
for some interesting queries.
Add a unit test for the code which wraps text at 60 characters.
The output of some Planner Tests in .test files has been updated to
include the Analyzed sql that is printed when explain_level is
at at least 'extended' level.

Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
---
M fe/src/main/java/org/apache/impala/analysis/AdminFnStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableAddDropRangePartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableAddPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterTableDropPartitionStmt.java
M fe/src/main/java/org/apache/impala/analysis/AlterViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/analysis/AnalyticExpr.java
M fe/src/main/java/org/apache/impala/analysis/AnalyticWindow.java
M fe/src/main/java/org/apache/impala/analysis/ArithmeticExpr.java
M fe/src/main/java/org/apache/impala/analysis/BaseTableRef.java
M fe/src/main/java/org/apache/impala/analysis/BetweenPredicate.java
M fe/src/main/java/org/apache/impala/analysis/BinaryPredicate.java
M fe/src/main/java/org/apache/impala/analysis/BoolLiteral.java
M fe/src/main/java/org/apache/impala/analysis/CaseExpr.java
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
M fe/src/main/java/org/apache/impala/analysis/CompoundPredicate.java
M fe/src/main/java/org/apache/impala/analysis/ComputeStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateDataSrcStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateDropRoleStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java
M fe/src/main/java/org/apache/impala/analysis/DescribeDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDataSrcStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropFunctionStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/ExistsPredicate.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/ExtractFromExpr.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/FunctionArgs.java
M fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
M fe/src/main/java/org/apache/impala/analysis/GrantRevokePrivStmt.java
M fe/src/main/java/org/apache/impala/analysis/GrantRevokeRoleStmt.java
M fe/src/main/java/org/apache/impala/analysis/HdfsCachingOp.java
M fe/src/main/java/org/apache/impala/analysis/InPredicate.java
M fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/IsNotEmptyPredicate.java
M fe/src/main/java/org/apache/impala/analysis/IsNullPredicate.java
M fe/src/main/java/org/apache/impala/analysis/KuduPartitionExpr.java
M fe/src/main/java/org/apache/impala/analysis/KuduPartitionParam.java
M fe/src/main/java/org/apache/impala/analysis/LikePredicate.java
M fe/src/main/java/org/apache/impala/analysis/LimitElement.java
M fe/src/main/java/org/apache/impala/analysis/LoadDataStmt.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/NullLiteral.java
M fe/src/main/java/org/apache/impala/analysis/NumericLiteral.java
M fe/src/main/java/org/apache/impala/analysis/OrderByElement.java
M fe/src/main/java/org/apache/impala/analysis/ParseNode.java
M fe/src/main/java/org/apache/impala/analysis/PartitionDef.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSet.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpec.java
M fe/src/main/java/org/apache/impala/analysis/PartitionSpecBase.java
M fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
M fe/src/main/java/org/apache/impala/analysis/RangePartition.java
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectListItem.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/SetStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowCreateFunctionStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowCreateTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowDataSrcsStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowDbsStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowFilesStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowFunctionsStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowGrantPrincipalStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowRolesStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowStatsStmt.java
M fe/src/main/java/org/apache/impala/analysis/ShowTablesStmt.java
M fe/src/main/java/org/apache/impala/analysis/SlotRef.java
M fe/src/main/java/org/apache/impala/analysis/StatementBase.java
M fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java
M fe/src/main/java/org/apache/impala/analysis/StringLiteral.java
M fe/src/main/java/org/apache/impala/analysis/Subquery.java
M fe/src/main/java/org/apache/impala/analysis/TableRef.java
M fe/src/main/java/org/apache/impala/analysis/TableSampleClause.java
M fe/src/main/java/org/apache/impala/analysis/TimestampArithmeticExpr.java
M fe/src/main/java/org/apache/impala/analysis/TimestampLiteral.java
A fe/src/main/java/org/apache/impala/analysis/ToSqlOptions.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/main/java/org/apache/impala/analysis/TruncateStmt.java
M fe/src/main/java/org/apache/impala/analysis/TupleIsNullPredicate.java
M fe/src/main/java/org/apache/impala/analysis/TypeDef.java
M fe/src/main/java/org/apache/impala/analysis/UnionStmt.java
M fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java
M fe/src/main/java/org/apache/impala/analysis/UseStmt.java
M fe/src/main/java/org/apache/impala/analysis/ValidTupleIdExpr.java
M fe/src/main/java/org/apache/impala/analysis/ValuesStmt.java
M fe/src/main/java/org/apache/impala/analysis/WithClause.java
M fe/src/main/java/org/apache/impala/common/PrintUtils.java
M fe/src/main/java/org/apache/impala/planner/AggregationNode.java
M fe/src/main/java/org/apache/impala/planner/AnalyticEvalNode.java
M fe/src/main/java/org/apache/impala/planner/DataSourceScanNode.java
M fe/src/main/java/org/apache/impala/planner/HBaseScanNode.java
M fe/src/main/java/org/apache/impala/planner/HashJoinNode.java
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M fe/src/main/java/org/apache/impala/planner/JoinBuildSink.java
M fe/src/main/java/org/apache/impala/planner/KuduScanNode.java
M fe/src/main/java/org/apache/impala/planner/NestedLoopJoinNode.java
M fe/src/main/java/org/apache/impala/planner/PlanNode.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/planner/SelectNode.java
M fe/src/main/java/org/apache/impala/planner/SubplanNode.java
M fe/src/main/java/org/apache/impala/planner/UnionNode.java
M fe/src/main/java/org/apache/impala/planner/UnnestNode.java
M fe/src/test/java/org/apache/impala/analysis/ExprRewriterTest.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
M fe/src/test/java/org/apache/impala/util/PrintUtilsTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/fk-pk-join-detection.test
M testdata/workloads/functional-planner/queries/PlannerTest/kudu-selectivity.test
M testdata/workloads/functional-planner/queries/PlannerTest/max-row-size.test
M testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering-disabled.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test
M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
M testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test
M testdata/workloads/functional-planner/queries/PlannerTest/tablesample.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level2.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level3.test
M testdata/workloads/functional-query/queries/QueryTest/parquet-stats.test
M testdata/workloads/functional-query/queries/QueryTest/stats-extrapolation.test
133 files changed, 1,344 insertions(+), 464 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/19/11719/6
-- 
To view, visit http://gerrit.cloudera.org:8080/11719
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I55c3bdacc295137f66b2316a912fc347da30d6b0
Gerrit-Change-Number: 11719
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Andrew Sherman <as...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Thomas Marshall <th...@cmu.edu>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>