Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/23816 )
Change subject: IMPALA-9935: Support individual partitions in catalog_object webUI page ...................................................................... Patch Set 7: (9 comments) http://gerrit.cloudera.org:8080/#/c/23816/7//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/23816/7//COMMIT_MSG@15 PS7, Line 15: for HDFS_PARTITION objects. Please add some URL examples. http://gerrit.cloudera.org:8080/#/c/23816/7/fe/src/main/java/org/apache/impala/catalog/Catalog.java File fe/src/main/java/org/apache/impala/catalog/Catalog.java: http://gerrit.cloudera.org:8080/#/c/23816/7/fe/src/main/java/org/apache/impala/catalog/Catalog.java@661 PS7, Line 661: REFRESH nit: DESCRIBE is a better approach since REFRESH triggers metadata loading and then reload the metadata again. http://gerrit.cloudera.org:8080/#/c/23816/7/fe/src/main/java/org/apache/impala/catalog/Catalog.java@696 PS7, Line 696: } catch (NullPointerException e) { : // getPartitionsForNames() throws NPE if partition doesn't exist : partition = null; We can add a simpler method, HdfsTable#getPartitionByName(), to use nameToPartitionMap_ and allow returning null. http://gerrit.cloudera.org:8080/#/c/23816/7/fe/src/main/java/org/apache/impala/catalog/Catalog.java@712 PS7, Line 712: tPartition.setPartition_name(partitionName); This is already set in FeCatalogUtils#fsPartitionToThrift(): https://github.com/apache/impala/blob/3e29f8aaa8f1a02630505866de45a918918aa216/fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java#L320 http://gerrit.cloudera.org:8080/#/c/23816/7/tests/webserver/test_web_pages.py File tests/webserver/test_web_pages.py: http://gerrit.cloudera.org:8080/#/c/23816/7/tests/webserver/test_web_pages.py@487 PS7, Line 487: ds=2024%2F12%2F25 I'm confused that if I change this to "ds=2024/12/25", the test still pass. But if I do the same for L489, the json test will fail. Could you take a look? http://gerrit.cloudera.org:8080/#/c/23816/7/tests/webserver/test_web_pages.py@573 PS7, Line 573: encoded_object_name = urllib.parse.quote(object_name, safe='') Doesn't this also encode the slash '/' in "year=2024/month=12/day=25"? http://gerrit.cloudera.org:8080/#/c/23816/7/tests/webserver/test_web_pages.py@578 PS7, Line 578: refresh nit: using "describe" is more lightweight. http://gerrit.cloudera.org:8080/#/c/23816/7/tests/webserver/test_web_pages.py@606 PS7, Line 606: refresh nit: using "describe" is more lightweight. http://gerrit.cloudera.org:8080/#/c/23816/7/tests/webserver/test_web_pages.py@610 PS7, Line 610: "json_string" nit: assert that the key exists -- To view, visit http://gerrit.cloudera.org:8080/23816 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5645a20283e664af12d04a9665c8870c7666a74c Gerrit-Change-Number: 23816 Gerrit-PatchSet: 7 Gerrit-Owner: Arnab Karmakar <[email protected]> Gerrit-Reviewer: Arnab Karmakar <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Pranav Lodha <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Reviewer: Steve Carlin <[email protected]> Gerrit-Comment-Date: Tue, 20 Jan 2026 02:32:12 +0000 Gerrit-HasComments: Yes
