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 8:

(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.
Done


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: DESCRIB
> nit: DESCRIBE is a better approach since REFRESH triggers metadata loading
Done


http://gerrit.cloudera.org:8080/#/c/23816/7/fe/src/main/java/org/apache/impala/catalog/Catalog.java@696
PS7, Line 696:           // Partitions use the table's catalog version
             :           result.setCatalog_version(table.getCatalogVersion());
             :           result.setLast_modi
> We can add a simpler method, HdfsTable#getPartitionByName(), to use nameToP
Done


http://gerrit.cloudera.org:8080/#/c/23816/7/fe/src/main/java/org/apache/impala/catalog/Catalog.java@712
PS7, Line 712:         Function fn = getFunction(desc, 
Function.CompareMode.IS_INDISTINGUISHABLE);
> This is already set in FeCatalogUtils#fsPartitionToThrift():
Done


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:  test methods wil
> I'm confused that if I change this to "ds=2024/12/25", the test still pass.
Im including all the explanation in this response that I found after some 
debugging:

The HTML test passed because html endpoint response (on error):
```<div class="panel-body">  CatalogException: Invalid partition name 
&apos;ds=2024/12/25&apos;: expected 1 partition keys, got 3</div>```

The test does:
# Line 585-586 self.get_and_check_status(obj_url, partition_name, 
ports_to_test=self.CATALOG_TEST_PORT)

get_and_check_status implementation:
assert string_to_search in response.text  # Line 295

Result:
Searches for "ds=2024/12/25" in html response and it finds it in the error 
message that contains this exact string: "Invalid partition name 
'ds=2024/12/25'"
It doesn't care if it's in an error or success message

The JSON test failed becaues json endpoint response (on error):
```{  "error": "CatalogException: Invalid partition name 'ds=2024/12/25': 
expected 1 partition keys, got 3\n",  "__common__": {...}}```
There's no "json_string" field and only "error" field is present.


http://gerrit.cloudera.org:8080/#/c/23816/7/tests/webserver/test_web_pages.py@573
PS7, Line 573:     # URL encode the entire object name (db.table:partition).
> Doesn't this also encode the slash '/' in "year=2024/month=12/day=25"?
Yes it does. safe='' means encode ALL special characters including '/', ':', 
and '='.

For regular partitions (no special chars in values), single encoding works. eg: 
year=2024/month=12 -> URL: year%3D2024%2Fmonth%3D12
But for partitions containing '/' we need double encoding. eg: 
ds=2024%2F12%2F25 -> URL: ds%3D2024%252F12%252F25

Ive modified the docstrings in FeCatalogUtils.java and catalog-util.cc


http://gerrit.cloudera.org:8080/#/c/23816/7/tests/webserver/test_web_pages.py@578
PS7, Line 578: S_PARTI
> nit: using "describe" is more lightweight.
Done


http://gerrit.cloudera.org:8080/#/c/23816/7/tests/webserver/test_web_pages.py@606
PS7, Line 606: G_OBJEC
> nit: using "describe" is more lightweight.
Done


http://gerrit.cloudera.org:8080/#/c/23816/7/tests/webserver/test_web_pages.py@610
PS7, Line 610: , tbl_name))
> nit: assert that the key exists
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: 8
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 09:04:38 +0000
Gerrit-HasComments: Yes

Reply via email to