[Impala-ASF-CR] IMPALA-6307: CTAS statement fails with duplicate column exception.

2018-01-12 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/8930 )

Change subject: IMPALA-6307: CTAS statement fails with duplicate column 
exception.
..

IMPALA-6307: CTAS statement fails with duplicate column exception.

A CTAS statement with a 'partition by' clause causes the statement
to fail with a duplicate column name exception. This is happening
because on expression rewrite, the partition defs state is not reset.

IMPALA-5796 added TableDef::reset(). This patch expands the method by
adding calls to reset ColumnDefs and PartitionColumnDefs.

Testing:
  * Regression test added to AnalyzeDDLTest.
  * Exhaustive Jenkins build and test.

Change-Id: Iee053abecd4384e15eec8db10cb06f5ace159da2
Reviewed-on: http://gerrit.cloudera.org:8080/8930
Reviewed-by: Dimitris Tsirogiannis 
Tested-by: Impala Public Jenkins
---
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java
M fe/src/main/java/org/apache/impala/analysis/TableDef.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
5 files changed, 27 insertions(+), 5 deletions(-)

Approvals:
  Dimitris Tsirogiannis: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Iee053abecd4384e15eec8db10cb06f5ace159da2
Gerrit-Change-Number: 8930
Gerrit-PatchSet: 6
Gerrit-Owner: Zoram Thanga 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 


[Impala-ASF-CR] IMPALA-6307: CTAS statement fails with duplicate column exception.

2018-01-12 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8930 )

Change subject: IMPALA-6307: CTAS statement fails with duplicate column 
exception.
..


Patch Set 5: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee053abecd4384e15eec8db10cb06f5ace159da2
Gerrit-Change-Number: 8930
Gerrit-PatchSet: 5
Gerrit-Owner: Zoram Thanga 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Sat, 13 Jan 2018 03:44:41 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6307: CTAS statement fails with duplicate column exception.

2018-01-12 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8930 )

Change subject: IMPALA-6307: CTAS statement fails with duplicate column 
exception.
..


Patch Set 5:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1723/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee053abecd4384e15eec8db10cb06f5ace159da2
Gerrit-Change-Number: 8930
Gerrit-PatchSet: 5
Gerrit-Owner: Zoram Thanga 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Sat, 13 Jan 2018 00:11:10 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6307: CTAS statement fails with duplicate column exception.

2018-01-12 Thread Dimitris Tsirogiannis (Code Review)
Dimitris Tsirogiannis has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8930 )

Change subject: IMPALA-6307: CTAS statement fails with duplicate column 
exception.
..


Patch Set 5: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee053abecd4384e15eec8db10cb06f5ace159da2
Gerrit-Change-Number: 8930
Gerrit-PatchSet: 5
Gerrit-Owner: Zoram Thanga 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Sat, 13 Jan 2018 00:10:57 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6307: CTAS statement fails with duplicate column exception.

2018-01-11 Thread Zoram Thanga (Code Review)
Hello Michael Ho, Dimitris Tsirogiannis, Alex Behm,

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

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

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

Change subject: IMPALA-6307: CTAS statement fails with duplicate column 
exception.
..

IMPALA-6307: CTAS statement fails with duplicate column exception.

A CTAS statement with a 'partition by' clause causes the statement
to fail with a duplicate column name exception. This is happening
because on expression rewrite, the partition defs state is not reset.

IMPALA-5796 added TableDef::reset(). This patch expands the method by
adding calls to reset ColumnDefs and PartitionColumnDefs.

Testing:
  * Regression test added to AnalyzeDDLTest.
  * Exhaustive Jenkins build and test.

Change-Id: Iee053abecd4384e15eec8db10cb06f5ace159da2
---
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java
M fe/src/main/java/org/apache/impala/analysis/TableDef.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
5 files changed, 27 insertions(+), 5 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iee053abecd4384e15eec8db10cb06f5ace159da2
Gerrit-Change-Number: 8930
Gerrit-PatchSet: 4
Gerrit-Owner: Zoram Thanga 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 


