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 <zo...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com> Gerrit-Comment-Date: Mon, 08 Jan 2018 19:52:20 +0000 Gerrit-HasComments: Yes
