You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Anonymous Coward (Code Review)" <ge...@cloudera.org> on 2017/01/09 22:09:37 UTC

[Impala-ASF-CR] IMPALA-4036: show create table outputs invalid SQL for partitioned tables with comments 

joemcdonnell@cloudera.com has uploaded a new change for review.

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

Change subject: IMPALA-4036: show create table outputs invalid SQL for partitioned tables with comments
......................................................................

IMPALA-4036: show create table outputs invalid SQL for partitioned tables
with comments

For a table that has both a table comment and a partition specified,
"show create table" incorrectly outputs the comment before the partition.
This is not the correct order, and it results in an invalid SQL.

This transaction fixes the ordering (partition comes before comment) and
adds tests for this case.

Change-Id: Iccf9488e8b1d28fcaf3a40f935157c9dd792812e

Update code style of ToSqlTest.java

Change-Id: I29a33cfd142b473997fdc3acfe3f0966bc7ed784
---
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
3 files changed, 42 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I29a33cfd142b473997fdc3acfe3f0966bc7ed784
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: joemcdonnell@cloudera.com

[Impala-ASF-CR] IMPALA-4036: show create table outputs invalid SQL for partitioned tables with comments

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change.

Change subject: IMPALA-4036: show create table outputs invalid SQL for partitioned tables with comments
......................................................................


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/5648/1//COMMIT_MSG
Commit Message:

Line 7: IMPALA-4036: show create table outputs invalid SQL for partitioned tables
We always put the description on a single line.


Line 17: Change-Id: Iccf9488e8b1d28fcaf3a40f935157c9dd792812e
Not sure what you did to end up with two Change-Ids. Maybe you squashed two commits together? I'm guessing this is related to the line after this 'Update code style...', which I'm not sure what it means.

Either way, you should only have one.


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

PS1, Line 307:   
Extra spaces.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I29a33cfd142b473997fdc3acfe3f0966bc7ed784
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: joemcdonnell@cloudera.com
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4036: show create table outputs invalid SQL for partitioned table with comment

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change.

Change subject: IMPALA-4036: show create table outputs invalid SQL for partitioned table with comment
......................................................................


Patch Set 2: Code-Review+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I29a33cfd142b473997fdc3acfe3f0966bc7ed784
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: joemcdonnell@cloudera.com
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4036: invalid SQL generated for partitioned table with comment

Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has posted comments on this change.

Change subject: IMPALA-4036: invalid SQL generated for partitioned table with comment
......................................................................


Patch Set 4: Code-Review+2

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I29a33cfd142b473997fdc3acfe3f0966bc7ed784
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4036: invalid SQL generated for partitioned table with comment

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4036: invalid SQL generated for partitioned table with comment
......................................................................


