Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/21760 )
Change subject: IMPALA-12349: Support Apache Hive 2.x in Impala ...................................................................... Patch Set 93: (5 comments) This is a great work! Thanks for your efforts on this! I'd like to run exhaustive tests but the job failed due to IMPALA-14308. Please rebase this patch to get the fix. http://gerrit.cloudera.org:8080/#/c/21760/93//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21760/93//COMMIT_MSG@18 PS93, Line 18: Please summarize the changes in fe/pom.xml and where the files under fe/src/compat-apache-hive-2 come from. Are they all copied from Hive? http://gerrit.cloudera.org:8080/#/c/21760/93/bin/impala-config.sh File bin/impala-config.sh: http://gerrit.cloudera.org:8080/#/c/21760/93/bin/impala-config.sh@406 PS93, Line 406: exit 1 I think "return 1" is better since "exit 1" will close the current shell session if I'm running "source bin/impala-config.sh" and I don't have chance to see the error messages. http://gerrit.cloudera.org:8080/#/c/21760/93/fe/src/compat-apache-hive-2/java/org/apache/hadoop/hive/common/ValidReaderWriteIdList.java File fe/src/compat-apache-hive-2/java/org/apache/hadoop/hive/common/ValidReaderWriteIdList.java: http://gerrit.cloudera.org:8080/#/c/21760/93/fe/src/compat-apache-hive-2/java/org/apache/hadoop/hive/common/ValidReaderWriteIdList.java@17 PS93, Line 17: */ For files under fe/src/compat-apache-hive-2, please add a comment if they are copied from other projects and mention if there are any changes. E.g. // This code is copied from apache/thrift and modified to return // HTTP response headers from transport. // Original Source: // https://github.com/apache/thrift/blob/v0.16.0/lib/java/src/org/apache/thrift/transport/ // THttpClient.java https://github.com/apache/impala/blob/ad7888898bc76438cc90b03b2561d91854df03e1/fe/src/test/java/org/apache/impala/customcluster/THttpClientWithHeaders.java#L20-L24 http://gerrit.cloudera.org:8080/#/c/21760/93/fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java File fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java: http://gerrit.cloudera.org:8080/#/c/21760/93/fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java@452 PS93, Line 452: public static String toHivePartitionName(List<String> partitionColNames, We have some similar getPartitionName() methods in FeCatalogUtils.java. Can we move this there? We can probably reuse some codes. http://gerrit.cloudera.org:8080/#/c/21760/93/java/shaded-deps/hive-exec/pom.xml File java/shaded-deps/hive-exec/pom.xml: http://gerrit.cloudera.org:8080/#/c/21760/93/java/shaded-deps/hive-exec/pom.xml@152 PS93, Line 152: <url>https://maven.aliyun.com/repository/spring-plugin</url> Please add a comment about why we need this. -- To view, visit http://gerrit.cloudera.org:8080/21760 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib5f104dc8d131835b8118b9d54077471db65681c Gerrit-Change-Number: 21760 Gerrit-PatchSet: 93 Gerrit-Owner: ttttttz <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Zihao Ye <[email protected]> Gerrit-Reviewer: ttttttz <[email protected]> Gerrit-Comment-Date: Fri, 22 Aug 2025 11:15:08 +0000 Gerrit-HasComments: Yes
