Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/24927 )
Change subject: IMPALA-15409: Fix file paths in OPTIMIZE and replace deprecated ContentFile.path() ...................................................................... Patch Set 3: Code-Review+1 (4 comments) Thanks for working on this! http://gerrit.cloudera.org:8080/#/c/24927/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/24927/3//COMMIT_MSG@13 PS3, Line 13: , nit: and other special characters http://gerrit.cloudera.org:8080/#/c/24927/3/fe/src/main/java/org/apache/impala/analysis/OptimizeStmt.java File fe/src/main/java/org/apache/impala/analysis/OptimizeStmt.java: http://gerrit.cloudera.org:8080/#/c/24927/3/fe/src/main/java/org/apache/impala/analysis/OptimizeStmt.java@245 PS3, Line 245: Preconditions.checkState( : selectedFileDescriptors.size() == selectedDataFiles.size(), : "Missing file descriptor for a data file selected by OPTIMIZE"); Can we provide information about the missing files? Like we do at https://github.com/apache/impala/blob/d757d3dfe5c7cdcc48aed2064ed28bbed3ef9ffe/fe/src/main/java/org/apache/impala/planner/IcebergScanPlanner.java#L252 http://gerrit.cloudera.org:8080/#/c/24927/3/fe/src/test/java/org/apache/impala/util/IcebergFileFilterTest.java File fe/src/test/java/org/apache/impala/util/IcebergFileFilterTest.java: http://gerrit.cloudera.org:8080/#/c/24927/3/fe/src/test/java/org/apache/impala/util/IcebergFileFilterTest.java@183 PS3, Line 183: public void testEscapedPartitionPaths() { Does it fail without the fix? If not, we can drop this. http://gerrit.cloudera.org:8080/#/c/24927/3/tests/query_test/test_iceberg.py File tests/query_test/test_iceberg.py: http://gerrit.cloudera.org:8080/#/c/24927/3/tests/query_test/test_iceberg.py@2317 PS3, Line 2317: 3, '11"14"31') Can you add more files to it, so it also gets rewritten? And more plain partition values as well? I know you want to test the PARTIAL case, so a partition with a single file will still be needed. -- To view, visit http://gerrit.cloudera.org:8080/24927 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia6df5387f520acd556468ee888c127d99f8fc00d Gerrit-Change-Number: 24927 Gerrit-PatchSet: 3 Gerrit-Owner: Noemi Pap-Takacs <[email protected]> Gerrit-Reviewer: Daniel Vanko <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Peter Rozsa <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Thu, 24 Sep 2026 09:09:58 +0000 Gerrit-HasComments: Yes
