Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/24979 )
Change subject: IMPALA-13704: Support OPTIMIZE TABLE for selected partitions ...................................................................... Patch Set 1: (10 comments) Some high level comments about the features + tests http://gerrit.cloudera.org:8080/#/c/24979/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/24979/1//COMMIT_MSG@11 PS1, Line 11: expr_list Is it possible to have a more complex expression, e.g. i>1? I only saw = in tests. I saw year(ts) ParserTests, but no actual execution. http://gerrit.cloudera.org:8080/#/c/24979/1//COMMIT_MSG@14 PS1, Line 14: other partitions are left untouched. Can you add note about delete files here? A delete file can only belong to one partition, so removing it for the optimized partition doesn't interfere with other partitions, right? http://gerrit.cloudera.org:8080/#/c/24979/1//COMMIT_MSG@16 PS1, Line 16: Details: Can you describe how partition evolution is handled? Can you optimize a partition that is not according to the latest spec? What will be done with rows in old spec that match the filter for the new spec? http://gerrit.cloudera.org:8080/#/c/24979/1//COMMIT_MSG@36 PS1, Line 36: Note that nit: weird line break http://gerrit.cloudera.org:8080/#/c/24979/1//COMMIT_MSG@37 PS1, Line 37: TIcebergOperationParam has no mode field, so a new coordinator against : an older catalogd applies the old semantics to files_to_replace and : still rewrites the whole table. Is this setup really relevant? http://gerrit.cloudera.org:8080/#/c/24979/1/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-optimize.test File testdata/workloads/functional-planner/queries/PlannerTest/iceberg-optimize.test: http://gerrit.cloudera.org:8080/#/c/24979/1/testdata/workloads/functional-planner/queries/PlannerTest/iceberg-optimize.test@161 PS1, Line 161: 03:SORT : | order by: action ASC NULLS LAST : | mem-estimate=12.00MB mem-reservation=12.00MB spill-buffer=2.00MB thread-reservation=0 : | tuple-ids=4 row-size=44B cardinality=3 : | in pipelines: 03(GETNEXT), 00(OPEN) Why do we have a sort in this case? http://gerrit.cloudera.org:8080/#/c/24979/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/24979/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-optimize.test@326 PS1, Line 326: PARTITIONED BY SPEC(i) Does optimize work for other partitioned spec than identity? http://gerrit.cloudera.org:8080/#/c/24979/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-optimize.test@382 PS1, Line 382: SET MAX_FS_WRITERS=1; Why does MAX_FS_WRITERS matter here? The optimize won't add shuffle to create a partition with a single writer otherwise? AFAIK this is a change compared to OPTIMIZE without partition and could be noted in the commit message. http://gerrit.cloudera.org:8080/#/c/24979/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-optimize.test@397 PS1, Line 397: ORDER BY i, s; here and at other places: order by is not needed we should add this to claude.md, it is a very common mistake http://gerrit.cloudera.org:8080/#/c/24979/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-optimize.test@408 PS1, Line 408: FILE_SIZE_THRESHOLD_MB Is there any test where FILE_SIZE_THRESHOLD_MB actually leads to keeping a file? Btw is FILE_SIZE_THRESHOLD_MB an int, or it can be less than 1 to express limits like "compact below 512KB"? -- To view, visit http://gerrit.cloudera.org:8080/24979 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I468f0bf5309ddce67047e3e5e52e60304967af10 Gerrit-Change-Number: 24979 Gerrit-PatchSet: 1 Gerrit-Owner: Noemi Pap-Takacs <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[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, 01 Oct 2026 08:54:01 +0000 Gerrit-HasComments: Yes
