Arnab Karmakar 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 4:

(7 comments)

Thanks for pointing out the gaps!

http://gerrit.cloudera.org:8080/#/c/23816/3/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/3/fe/src/main/java/org/apache/impala/catalog/Catalog.java@673
PS3, Line 673:  HDFS table
> We don't check the number of partition columns here. Please add the check f
Thanks for finding this bug, I dont know how I missed this. Done


http://gerrit.cloudera.org:8080/#/c/23816/3/fe/src/main/java/org/apache/impala/catalog/Catalog.java@1006
PS3, Line 1006:
> Please move this static util method to FeCatalogUtils since there are other
Moved. New tests have been added in CatalogTest.java


http://gerrit.cloudera.org:8080/#/c/23816/3/fe/src/main/java/org/apache/impala/catalog/Catalog.java@1013
PS3, Line 1013:
> How do we handle the case when the partition values have "/" in the string?
If partition values contain '/', they should be URL-encoded in the request. For 
example, if a partition value is "12/25/2025", the URL should encode it as 
"ds=12%2F25%2F2025".


http://gerrit.cloudera.org:8080/#/c/23816/3/fe/src/test/java/org/apache/impala/catalog/CatalogTest.java
File fe/src/test/java/org/apache/impala/catalog/CatalogTest.java:

http://gerrit.cloudera.org:8080/#/c/23816/3/fe/src/test/java/org/apache/impala/catalog/CatalogTest.java@1072
PS3, Line 1072: Test valid parti
> nit: we usually add JIRA id if this is added for a bug fix. For new feature
Done


http://gerrit.cloudera.org:8080/#/c/23816/3/fe/src/test/java/org/apache/impala/catalog/CatalogTest.java@1075
PS3, Line 1075: testGetValidPartitions() throws CatalogExcept
> nit: use shorter names like testGetValidPartitions, testGetNonExistingParti
Done


http://gerrit.cloudera.org:8080/#/c/23816/3/fe/src/test/java/org/apache/impala/catalog/CatalogTest.java@1158
PS3, Line 1158:     catalog_.getOrLoadTable("functional", "alltypes", "test", 
null);
> The test codes are simple and similar. Can we refactor some of them into on
Done


http://gerrit.cloudera.org:8080/#/c/23816/3/tests/webserver/test_web_pages.py
File tests/webserver/test_web_pages.py:

http://gerrit.cloudera.org:8080/#/c/23816/3/tests/webserver/test_web_pages.py@463
PS3, Line 463:     multi_part_insert_query = "insert into {0}.foo_multi_part 
partition " \
> Can we add tests for multi-key partition names and partition values that ha
Done



--
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: 4
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: Mon, 12 Jan 2026 19:31:07 +0000
Gerrit-HasComments: Yes

Reply via email to