Patch Set 4: Verified+1

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I29a33cfd142b473997fdc3acfe3f0966bc7ed784
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4036: show create table outputs invalid SQL for partitioned table with comment

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
joemcdonnell@cloudera.com has uploaded a new patch set (#2).

Change subject: IMPALA-4036: show create table outputs invalid SQL for partitioned table with comment
......................................................................

IMPALA-4036: show create table outputs invalid SQL for partitioned table with comment

For a table that has both a table comment and a partition specified,
"show create table" incorrectly outputs the comment before the partition.
This is not the correct order, and it results in an invalid SQL.

This transaction fixes the ordering (partition comes before comment) and
adds tests for this case.

Change-Id: I29a33cfd142b473997fdc3acfe3f0966bc7ed784
---
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
3 files changed, 42 insertions(+), 1 deletion(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I29a33cfd142b473997fdc3acfe3f0966bc7ed784
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: joemcdonnell@cloudera.com
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>

[Impala-ASF-CR] IMPALA-4036: show create table outputs invalid SQL for partitioned table with comment

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Thomas Tauber-Marshall,

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

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

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

Change subject: IMPALA-4036: show create table outputs invalid SQL for partitioned table with comment
......................................................................

IMPALA-4036: show create table outputs invalid SQL for partitioned table with comment

For a table that has both a table comment and a partition specified,
"show create table" incorrectly outputs the comment before the partition.
This is not the correct order, and it results in an invalid SQL.

This transaction fixes the ordering (partition comes before comment) and
adds tests for this case.

Change-Id: I29a33cfd142b473997fdc3acfe3f0966bc7ed784
---
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
3 files changed, 8 insertions(+), 3 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I29a33cfd142b473997fdc3acfe3f0966bc7ed784
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>

[Impala-ASF-CR] IMPALA-4036: show create table outputs invalid SQL for partitioned table with comment

Posted by "Marcel Kornacker (Code Review)" <ge...@cloudera.org>.
Marcel Kornacker has posted comments on this change.

Change subject: IMPALA-4036: show create table outputs invalid SQL for partitioned table with comment
......................................................................


Patch Set 3: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/5648/3//COMMIT_MSG
Commit Message:

Line 7: IMPALA-4036: show create table outputs invalid SQL for partitioned table with comment
long line


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I29a33cfd142b473997fdc3acfe3f0966bc7ed784
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-4036: invalid SQL generated for partitioned table with comment

Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has submitted this change and it was merged.

Change subject: IMPALA-4036: invalid SQL generated for partitioned table with comment
......................................................................


IMPALA-4036: invalid SQL generated for partitioned table with comment

For a table that has both a table comment and a partition specified,
"show create table" incorrectly outputs the comment before the partition.
This is not the correct order, and it results in an invalid SQL.

This transaction fixes the ordering (partition comes before comment) and
adds tests for this case.

Change-Id: I29a33cfd142b473997fdc3acfe3f0966bc7ed784
Reviewed-on: http://gerrit.cloudera.org:8080/5648
Tested-by: Impala Public Jenkins
Reviewed-by: Henry Robinson <he...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
3 files changed, 8 insertions(+), 3 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Henry Robinson: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I29a33cfd142b473997fdc3acfe3f0966bc7ed784
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>

[Impala-ASF-CR] IMPALA-4036: invalid SQL generated for partitioned table with comment

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change.

Change subject: IMPALA-4036: invalid SQL generated for partitioned table with comment
......................................................................


Patch Set 4:

Build started: http://jenkins.impala.io:8080/job/gerrit-verify-dryrun/169/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I29a33cfd142b473997fdc3acfe3f0966bc7ed784
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-4036: invalid SQL generated for partitioned table with comment

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Hello Marcel Kornacker, Thomas Tauber-Marshall,

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

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

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

Change subject: IMPALA-4036: invalid SQL generated for partitioned table with comment
......................................................................

IMPALA-4036: invalid SQL generated for partitioned table with comment

For a table that has both a table comment and a partition specified,
"show create table" incorrectly outputs the comment before the partition.
This is not the correct order, and it results in an invalid SQL.

This transaction fixes the ordering (partition comes before comment) and
adds tests for this case.

Change-Id: I29a33cfd142b473997fdc3acfe3f0966bc7ed784
---
M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
3 files changed, 8 insertions(+), 3 deletions(-)


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

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I29a33cfd142b473997fdc3acfe3f0966bc7ed784
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>

[Impala-ASF-CR] IMPALA-4036: show create table outputs invalid SQL for partitioned table with comment

Posted by "Marcel Kornacker (Code Review)" <ge...@cloudera.org>.
Marcel Kornacker has posted comments on this change.

Change subject: IMPALA-4036: show create table outputs invalid SQL for partitioned table with comment
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5648/2/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
File fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java:

Line 305:   public void TestCreateTableWCommentPartition() throws AnalysisException {
just roll this into an existing test, no need to have something separate (each statement takes a little bit of time to execute).


http://gerrit.cloudera.org:8080/#/c/5648/2/testdata/workloads/functional-query/queries/QueryTest/show-create-table.test
File testdata/workloads/functional-query/queries/QueryTest/show-create-table.test:

Line 119: # With a table comment and a partition
same here, roll this into an existing test case.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I29a33cfd142b473997fdc3acfe3f0966bc7ed784
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <ma...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-HasComments: Yes