Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/14811 )
Change subject: WIP: IMPALA-8046: Support CREATE TABLE from an ORC file ...................................................................... Patch Set 7: (8 comments) Thanks for taking care of this! I did a run through. The patch seems okay for me, however my major concern is the HIVE3 dependency for ORC support. As I found I can create and query ORC table even with HIVE2. Are you sure about this dependency? http://gerrit.cloudera.org:8080/#/c/14811/7/fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java File fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java: http://gerrit.cloudera.org:8080/#/c/14811/7/fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java@74 PS7, Line 74: Scheme nit: Isn't this meant to be Schema http://gerrit.cloudera.org:8080/#/c/14811/7/fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java@77 PS7, Line 77: Scheme nit: same as above http://gerrit.cloudera.org:8080/#/c/14811/7/fe/src/main/java/org/apache/impala/analysis/ORCSchemeExtractor.java File fe/src/main/java/org/apache/impala/analysis/ORCSchemeExtractor.java: http://gerrit.cloudera.org:8080/#/c/14811/7/fe/src/main/java/org/apache/impala/analysis/ORCSchemeExtractor.java@50 PS7, Line 50: ERROR_MSG I haven't seen any tests for this error msg. Did I miss something? http://gerrit.cloudera.org:8080/#/c/14811/7/fe/src/main/java/org/apache/impala/analysis/ORCSchemeExtractor.java@139 PS7, Line 139: fieldNames.get(i) Might worth a DCHECK here as well that the size of fieldNames equals to the size of subTypes. http://gerrit.cloudera.org:8080/#/c/14811/7/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/14811/7/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java@2029 PS7, Line 2029: TestCreateTableLikeORC I'd also add the "file does't exist" coverage to this function. http://gerrit.cloudera.org:8080/#/c/14811/7/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java@2029 PS7, Line 2029: TestCreateTableLikeORC To be in line with TestCreateTableLikeFile() could you rename this to TestCreateTableLikeFileORC()? http://gerrit.cloudera.org:8080/#/c/14811/7/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java@2030 PS7, Line 2030: ORC is only supported in Impala with Hive 3. I though Apache Impala has ORC support regardless of the Hive version. I can for instance create and query an ORC table without compiling with USE_CDP_HIVE=true. http://gerrit.cloudera.org:8080/#/c/14811/7/testdata/workloads/functional-query/queries/QueryTest/create-table-like-table.test File testdata/workloads/functional-query/queries/QueryTest/create-table-like-table.test: http://gerrit.cloudera.org:8080/#/c/14811/7/testdata/workloads/functional-query/queries/QueryTest/create-table-like-table.test@254 PS7, Line 254: --- QUERY Why was this parquet test necessary for this ORC related patch? -- To view, visit http://gerrit.cloudera.org:8080/14811 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I77cd84cda2ed86516937a67eb320fd41e3f1cf2d Gerrit-Change-Number: 14811 Gerrit-PatchSet: 7 Gerrit-Owner: Norbert Luksa <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Daniel Becker <[email protected]> Gerrit-Reviewer: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Norbert Luksa <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Tue, 03 Dec 2019 15:21:29 +0000 Gerrit-HasComments: Yes