[Impala-ASF-CR] IMPALA-6307: CTAS statement fails with duplicate column exception.

2018-01-11 Thread Zoram Thanga (Code Review)
Zoram Thanga has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8930 )

Change subject: IMPALA-6307: CTAS statement fails with duplicate column 
exception.
..


Patch Set 3:

(1 comment)

Thanks for the comment. Have added another test case, this one for Kudu.

http://gerrit.cloudera.org:8080/#/c/8930/3/fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java
File fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java:

http://gerrit.cloudera.org:8080/#/c/8930/3/fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java@58
PS3, Line 58: kuduPartitionParams_.clear();
> This is new. Do we have any existing tests with a CTAS on Kudu where the se
Added a new CTAS test for Kudu, which is the same repro test case tailored for 
Kudu.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee053abecd4384e15eec8db10cb06f5ace159da2
Gerrit-Change-Number: 8930
Gerrit-PatchSet: 3
Gerrit-Owner: Zoram Thanga 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Thu, 11 Jan 2018 22:44:01 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6307: CTAS statement fails with duplicate column exception.

2018-01-09 Thread Dimitris Tsirogiannis (Code Review)
Dimitris Tsirogiannis has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8930 )

Change subject: IMPALA-6307: CTAS statement fails with duplicate column 
exception.
..


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/8930/3/fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java
File fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java:

http://gerrit.cloudera.org:8080/#/c/8930/3/fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java@58
PS3, Line 58: kuduPartitionParams_.clear();
This is new. Do we have any existing tests with a CTAS on Kudu where the select 
gets rewritten? You may want to verify that nothing breaks.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee053abecd4384e15eec8db10cb06f5ace159da2
Gerrit-Change-Number: 8930
Gerrit-PatchSet: 3
Gerrit-Owner: Zoram Thanga 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Tue, 09 Jan 2018 23:16:35 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6307: CTAS statement fails with duplicate column exception.

2018-01-09 Thread Zoram Thanga (Code Review)
Hello Michael Ho, Dimitris Tsirogiannis, Alex Behm,

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

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

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

Change subject: IMPALA-6307: CTAS statement fails with duplicate column 
exception.
..

IMPALA-6307: CTAS statement fails with duplicate column exception.

A CTAS statement with a 'partition by' clause causes the statement
to fail with a duplicate column name exception. This is happening
because on expression rewrite, the partition defs state is not reset.

IMPALA-5796 added TableDef::reset(). This patch expands the method by
adding calls to reset ColumnDefs and PartitionColumnDefs.

Testing:
  * Regression test added to AnalyzeDDLTest.
  * Exhaustive Jenkins build and test.

Change-Id: Iee053abecd4384e15eec8db10cb06f5ace159da2
---
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java
M fe/src/main/java/org/apache/impala/analysis/TableDef.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
5 files changed, 21 insertions(+), 5 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iee053abecd4384e15eec8db10cb06f5ace159da2
Gerrit-Change-Number: 8930
Gerrit-PatchSet: 3
Gerrit-Owner: Zoram Thanga 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 


[Impala-ASF-CR] IMPALA-6307: CTAS statement fails with duplicate column exception.

2018-01-09 Thread Zoram Thanga (Code Review)
Zoram Thanga has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8930 )

Change subject: IMPALA-6307: CTAS statement fails with duplicate column 
exception.
..


Patch Set 2:

(3 comments)

Thanks! Uploading patch set #3.

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

http://gerrit.cloudera.org:8080/#/c/8930/2/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java@408
PS2, Line 408: // of the table that will be created.
> including the  partition columns, if any.
Done


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

http://gerrit.cloudera.org:8080/#/c/8930/2/fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java@56
PS2, Line 56: partitionColDefs_.clear(); kuduPartitionParams_.clear();
> formatting nit: one statement per line
Fixed.


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

http://gerrit.cloudera.org:8080/#/c/8930/2/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java@1510
PS2, Line 1510: IMPALA-6307: A CTAS query fails with error: AnalysisException:
  : // Duplicate column name: 
