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/01/07 06:55:41 UTC

[Impala-ASF-CR] IMPALA-8041, Part 2: Refactor SELECT list

Paul Rogers has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12144


Change subject: IMPALA-8041, Part 2: Refactor SELECT list
......................................................................

IMPALA-8041, Part 2: Refactor SELECT list

Another step toward combining analysis and rewrites. This step refactors
the SELECT list to:

* Enable each select list item to hold its "source" (before analysis and
  rewrite) SQL.
* Split the select list item into two classes: one for the wildcard
  (which holds no expresion) and the other for actual select
  expressions.
* Moves some analysis steps into the select list item itself.
* Cleans up a few other minor items.

Tests: No functional change, just refactoring. Reran all FE tests.

Change-Id: I5e173dad717e739c097f1f0cd86a0352648ff886
---
M fe/src/main/cup/sql-parser.cup
A fe/src/main/java/org/apache/impala/analysis/AbstractExpression.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectList.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/StmtRewriter.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeSubqueriesTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
12 files changed, 292 insertions(+), 130 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5e173dad717e739c097f1f0cd86a0352648ff886
Gerrit-Change-Number: 12144
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>

[Impala-ASF-CR] IMPALA-8041, Part 2: Refactor SELECT list

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

Change subject: IMPALA-8041, Part 2: Refactor SELECT list
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/1722/ : 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/12144
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5e173dad717e739c097f1f0cd86a0352648ff886
Gerrit-Change-Number: 12144
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: Mon, 07 Jan 2019 07:35:37 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8041, Part 2: Refactor SELECT list

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

Change subject: IMPALA-8041, Part 2: Refactor SELECT list
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/1868/ : 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/12144
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5e173dad717e739c097f1f0cd86a0352648ff886
Gerrit-Change-Number: 12144
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Wed, 23 Jan 2019 23:11:19 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8041, Part 2: Refactor SELECT list

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

Change subject: IMPALA-8041, Part 2: Refactor SELECT list
......................................................................


Patch Set 4:

Rebased on master to resolve merge conflicts. This is one of several steps needed to integrate rewrite rules into analysis to improve performance and remove the analyze/rewrite/reset/analyze cycle.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5e173dad717e739c097f1f0cd86a0352648ff886
Gerrit-Change-Number: 12144
Gerrit-PatchSet: 4
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 14 Feb 2019 03:17:47 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8041, Part 2: Refactor SELECT list

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

Change subject: IMPALA-8041, Part 2: Refactor SELECT list
......................................................................


Patch Set 2:

(6 comments)

Thanks Fredy for the review. Addressed your comments. Had a question about two of them.

http://gerrit.cloudera.org:8080/#/c/12144/2/fe/src/main/java/org/apache/impala/analysis/AbstractExpression.java
File fe/src/main/java/org/apache/impala/analysis/AbstractExpression.java:

http://gerrit.cloudera.org:8080/#/c/12144/2/fe/src/main/java/org/apache/impala/analysis/AbstractExpression.java@23
PS2, Line 23: Represents an expression a while
> confusing comment
Done


http://gerrit.cloudera.org:8080/#/c/12144/2/fe/src/main/java/org/apache/impala/analysis/SelectListItem.java
File fe/src/main/java/org/apache/impala/analysis/SelectListItem.java:

http://gerrit.cloudera.org:8080/#/c/12144/2/fe/src/main/java/org/apache/impala/analysis/SelectListItem.java@28
PS2, Line 28: import com.google.common.base.Predicates;
> nit: empty line after import
Done


http://gerrit.cloudera.org:8080/#/c/12144/2/fe/src/main/java/org/apache/impala/analysis/SelectListItem.java@40
PS2, Line 40: alias_
> can be final
Done


http://gerrit.cloudera.org:8080/#/c/12144/2/fe/src/main/java/org/apache/impala/analysis/SelectListItem.java@63
PS2, Line 63: public void setExpr(Expr expr) { expr_ = expr; }
> since we already have clone() method, is it necessary to make expr_ mutable
Great question; one that touches on the key issue we're trying to address.

