You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Paul Rogers (Code Review)" <ge...@cloudera.org> on 2019/02/02 22:17:51 UTC
[Impala-ASF-CR] IMPALA-8156: Add format options to the EXPLAIN statement
Paul Rogers has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12340
Change subject: IMPALA-8156: Add format options to the EXPLAIN statement
......................................................................
IMPALA-8156: Add format options to the EXPLAIN statement
Adds an optional FORMAT clause to the EXPLAIN statement. Adds two
options:
* level='<level>' to set the explain level, and
* rewritten="true" to show rewritten SQL with views expanded
The new syntax is backward-compatible with the existing EXPLAIN option.
Examples:
EXPLAIN FORMAT(rewritten='true') SELECT * FROM view_view;
EXPLAIN FORMAT(level='verbose') SELECT * FROM alltypes;
Please see IMPALA-8156 for the details of syntax and semantics, and for
potential future extensions.
Tests: added tests for the new syntax, and to verify that each option
returns the expected results.
Change-Id: I84a8bc4fbff52b70a747f3a3c08abe6973e37fc1
---
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
A fe/src/main/java/org/apache/impala/analysis/ExplainOptions.java
M fe/src/main/java/org/apache/impala/analysis/FromClause.java
M fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java
M fe/src/main/java/org/apache/impala/analysis/StatementBase.java
M fe/src/main/java/org/apache/impala/analysis/ToSqlOptions.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/planner/PlannerContext.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
A fe/src/test/java/org/apache/impala/planner/ExplainTest.java
12 files changed, 438 insertions(+), 78 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/40/12340/1
--
To view, visit http://gerrit.cloudera.org:8080/12340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I84a8bc4fbff52b70a747f3a3c08abe6973e37fc1
Gerrit-Change-Number: 12340
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
[Impala-ASF-CR] IMPALA-8156: Add format options to the EXPLAIN statement
Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Paul Rogers has posted comments on this change. ( http://gerrit.cloudera.org:8080/12340 )
Change subject: IMPALA-8156: Add format options to the EXPLAIN statement
......................................................................
Patch Set 1:
This version is for preview: do we like the idea? Still need to add authorization checks for view expansion: will only expand views for which the user has permissions on the underlying tables.
--
To view, visit http://gerrit.cloudera.org:8080/12340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84a8bc4fbff52b70a747f3a3c08abe6973e37fc1
Gerrit-Change-Number: 12340
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 07 Feb 2019 00:08:49 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8156: Add format options to the EXPLAIN statement
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12340 )
Change subject: IMPALA-8156: Add format options to the EXPLAIN statement
......................................................................
Patch Set 1:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/1970/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/12340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84a8bc4fbff52b70a747f3a3c08abe6973e37fc1
Gerrit-Change-Number: 12340
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Sat, 02 Feb 2019 22:42:56 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8156: Add format options to the EXPLAIN statement
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12340 )
Change subject: IMPALA-8156: Add format options to the EXPLAIN statement
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/12340/1/fe/src/main/java/org/apache/impala/analysis/ExplainOptions.java
File fe/src/main/java/org/apache/impala/analysis/ExplainOptions.java:
http://gerrit.cloudera.org:8080/#/c/12340/1/fe/src/main/java/org/apache/impala/analysis/ExplainOptions.java@119
PS1, Line 119: public static TExplainLevel getExplainLevel(ExplainOptions opts, TQueryOptions queryOptions) {
line too long (96 > 90)
http://gerrit.cloudera.org:8080/#/c/12340/1/fe/src/main/java/org/apache/impala/planner/PlannerContext.java
File fe/src/main/java/org/apache/impala/planner/PlannerContext.java:
http://gerrit.cloudera.org:8080/#/c/12340/1/fe/src/main/java/org/apache/impala/planner/PlannerContext.java@116
PS1, Line 116: explainLevel = ExplainOptions.getExplainLevel(getAnalysisResult().explainOptions(), getQueryOptions());
line too long (109 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/12340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84a8bc4fbff52b70a747f3a3c08abe6973e37fc1
Gerrit-Change-Number: 12340
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Sat, 02 Feb 2019 22:18:52 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8156: Add format options to the EXPLAIN statement
Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/12340 )
Change subject: IMPALA-8156: Add format options to the EXPLAIN statement
......................................................................
Patch Set 1:
(7 comments)
I think this is super helpful with debugging. Glad we will be able to pass explain opts inline with the EXPLAIN statement rather than as a session option. It helps to see the EXPANDED and REWRITTEN forms of the query, especially with complex queries containing lots of views. It probably helps to clarify what is the difference between these two formats with some examples.
** After going through the parser changes, I figured this is a preview patch. I still preserved my comments there for the record.
http://gerrit.cloudera.org:8080/#/c/12340/1/fe/src/main/cup/sql-parser.cup
File fe/src/main/cup/sql-parser.cup:
http://gerrit.cloudera.org:8080/#/c/12340/1/fe/src/main/cup/sql-parser.cup@740
PS1, Line 740: setExplain
call it setExplainOpts()?
http://gerrit.cloudera.org:8080/#/c/12340/1/fe/src/main/cup/sql-parser.cup@743
PS1, Line 743: KW_EXPLAIN KW_FORMAT LPAREN RPAREN explainable_stmt:stmt
: {:
: stmt.setExplain(new ExplainOptions());
: RESULT = stmt;
: :}
Handle this in the explain_opts below, as an empty case?
http://gerrit.cloudera.org:8080/#/c/12340/1/fe/src/main/cup/sql-parser.cup@773
PS1, Line 773: HashMap
Use a LinkedHashMap to preserve the order, just incase we need to break the ties somewhere.
http://gerrit.cloudera.org:8080/#/c/12340/1/fe/src/main/cup/sql-parser.cup@776
PS1, Line 776: explain_opts
btw, we do have a properties_map below incase you didn't notice it already. It is for string:string mapping though.
http://gerrit.cloudera.org:8080/#/c/12340/1/fe/src/main/cup/sql-parser.cup@790
PS1, Line 790: KW_FORMAT EQUAL STRING_LITERAL:value
: {: RESULT = new Pair<String,String>(ExplainOptions.FORMAT_KEY, value); :}
I don't understand this.
FORMAT (FORMAT="foo")?
http://gerrit.cloudera.org:8080/#/c/12340/1/fe/src/main/cup/sql-parser.cup@799
PS1, Line 799: STRING_LITERAL:value
: {: RESULT = new Pair<String,String>(ExplainOptions.FORMAT_KEY, value); :}
: ;
nit: Multiple keys seem to be overwriting each other, especially if there are multiple STRING_LITERALs.
http://gerrit.cloudera.org:8080/#/c/12340/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/12340/1/fe/src/main/java/org/apache/impala/analysis/ToSqlOptions.java@41
PS1, Line 41: SHOW_IMPLICIT_CASTS(true, true),
: EXPANDED(true, true);
Could you clarify what is the difference between EXPANDED vs REWRITTEN? Probably give a couple of examples where EXPANDED is different from REWRITTEN (and when to use them)
--
To view, visit http://gerrit.cloudera.org:8080/12340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I84a8bc4fbff52b70a747f3a3c08abe6973e37fc1
Gerrit-Change-Number: 12340
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 07 Feb 2019 07:38:25 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8156: Add format options to the EXPLAIN statement
Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Paul Rogers has abandoned this change. ( http://gerrit.cloudera.org:8080/12340 )
Change subject: IMPALA-8156: Add format options to the EXPLAIN statement
......................................................................
Abandoned
Not a priority at the moment.
--
To view, visit http://gerrit.cloudera.org:8080/12340
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I84a8bc4fbff52b70a747f3a3c08abe6973e37fc1
Gerrit-Change-Number: 12340
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>