> I think it's best to outline what this query is testing, i.e. CTAS with a s
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee053abecd4384e15eec8db10cb06f5ace159da2
Gerrit-Change-Number: 8930
Gerrit-PatchSet: 2
Gerrit-Owner: Zoram Thanga 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Tue, 09 Jan 2018 22:04:56 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6307: CTAS statement fails with duplicate column exception.

2018-01-09 Thread Dimitris Tsirogiannis (Code Review)
Dimitris Tsirogiannis has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8930 )

Change subject: IMPALA-6307: CTAS statement fails with duplicate column 
exception.
..


Patch Set 2:

(3 comments)

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

http://gerrit.cloudera.org:8080/#/c/8930/2/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java@408
PS2, Line 408: // of the table that will be created.
including the  partition columns, if any.


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

http://gerrit.cloudera.org:8080/#/c/8930/2/fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java@56
PS2, Line 56: partitionColDefs_.clear(); kuduPartitionParams_.clear();
formatting nit: one statement per line


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

http://gerrit.cloudera.org:8080/#/c/8930/2/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java@1510
PS2, Line 1510: IMPALA-6307: A CTAS query fails with error: AnalysisException:
  : // Duplicate column name: 
I think it's best to outline what this query is testing, i.e. CTAS with a 
select query that requires a rewrite (IMPALA-6307).



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee053abecd4384e15eec8db10cb06f5ace159da2
Gerrit-Change-Number: 8930
Gerrit-PatchSet: 2
Gerrit-Owner: Zoram Thanga 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Michael Ho 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Tue, 09 Jan 2018 21:28:12 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6307: CTAS statement fails with duplicate column exception.

2018-01-08 Thread Zoram Thanga (Code Review)
Zoram Thanga has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8930 )

Change subject: IMPALA-6307: CTAS statement fails with duplicate column 
exception.
..


Patch Set 1:

(3 comments)

Thanks for the comments. Please have a look at patch set #2.

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

http://gerrit.cloudera.org:8080/#/c/8930/1/fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java@223
PS1, Line 223: Preconditions.checkState(colDefs.size() + 
partitionColDefs.size() == types.size());
 : for (int i = 0; i < colDefs.size(); ++i) 
colDefs.get(i).setType(types.get(i));
 : for (int i = 0; i < partitionColDefs.size(); ++i) {
 :   partitionColDefs.get(i).setType(types.get(i + 
colDefs.size()));
 : }
> I believe it would be nice to add a comment in AnalysisContext.java L405 to
Done


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

http://gerrit.cloudera.org:8080/#/c/8930/1/fe/src/main/java/org/apache/impala/analysis/TableDef.java@160
PS1, Line 160: dataLayout_.getPartitionColumnDefs().clear();
> I would add a reset() function in TableDataLayout() and put that there.
Done


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

http://gerrit.cloudera.org:8080/#/c/8930/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java@1510
PS1, Line 1510: AnalyzesOk("create table functional.ctas_tbl partitioned by 
(year) as " +
  : "with tmp as (select a.timestamp_col, a.year from 
functional.alltypes a " +
  : "left join functional.alltypes b " +
  : "on b.timestamp_col between a.timestamp_col and 
a.timestamp_col) " +
  : "select a.timestamp_col, a.year from tmp a");
> Add a comment. It will not be clear to everyone what this thing is testing.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee053abecd4384e15eec8db10cb06f5ace159da2
Gerrit-Change-Number: 8930
Gerrit-PatchSet: 1
Gerrit-Owner: Zoram Thanga 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Zoram Thanga 
Gerrit-Comment-Date: Mon, 08 Jan 2018 19:52:20 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6307: CTAS statement fails with duplicate column exception.

2018-01-08 Thread Zoram Thanga (Code Review)
Hello Dimitris Tsirogiannis, Alex Behm,

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

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

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

Change subject: IMPALA-6307: CTAS statement fails with duplicate column 
exception.
..

IMPALA-6307: CTAS statement fails with duplicate column exception.

A CTAS statement with a 'partition by' clause causes the statement
to fail with a duplicate column name exception. This is happening
because on expression rewrite, the partition defs state is not reset.