The select list is a list of (abstract) expressions. Once we form the list (in this new version), the abstract expressions within the list will never change. This is different than the current master version in which the list of expressions do change.

The analyzer does rewrites. During that time, in the base version, the analyzer replaces one select list item with the new, rewritten expression. In this version, the abstract expression is unchanged. However, the Expr node does change after rewrite, and that new version is set user using the setExpr() method.

In the old version, we had ambiguity about whether the select list contained the original, unwritten expressions or the post-analysis, rewritten versions. (And we had code to try to reset them, which didn't quite work.)

In this version, this class holds onto both versions making it clear which is the user's "source" expression and which is rewritten.

All of this is moving toward integrating rewrites into the analyzer rather than as a separate bolt-on step.


http://gerrit.cloudera.org:8080/#/c/12144/2/fe/src/main/java/org/apache/impala/analysis/SelectListItem.java@220
PS2, Line 220:     return (SelectWildcard) item;
> Add Preconditions.checkArgument(item instanceof SelectWildcard)
Certainly could do so. But all that would do is the very same check that the cast does. Since any error thrown from the cast would clearly indicate the issue, the additional check didn't seem to add much additional checking or information. What do you think?


http://gerrit.cloudera.org:8080/#/c/12144/2/fe/src/main/java/org/apache/impala/analysis/SelectListItem.java@223
PS2, Line 223: return (SelectExpr) item;
> Add Preconditions.checkArgument(item instanceof SelectExpr)
Seme comment/question as above.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5e173dad717e739c097f1f0cd86a0352648ff886
Gerrit-Change-Number: 12144
Gerrit-PatchSet: 2
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Wed, 23 Jan 2019 22:39:12 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8041, Part 2: Refactor SELECT list

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

Change subject: IMPALA-8041, Part 2: Refactor SELECT list
......................................................................


Patch Set 2:

(6 comments)

Couple nits, but LGTM.

http://gerrit.cloudera.org:8080/#/c/12144/2/fe/src/main/java/org/apache/impala/analysis/AbstractExpression.java
File fe/src/main/java/org/apache/impala/analysis/AbstractExpression.java:

http://gerrit.cloudera.org:8080/#/c/12144/2/fe/src/main/java/org/apache/impala/analysis/AbstractExpression.java@23
PS2, Line 23: Represents an expression a while
confusing comment


http://gerrit.cloudera.org:8080/#/c/12144/2/fe/src/main/java/org/apache/impala/analysis/SelectListItem.java
File fe/src/main/java/org/apache/impala/analysis/SelectListItem.java:

http://gerrit.cloudera.org:8080/#/c/12144/2/fe/src/main/java/org/apache/impala/analysis/SelectListItem.java@28
PS2, Line 28: import com.google.common.base.Predicates;
nit: empty line after import


http://gerrit.cloudera.org:8080/#/c/12144/2/fe/src/main/java/org/apache/impala/analysis/SelectListItem.java@40
PS2, Line 40: alias_
can be final


http://gerrit.cloudera.org:8080/#/c/12144/2/fe/src/main/java/org/apache/impala/analysis/SelectListItem.java@63
PS2, Line 63: public void setExpr(Expr expr) { expr_ = expr; }
since we already have clone() method, is it necessary to make expr_ mutable?


http://gerrit.cloudera.org:8080/#/c/12144/2/fe/src/main/java/org/apache/impala/analysis/SelectListItem.java@220
PS2, Line 220:     return (SelectWildcard) item;
Add Preconditions.checkArgument(item instanceof SelectWildcard)


http://gerrit.cloudera.org:8080/#/c/12144/2/fe/src/main/java/org/apache/impala/analysis/SelectListItem.java@223
PS2, Line 223: return (SelectExpr) item;
Add Preconditions.checkArgument(item instanceof SelectExpr)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5e173dad717e739c097f1f0cd86a0352648ff886
Gerrit-Change-Number: 12144
Gerrit-PatchSet: 2
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Wed, 23 Jan 2019 21:38:13 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8041, Part 2: Refactor SELECT list

Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Fredy Wijaya, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-8041, Part 2: Refactor SELECT list
......................................................................

IMPALA-8041, Part 2: Refactor SELECT list

Another step toward combining analysis and rewrites. This step refactors
the SELECT list to:

* Enable each select list item to hold its "source" (before analysis and
  rewrite) SQL.
* Split the select list item into two classes: one for the wildcard
  (which holds no expresion) and the other for actual select
  expressions.
* Moves some analysis steps into the select list item itself.
* Cleans up a few other minor items.

Tests: No functional change, just refactoring. Reran all FE tests.

Change-Id: I5e173dad717e739c097f1f0cd86a0352648ff886
---
M fe/src/main/cup/sql-parser.cup
A fe/src/main/java/org/apache/impala/analysis/AbstractExpression.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectList.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/StmtRewriter.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeSubqueriesTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
12 files changed, 302 insertions(+), 132 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5e173dad717e739c097f1f0cd86a0352648ff886
Gerrit-Change-Number: 12144
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>

[Impala-ASF-CR] IMPALA-8041, Part 2: Refactor SELECT list

Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-8041, Part 2: Refactor SELECT list
......................................................................

IMPALA-8041, Part 2: Refactor SELECT list

Another step toward combining analysis and rewrites. This step refactors
the SELECT list to:

* Enable each select list item to hold its "source" (before analysis and
  rewrite) SQL.
* Split the select list item into two classes: one for the wildcard
  (which holds no expresion) and the other for actual select
  expressions.
* Moves some analysis steps into the select list item itself.
* Cleans up a few other minor items.

Tests: No functional change, just refactoring. Reran all FE tests.

Change-Id: I5e173dad717e739c097f1f0cd86a0352648ff886
---
M fe/src/main/cup/sql-parser.cup
A fe/src/main/java/org/apache/impala/analysis/AbstractExpression.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectList.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/StmtRewriter.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeSubqueriesTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
12 files changed, 300 insertions(+), 133 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5e173dad717e739c097f1f0cd86a0352648ff886
Gerrit-Change-Number: 12144
Gerrit-PatchSet: 2
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>

[Impala-ASF-CR] IMPALA-8041, Part 2: Refactor SELECT list

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

Change subject: IMPALA-8041, Part 2: Refactor SELECT list
......................................................................


Patch Set 1:

(4 comments)

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

http://gerrit.cloudera.org:8080/#/c/12144/1/fe/src/main/java/org/apache/impala/analysis/SelectListItem.java@108
PS1, Line 108:      * Hive's auto-generated column labels have a "_c" prefix and a select-list pos suffix,
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/12144/1/fe/src/main/java/org/apache/impala/analysis/SelectListItem.java@219
PS1, Line 219:   public static SelectWildcard asWildcard(SelectListItem item) { return (SelectWildcard) item; }
line too long (96 > 90)


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

http://gerrit.cloudera.org:8080/#/c/12144/1/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@290
PS1, Line 290:           if (existingAliasExpr != null && !existingAliasExpr.equals(selectExpr.getExpr())) {
line too long (93 > 90)


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

http://gerrit.cloudera.org:8080/#/c/12144/1/fe/src/main/java/org/apache/impala/analysis/StmtRewriter.java@261
PS1, Line 261:           new SelectList(Lists.newArrayList(SelectListItem.createColumn(wrapperResult, null))),
line too long (95 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5e173dad717e739c097f1f0cd86a0352648ff886
Gerrit-Change-Number: 12144
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: Mon, 07 Jan 2019 06:56:47 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8041, Part 2: Refactor SELECT list

Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Hello Bharath Vissapragada, Fredy Wijaya, Impala Public Jenkins, 

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

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

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

Change subject: IMPALA-8041, Part 2: Refactor SELECT list
......................................................................

IMPALA-8041, Part 2: Refactor SELECT list

Another step toward combining analysis and rewrites. This step refactors
the SELECT list to:

* Enable each select list item to hold its "source" (before analysis and
  rewrite) SQL.
* Split the select list item into two classes: one for the wildcard
  (which holds no expresion) and the other for actual select
  expressions.
* Moves some analysis steps into the select list item itself.
* Cleans up a few other minor items.

Tests: No functional change, just refactoring. Reran all FE tests.

Change-Id: I5e173dad717e739c097f1f0cd86a0352648ff886
---
M fe/src/main/cup/sql-parser.cup
A fe/src/main/java/org/apache/impala/analysis/AbstractExpression.java
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ModifyStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectList.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/StmtRewriter.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeSubqueriesTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
12 files changed, 296 insertions(+), 124 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5e173dad717e739c097f1f0cd86a0352648ff886
Gerrit-Change-Number: 12144
Gerrit-PatchSet: 4
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>

[Impala-ASF-CR] IMPALA-8041, Part 2: Refactor SELECT list

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

Change subject: IMPALA-8041, Part 2: Refactor SELECT list
......................................................................


Patch Set 2:

Rebased on master. Fixed code style issues.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5e173dad717e739c097f1f0cd86a0352648ff886
Gerrit-Change-Number: 12144
Gerrit-PatchSet: 2
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: Mon, 07 Jan 2019 22:42:23 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8041, Part 2: Refactor SELECT list

Posted by "Paul Rogers (Code Review)" <ge...@cloudera.org>.
Paul Rogers has abandoned this change. ( http://gerrit.cloudera.org:8080/12144 )

Change subject: IMPALA-8041, Part 2: Refactor SELECT list
......................................................................


Abandoned

Not a priority at the moment.
-- 
To view, visit http://gerrit.cloudera.org:8080/12144
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I5e173dad717e739c097f1f0cd86a0352648ff886
Gerrit-Change-Number: 12144
Gerrit-PatchSet: 4
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>

[Impala-ASF-CR] IMPALA-8041, Part 2: Refactor SELECT list

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

Change subject: IMPALA-8041, Part 2: Refactor SELECT list
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/2121/ : 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/12144
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5e173dad717e739c097f1f0cd86a0352648ff886
Gerrit-Change-Number: 12144
Gerrit-PatchSet: 4
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Thu, 14 Feb 2019 03:56:45 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8041, Part 2: Refactor SELECT list

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

Change subject: IMPALA-8041, Part 2: Refactor SELECT list
......................................................................


Patch Set 1:

Passed pre-review tests: https://jenkins.impala.io/job/pre-review-test/274/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5e173dad717e739c097f1f0cd86a0352648ff886
Gerrit-Change-Number: 12144
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Mon, 07 Jan 2019 06:56:03 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8041, Part 2: Refactor SELECT list

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

Change subject: IMPALA-8041, Part 2: Refactor SELECT list
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/1727/ : 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/12144
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5e173dad717e739c097f1f0cd86a0352648ff886
Gerrit-Change-Number: 12144
Gerrit-PatchSet: 2
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: Mon, 07 Jan 2019 23:15:44 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8041, Part 2: Refactor SELECT list

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

Change subject: IMPALA-8041, Part 2: Refactor SELECT list
......................................................................


Patch Set 3: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/12144/2/fe/src/main/java/org/apache/impala/analysis/SelectListItem.java
File fe/src/main/java/org/apache/impala/analysis/SelectListItem.java:

http://gerrit.cloudera.org:8080/#/c/12144/2/fe/src/main/java/org/apache/impala/analysis/SelectListItem.java@63
PS2, Line 63: public String getAlias() { return alias_; }
> Great question; one that touches on the key issue we're trying to address.
Thanks for the explanation.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5e173dad717e739c097f1f0cd86a0352648ff886
Gerrit-Change-Number: 12144
Gerrit-PatchSet: 3
Gerrit-Owner: Paul Rogers <pr...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <pr...@cloudera.com>
Gerrit-Comment-Date: Wed, 23 Jan 2019 23:22:51 +0000
Gerrit-HasComments: Yes