Daniel Vanko has posted comments on this change. ( http://gerrit.cloudera.org:8080/24633 )
Change subject: IMPALA-15222: Short-circuit OPTIMIZE when the table is compact ...................................................................... Patch Set 1: (8 comments) http://gerrit.cloudera.org:8080/#/c/24633/1/common/thrift/Query.thrift File common/thrift/Query.thrift: http://gerrit.cloudera.org:8080/#/c/24633/1/common/thrift/Query.thrift@1037 PS1, Line 1037: NO_OP Good catch for consistency! (and bad from me in IMPALA-14536...) http://gerrit.cloudera.org:8080/#/c/24633/1/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/24633/1/fe/src/main/java/org/apache/impala/analysis/OptimizeStmt.java@318 PS1, Line 318: "No files selected for optimization." How about "The table is already optimized." instead? http://gerrit.cloudera.org:8080/#/c/24633/1/fe/src/main/java/org/apache/impala/service/Frontend.java File fe/src/main/java/org/apache/impala/service/Frontend.java: http://gerrit.cloudera.org:8080/#/c/24633/1/fe/src/main/java/org/apache/impala/service/Frontend.java@3125 PS1, Line 3125: } else if (analysisResult.getStmt().isNoOp()) { I'm wondering how this move will affect EXPLAIN statements. But I guess it would be okay, since only CONVERT TO ICEBERG and OPTIMIZE use the noOp field. http://gerrit.cloudera.org:8080/#/c/24633/1/fe/src/main/java/org/apache/impala/service/Frontend.java@3184 PS1, Line 3184: result.setStmt_type(TStmtType.CONVERT); : fix indentation http://gerrit.cloudera.org:8080/#/c/24633/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-optimize.test File testdata/workloads/functional-query/queries/QueryTest/iceberg-optimize.test: http://gerrit.cloudera.org:8080/#/c/24633/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-optimize.test@11 PS1, Line 11: DELETE FROM ice_optimize_part WHERE bool_col=true; Could you add an OPTIMIZE after the insert and before the delete? To check if it's a no-op for one file as well. http://gerrit.cloudera.org:8080/#/c/24633/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-optimize.test@46 PS1, Line 46: 'No files selected for optimization.' Can you mirror this part to the non-partitioned table (lines 87-110)? http://gerrit.cloudera.org:8080/#/c/24633/1/tests/query_test/test_iceberg.py File tests/query_test/test_iceberg.py: http://gerrit.cloudera.org:8080/#/c/24633/1/tests/query_test/test_iceberg.py@2219 PS1, Line 2219: NOOP nit: rename for consistency http://gerrit.cloudera.org:8080/#/c/24633/1/tests/query_test/test_iceberg.py@2221 PS1, Line 2221: assert files_after == files_before Can we have an assert here regarding the summary message? -- To view, visit http://gerrit.cloudera.org:8080/24633 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I795967ca18231ef1fefdbc71a9b4505905906f95 Gerrit-Change-Number: 24633 Gerrit-PatchSet: 1 Gerrit-Owner: Noemi Pap-Takacs <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Daniel Vanko <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Mihaly Szjatinya <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Tue, 28 Jul 2026 13:16:54 +0000 Gerrit-HasComments: Yes