IMPALA-5796 added TableDef::reset(). This patch expands the method by
adding calls to reset ColumnDefs and PartitionColumnDefs.

Testing:
  * Regression test added to AnalyzeDDLTest.
  * Exhaustive Jenkins build and test.

Change-Id: Iee053abecd4384e15eec8db10cb06f5ace159da2
---
M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/TableDataLayout.java
M fe/src/main/java/org/apache/impala/analysis/TableDef.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
5 files changed, 19 insertions(+), 5 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iee053abecd4384e15eec8db10cb06f5ace159da2
Gerrit-Change-Number: 8930
Gerrit-PatchSet: 2
Gerrit-Owner: Zoram Thanga 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Reviewer: Zoram Thanga 


[Impala-ASF-CR] IMPALA-6307: CTAS statement fails with duplicate column exception.

2018-01-05 Thread Dimitris Tsirogiannis (Code Review)
Dimitris Tsirogiannis has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8930 )

Change subject: IMPALA-6307: CTAS statement fails with duplicate column 
exception.
..


Patch Set 1:

(3 comments)

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

http://gerrit.cloudera.org:8080/#/c/8930/1/fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java@223
PS1, Line 223: Preconditions.checkState(colDefs.size() + 
partitionColDefs.size() == types.size());
 : for (int i = 0; i < colDefs.size(); ++i) 
colDefs.get(i).setType(types.get(i));
 : for (int i = 0; i < partitionColDefs.size(); ++i) {
 :   partitionColDefs.get(i).setType(types.get(i + 
colDefs.size()));
 : }
I believe it would be nice to add a comment in AnalysisContext.java L405 to 
indicate what these types represent for the case of a CTAS. Then it will be 
more clear why this block is needed.


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

http://gerrit.cloudera.org:8080/#/c/8930/1/fe/src/main/java/org/apache/impala/analysis/TableDef.java@160
PS1, Line 160: dataLayout_.getPartitionColumnDefs().clear();
I would add a reset() function in TableDataLayout() and put that there.


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

http://gerrit.cloudera.org:8080/#/c/8930/1/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java@1510
PS1, Line 1510: AnalyzesOk("create table functional.ctas_tbl partitioned by 
(year) as " +
  : "with tmp as (select a.timestamp_col, a.year from 
functional.alltypes a " +
  : "left join functional.alltypes b " +
  : "on b.timestamp_col between a.timestamp_col and 
a.timestamp_col) " +
  : "select a.timestamp_col, a.year from tmp a");
Add a comment. It will not be clear to everyone what this thing is testing.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iee053abecd4384e15eec8db10cb06f5ace159da2
Gerrit-Change-Number: 8930
Gerrit-PatchSet: 1
Gerrit-Owner: Zoram Thanga 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Dimitris Tsirogiannis 
Gerrit-Comment-Date: Sat, 06 Jan 2018 01:13:03 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-6307: CTAS statement fails with duplicate column exception.

2018-01-03 Thread Zoram Thanga (Code Review)
Zoram Thanga has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/8930


Change subject: IMPALA-6307: CTAS statement fails with duplicate column 
exception.
..

IMPALA-6307: CTAS statement fails with duplicate column exception.

A CTAS statement with a 'partition by' clause causes the statement
to fail with a duplicate column name exception. This is happening
because on expression rewrite, the partition defs state is not reset.

IMPALA-5796 added TableDef::reset(). This patch expands the method by
adding calls to reset ColumnDefs and PartitionColumnDefs.

Testing:
  * Regression test added to AnalyzeDDLTest.
  * Exhaustive Jenkins build and test.

Change-Id: Iee053abecd4384e15eec8db10cb06f5ace159da2
---
M fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java
M fe/src/main/java/org/apache/impala/analysis/TableDef.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
3 files changed, 13 insertions(+), 4 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iee053abecd4384e15eec8db10cb06f5ace159da2
Gerrit-Change-Number: 8930
Gerrit-PatchSet: 1
Gerrit-Owner: Zoram Thanga