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 <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-Comment-Date: Sat, 06 Jan 2018 01:13:03 +0000 Gerrit-HasComments: Yes
