Noemi Pap-Takacs has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/19412 )

Change subject: IMPALA-11821: Adjusting manifest_length in case of metadata 
rewrite
......................................................................


Patch Set 2:

(2 comments)

Looks nice. I added some suggestions so that this script could handle tables 
created by Hive as well.
That would solve IMPALA-11807.

http://gerrit.cloudera.org:8080/#/c/19412/2/testdata/bin/rewrite-iceberg-metadata.py
File testdata/bin/rewrite-iceberg-metadata.py:

http://gerrit.cloudera.org:8080/#/c/19412/2/testdata/bin/rewrite-iceberg-metadata.py@69
PS2, Line 69:   for mfile in glob.glob(os.path.join(arg, 'v*.metadata.json')):
I think we should remove 'v' from the regex to also match metadata.json files 
created by Hive.


http://gerrit.cloudera.org:8080/#/c/19412/2/testdata/bin/rewrite-iceberg-metadata.py@83
PS2, Line 83:     metadata['location'] = prefix + metadata['location']
Could you please add support for absolute paths?
Iceberg creates metadata files with absolute paths. We used to rewrite metadata 
files manually to have relative paths instead. This script currently only adds 
filesystem prefixes to relative paths. It would be nice if it could handle 
changing absolute paths as well: removing the old prefix (everything before 
'/test-warehouse/iceberg_test') and adding the new one. Then we would not need 
to manually rewrite metadata and we could create test tables using Hive from 
Impala directly.



--
To view, visit http://gerrit.cloudera.org:8080/19412
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I89b9208f25552012cc1ab16fa60a819dd5a683d9
Gerrit-Change-Number: 19412
Gerrit-PatchSet: 2
Gerrit-Owner: Gergely Fürnstáhl <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Reviewer: Noemi Pap-Takacs <[email protected]>
Gerrit-Comment-Date: Sun, 15 Jan 2023 20:32:18 +0000
Gerrit-HasComments: Yes

Reply via email to