Andrew Sherman has posted comments on this change. ( http://gerrit.cloudera.org:8080/20110 )
Change subject: IMPALA-11901: Support COPY TESTCASE in local catalog mode ...................................................................... Patch Set 4: (7 comments) Looks good, I have a few small nits and questions http://gerrit.cloudera.org:8080/#/c/20110/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/20110/4//COMMIT_MSG@10 PS4, Line 10: in Nit: "on the" http://gerrit.cloudera.org:8080/#/c/20110/4//COMMIT_MSG@19 PS4, Line 19: of Nit: "as" http://gerrit.cloudera.org:8080/#/c/20110/4//COMMIT_MSG@27 PS4, Line 27: graiend Nit: "grained" http://gerrit.cloudera.org:8080/#/c/20110/4//COMMIT_MSG@33 PS4, Line 33: improting Nit: "importing" http://gerrit.cloudera.org:8080/#/c/20110/4/fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java File fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java: http://gerrit.cloudera.org:8080/#/c/20110/4/fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java@443 PS4, Line 443: } Should we have an else block which throws so we can find missing (possibly future) options? http://gerrit.cloudera.org:8080/#/c/20110/4/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java File fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java: http://gerrit.cloudera.org:8080/#/c/20110/4/fe/src/main/java/org/apache/impala/catalog/local/LocalFsTable.java@330 PS4, Line 330: public THdfsTable toTHdfsTable(Set<Long> referencedPartitions, ThriftObjectType type) { Could be private? http://gerrit.cloudera.org:8080/#/c/20110/4/tests/metadata/test_testcase_builder.py File tests/metadata/test_testcase_builder.py: http://gerrit.cloudera.org:8080/#/c/20110/4/tests/metadata/test_testcase_builder.py@57 PS4, Line 57: testcase_generate_query = "COPY TESTCASE TO '%s' %s" % (tmp_path, query) I think now that the testcase includes a table, this will cover the case that was not working (when we run in local catalog mode), is that right? -- To view, visit http://gerrit.cloudera.org:8080/20110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I02c1c76d7af15f28bdbc8d98d92a1553570e9e27 Gerrit-Change-Number: 20110 Gerrit-PatchSet: 4 Gerrit-Owner: Quanlong Huang <[email protected]> Gerrit-Reviewer: Andrew Sherman <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Kurt Deschler <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Comment-Date: Tue, 01 Aug 2023 01:12:47 +0000 Gerrit-HasComments: Yes
