You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Mihaly Szjatinya (Code Review)" <ge...@cloudera.org> on 2025/02/11 23:00:50 UTC
[Impala-ASF-CR] WIP IMPALA-11597: DROP STATS should unset table property impala.lastComputeStatsTime
Mihaly Szjatinya has uploaded this change for review. ( http://gerrit.cloudera.org:8080/22474
Change subject: WIP IMPALA-11597: DROP STATS should unset table property impala.lastComputeStatsTime
......................................................................
WIP IMPALA-11597: DROP STATS should unset table property
impala.lastComputeStatsTime
Removes 'impala.lastComputeStatsTime' property from table on DROP STATS
catalog operation. Does not affect the incremental variant:
'DROP INCREMENTAL STATS t PARTITION (partition_spec)'
Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-compute-stats.test
2 files changed, 3 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/74/22474/1
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 1
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
[Impala-ASF-CR] IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/22474 )
Change subject: IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/22474/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/22474/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2820
PS3, Line 2820: InProgressTableModification modification =
: new InProgressTableModification(catalog_, table);
: org.apache.iceberg.Transaction iceTxn =
: IcebergUtil.getIcebergTransaction((FeIcebergTable)table);
: I
> Why impala.lastComputeStatsTime is dropped, and the two others are not, for
This is not addressed yet.
What happen if user manually set TBL_PROP_LAST_COMPUTE_STATS_TIME, ROW_COUNT, and TOTAL_SIZE property to an iceberg table using ALTER TABLE SET TBLPROPERTIES query?
Running DROP STATS will only drop TBL_PROP_LAST_COMPUTE_STATS_TIME and leave ROW_COUNT and TOTAL_SIZE intact?
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 4
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 07 Mar 2025 13:43:41 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Posted by "Mihaly Szjatinya (Code Review)" <ge...@cloudera.org>.
Mihaly Szjatinya has posted comments on this change. ( http://gerrit.cloudera.org:8080/22474 )
Change subject: IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/22474/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/22474/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2820
PS3, Line 2820: InProgressTableModification modification =
: new InProgressTableModification(catalog_, table);
: org.apache.iceberg.Transaction iceTxn =
: IcebergUtil.getIcebergTransaction((FeIcebergTable)table);
: I
> This is not addressed yet.
Correct, the behavior for ROW_COUNT and TOTAL_SIZE does not change.
Without much experience with Iceberg tables, my vision is that there is a reason why those aren't removed for this table type and that there is some parallel mechanism on Iceberg's side to handle those.
As far as TBL_PROP_LAST_COMPUTE_STATS_TIME it is not really a statistic value itself so it doesn't necessarily have to be handled in the same way.
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 4
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 07 Mar 2025 14:28:37 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22474 )
Change subject: IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
......................................................................
Patch Set 6:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/18510/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 6
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 07 Mar 2025 18:53:49 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] WIP IMPALA-11597: DROP STATS should unset table property impala.lastComputeStatsTime
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/22474 )
Change subject: WIP IMPALA-11597: DROP STATS should unset table property impala.lastComputeStatsTime
......................................................................
Patch Set 1:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/22474/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/22474/1//COMMIT_MSG@7
PS1, Line 7: WIP IMPALA-11597: DROP STATS should unset table property
: impala.lastComputeStatsTime
Title can be shorter. For example:
IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
http://gerrit.cloudera.org:8080/#/c/22474/1//COMMIT_MSG@11
PS1, Line 11: Does not affect the incremental variant:
: 'DROP INCREMENTAL STATS t PARTITION (partition_spec)'
Is there any test that validate this statement?
http://gerrit.cloudera.org:8080/#/c/22474/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/22474/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2824
PS1, Line 2824: boolean droppedLastCompute =
: msTbl.getParameters().remove(HdfsTable.TBL_PROP_LAST_COMPUTE_STATS_TIME) != null;
> You mean in terms of why this issue is needed? That's probably better to di
What I'm asking is whether there is any other Catalog operation that depends on existence of 'impala.lastComputeStatsTime'.
For example, what will happen in https://github.com/apache/impala/blob/c9e9f86c0536f0991f70613675bbcfbbccc6a324/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java#L2036C43-L2036C61 (IMPALA-11583)? Is it still work as intended?
http://gerrit.cloudera.org:8080/#/c/22474/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-compute-stats.test
File testdata/workloads/functional-query/queries/QueryTest/iceberg-compute-stats.test:
http://gerrit.cloudera.org:8080/#/c/22474/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-compute-stats.test@266
PS1, Line 266: '','numRows ','2
Add validation that 'totalSize' is also gone.
Maybe just copy RESULTS section of L248, but change it from VERIFY_IS_SUBSET to VERIFY_IS_NOT_IN
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 1
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Wed, 12 Feb 2025 23:27:26 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] WIP IMPALA-11597: DROP STATS should unset table property impala.lastComputeStatsTime
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22474 )
Change subject: WIP IMPALA-11597: DROP STATS should unset table property impala.lastComputeStatsTime
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/18264/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 1
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 11 Feb 2025 23:26:41 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] WIP IMPALA-11597: DROP STATS should unset table property impala.lastComputeStatsTime
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/22474 )
Change subject: WIP IMPALA-11597: DROP STATS should unset table property impala.lastComputeStatsTime
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/22474/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/22474/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2824
PS1, Line 2824: boolean droppedLastCompute =
: msTbl.getParameters().remove(HdfsTable.TBL_PROP_LAST_COMPUTE_STATS_TIME) != null;
Looks like there is a detail missing here that is not explained.
Why it matter to drop this property now? What other operation relies on value of this property?
'impala.lastComputeStatsTime' should be informational only and not critical, but maybe something changed recently that makes this important.
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 1
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Wed, 12 Feb 2025 17:14:34 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22474 )
Change subject: IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
......................................................................
Patch Set 7:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/18511/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 7
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 07 Mar 2025 18:53:23 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Posted by "Mihaly Szjatinya (Code Review)" <ge...@cloudera.org>.
Mihaly Szjatinya has posted comments on this change. ( http://gerrit.cloudera.org:8080/22474 )
Change subject: IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/22474/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/22474/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2823
PS3, Line 2823: IcebergUtil.getIcebergTransaction((FeIcebergTable)t
> We should never directly modify the msTbl of integrated Iceberg tables, we
I see, pls check the new patch.
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 4
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 07 Mar 2025 13:36:29 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] WIP IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Posted by "Mihaly Szjatinya (Code Review)" <ge...@cloudera.org>.
Mihaly Szjatinya has posted comments on this change. ( http://gerrit.cloudera.org:8080/22474 )
Change subject: WIP IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
......................................................................
Patch Set 2:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/22474/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/22474/1//COMMIT_MSG@7
PS1, Line 7: WIP IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
:
> Title can be shorter. For example:
Ack
http://gerrit.cloudera.org:8080/#/c/22474/1//COMMIT_MSG@11
PS1, Line 11: TATS t PARTITION (partition_spec)'
:
> Is there any test that validate this statement?
I haven't found one. We can't do it here since iceberg does not support 'incremental' variants.
There is also 'compute-stats-*.test' family of tests, among which DROP INCREMENTAL STATS is tested, but I don't see test cases for any 'impala.lastComputeStatsTime' scenario there. So it's probably non consistent to add a test just for this scenario alone. Especially given that we do not change the previous behavior here.
http://gerrit.cloudera.org:8080/#/c/22474/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/22474/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2824
PS1, Line 2824: boolean droppedLastCompute =
: msTbl.getParameters().remove(HdfsTable.TBL_PROP_LAST_COMPUTE_STATS_TIME) != null;
> Will 'impala.lastComputeStatsTime' removal raise HMS event? If yes, what wi
Seems like it doesn't in this case. There is 'TestEventProcessingWithImpala.test_self_events_with_impala' which was falling for one of my previous patches and passes for this code.
But I haven't looked really deeply into the whole events mechanism yet, let me know if you think this is still error-prone.
http://gerrit.cloudera.org:8080/#/c/22474/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-compute-stats.test
File testdata/workloads/functional-query/queries/QueryTest/iceberg-compute-stats.test:
http://gerrit.cloudera.org:8080/#/c/22474/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-compute-stats.test@266
PS1, Line 266: '','numRows ','2
> Add validation that 'totalSize' is also gone.
Ok, not directly related, but can do.
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 2
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Feb 2025 13:47:34 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/22474 )
Change subject: IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
......................................................................
Patch Set 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/22474/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/22474/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2820
PS3, Line 2820: boolean droppedLastCompute =
: msTbl.getParameters().remove(HdfsTable.TBL_PROP_LAST_COMPUTE_STATS_TIME) != null;
: if (droppedLastCompute) {
: applyAlterTable(msTbl, false, null, catalogTimeline);
: }
> Correct, the behavior for ROW_COUNT and TOTAL_SIZE does not change.
Yeah, I think it's OK to keep ROW_COUNT and TOTAL_SIZE untouched as they are managed by Iceberg and recreated for each snapshot.
http://gerrit.cloudera.org:8080/#/c/22474/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/22474/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2820
PS4, Line 2820: boolean droppedLastCompute =
: msTbl.getParameters().remove(HdfsTable.TBL_PROP_LAS
I think we also need:
modification.addCatalogServiceIdentifiersToTable();
modification.registerInflightEvent();
http://gerrit.cloudera.org:8080/#/c/22474/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3540
PS4, Line 3540: if (isIcebergHmsIntegrationEnabled(iceTbl.getMetaStoreTable())) {
: modification.registerInflightEvent();
: IcebergCatalogOpExecutor.addCatalogVersionToTxn(
: iceTxn, catalog_.getCatalogServiceId(), modification.newVersionNumber());
: }
: iceTxn.commitTransaction();
: modification.markInflightEventRegistrationComplete();
: modification.validateInProgressModificationComplete();
: } catch (ImpalaException ex) {
: modification.cancelInflight
Here in truncateIcebergTable() it would be better to use the same iceTxn and modification object to drop table stats of integrated Iceberg tables.
I think it should be doable if we create a new method: dropIcebergTableStats(), and dropTableStats() wouldn't do anything for integrated Iceberg tables.
And in dropStats() we could also invoke dropIcebergTableStats() for integrated Iceberg tables.
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 3
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 07 Mar 2025 15:20:59 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22474 )
Change subject: IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/18497/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 3
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 06 Mar 2025 20:41:36 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Posted by "Mihaly Szjatinya (Code Review)" <ge...@cloudera.org>.
Mihaly Szjatinya has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/22474 )
Change subject: IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
......................................................................
IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Removes 'impala.lastComputeStatsTime' property from table on DROP STATS
catalog operation. Does not affect the incremental variant:
'DROP INCREMENTAL STATS t PARTITION (partition_spec)'
Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-compute-stats.test
2 files changed, 37 insertions(+), 25 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/74/22474/7
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 7
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] WIP IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/22474 )
Change subject: WIP IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/22474/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/22474/2//COMMIT_MSG@7
PS2, Line 7: WIP
Is it still WIP?
http://gerrit.cloudera.org:8080/#/c/22474/2/testdata/workloads/functional-query/queries/QueryTest/iceberg-compute-stats.test
File testdata/workloads/functional-query/queries/QueryTest/iceberg-compute-stats.test:
http://gerrit.cloudera.org:8080/#/c/22474/2/testdata/workloads/functional-query/queries/QueryTest/iceberg-compute-stats.test@77
PS2, Line 77: ---- QUERY
: alter table ice_alltypes unset tblproperties('impala.lastComputeStatsTime');
: ---- RESULTS
: 'Updated table.'
: ---- TYPES
: STRING
: ====
Do you also want to resolve this for Iceberg tables in HiveCatalog?
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 2
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 03 Mar 2025 16:28:53 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22474 )
Change subject: IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
......................................................................
Patch Set 9: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 9
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 14 Mar 2025 12:44:55 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Posted by "Mihaly Szjatinya (Code Review)" <ge...@cloudera.org>.
Mihaly Szjatinya has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/22474 )
Change subject: IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
......................................................................
IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Removes 'impala.lastComputeStatsTime' property from table on DROP STATS
catalog operation. Does not affect the incremental variant:
'DROP INCREMENTAL STATS t PARTITION (partition_spec)'
Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-compute-stats.test
2 files changed, 15 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/74/22474/5
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 5
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Posted by "Mihaly Szjatinya (Code Review)" <ge...@cloudera.org>.
Mihaly Szjatinya has posted comments on this change. ( http://gerrit.cloudera.org:8080/22474 )
Change subject: IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
......................................................................
Patch Set 7:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/22474/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/22474/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2820
PS4, Line 2820: /**
: * Drops all table and partition stats from this table in
> I think we also need:
Ack
http://gerrit.cloudera.org:8080/#/c/22474/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3540
PS4, Line 3540: Preconditions.checkState(table.isWriteLockedByCurrentThread());
: Preconditions.checkState(catalog_.getLock().isWriteLockedByCurrentThread());
: Preconditions.checkState(table instanceof FeIcebergTable);
: InProgressTableModification modification =
: new InProgressTableModification(catalog_, table);
: catalog_.getLock().writeLock().unlock();
: modification.addCatalogServiceIdentifiersToTable();
: try {
: FeIcebergTable iceTbl = (FeIcebergTable) table;
: org.apache.iceberg.Transact
> Here in truncateIcebergTable() it would be better to use the same iceTxn an
Makes sense. Pls check the newest patch, pls correct it if I'm misusing modification or txn.
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 7
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 07 Mar 2025 18:27:08 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Posted by "Mihaly Szjatinya (Code Review)" <ge...@cloudera.org>.
Mihaly Szjatinya has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/22474 )
Change subject: IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
......................................................................
IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Removes 'impala.lastComputeStatsTime' property from table on DROP STATS
catalog operation. Does not affect the incremental variant:
'DROP INCREMENTAL STATS t PARTITION (partition_spec)'
Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-compute-stats.test
2 files changed, 36 insertions(+), 25 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/74/22474/6
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 6
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/22474 )
Change subject: IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
......................................................................
Patch Set 7:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/22474/7/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/22474/7/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2742
PS7, Line 2742: if (isIcebergHmsIntegrationEnabled(table.getMetaStoreTable())) {
This should be always true because of L2737
http://gerrit.cloudera.org:8080/#/c/22474/7/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2743
PS7, Line 2743: modification.registerInflightEvent();
This is already called at L2729.
http://gerrit.cloudera.org:8080/#/c/22474/7/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2830
PS7, Line 2830: org.apache.hadoop.hive.metastore.api.Table msTbl = table.getMetaStoreTable();
Can we have a Precondition check that the table is not an integrated Iceberg table?
http://gerrit.cloudera.org:8080/#/c/22474/7/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2890
PS7, Line 2890: EventSequence catalogTimeline
Unused parameter
http://gerrit.cloudera.org:8080/#/c/22474/7/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2891
PS7, Line 2891: modification.addCatalogServiceIdentifiersToTable();
: modification.registerInflightEvent();
I think these are redundant, as these are also called by the callers.
http://gerrit.cloudera.org:8080/#/c/22474/7/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3555
PS7, Line 3555: dropIcebergTableStats
IIUC, now when we issue TRUNCATE TABLE on a non-integrated Iceberg table (i.e. an Iceberg table in HadoopTables or HadoopCatalog), the table stats won't be cleared from HMS. For such tables dropTableStats is necessary.
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 7
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 07 Mar 2025 20:41:35 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Posted by "Mihaly Szjatinya (Code Review)" <ge...@cloudera.org>.
Mihaly Szjatinya has posted comments on this change. ( http://gerrit.cloudera.org:8080/22474 )
Change subject: IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
......................................................................
Patch Set 8:
(6 comments)
Thanks for corrections, pls check.
http://gerrit.cloudera.org:8080/#/c/22474/7/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/22474/7/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2742
PS7, Line 2742: iceTxn, catalog_.getCatalogServiceId(), modification.newVersionNumber());
> This should be always true because of L2737
Fixed
http://gerrit.cloudera.org:8080/#/c/22474/7/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2743
PS7, Line 2743: eTxn.commitTransaction();
> This is already called at L2729.
Fixed
http://gerrit.cloudera.org:8080/#/c/22474/7/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2830
PS7, Line 2830: int numTargetedPartitions = 0;
> Can we have a Precondition check that the table is not an integrated Iceber
Sure, also adding the opposite to dropIcebergTableStats.
http://gerrit.cloudera.org:8080/#/c/22474/7/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2890
PS7, Line 2890: olean isIntegratedIcebergTbl
> Unused parameter
Fixed
http://gerrit.cloudera.org:8080/#/c/22474/7/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2891
PS7, Line 2891: IcebergTable.isIcebergTable(msTbl) && isIcebergHmsIntegrationEnabled(msTbl);
: Preconditions.checkState(isIntegrated
> I think these are redundant, as these are also called by the callers.
Ok, in that case no need to pass modification. In fact even table isn't needed, but I'd keep it for precondition.
http://gerrit.cloudera.org:8080/#/c/22474/7/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3555
PS7, Line 3555: if (isIcebergHmsInteg
> IIUC, now when we issue TRUNCATE TABLE on a non-integrated Iceberg table (i
I see, we probably should also rename 'dropIcebergTableStats' to 'dropIntegratedIcebergTableStats'
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 8
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 07 Mar 2025 21:31:49 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22474 )
Change subject: IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
......................................................................
Patch Set 9:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/11622/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 9
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 14 Mar 2025 12:44:56 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22474 )
Change subject: IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
......................................................................
Patch Set 9: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 9
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 14 Mar 2025 18:06:38 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] WIP IMPALA-11597: DROP STATS should unset table property impala.lastComputeStatsTime
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/22474 )
Change subject: WIP IMPALA-11597: DROP STATS should unset table property impala.lastComputeStatsTime
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/22474/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/22474/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2824
PS1, Line 2824: boolean droppedLastCompute =
: msTbl.getParameters().remove(HdfsTable.TBL_PROP_LAST_COMPUTE_STATS_TIME) != null;
> What I'm asking is whether there is any other Catalog operation that depend
updateTableStatsViaIceberg() is invoked when a COMPUTE STATS is finalized and we need to set this property via Iceberg API, so that will work fine.
Also, at L2817 we jump out from this method when the Iceberg table is in a HiveCatalog.
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 1
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 13 Feb 2025 16:55:50 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22474 )
Change subject: IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/18507/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 5
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 07 Mar 2025 15:55:55 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22474 )
Change subject: IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
......................................................................
Patch Set 8:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/18512/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 8
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 07 Mar 2025 21:56:51 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] WIP IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Posted by "Mihaly Szjatinya (Code Review)" <ge...@cloudera.org>.
Mihaly Szjatinya has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/22474 )
Change subject: WIP IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
......................................................................
WIP IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Removes 'impala.lastComputeStatsTime' property from table on DROP STATS
catalog operation. Does not affect the incremental variant:
'DROP INCREMENTAL STATS t PARTITION (partition_spec)'
Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-compute-stats.test
2 files changed, 4 insertions(+), 8 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/74/22474/2
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 2
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] WIP IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22474 )
Change subject: WIP IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/18292/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 2
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Feb 2025 14:13:16 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] WIP IMPALA-11597: DROP STATS should unset table property impala.lastComputeStatsTime
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/22474 )
Change subject: WIP IMPALA-11597: DROP STATS should unset table property impala.lastComputeStatsTime
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/22474/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/22474/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2824
PS1, Line 2824: boolean droppedLastCompute =
: msTbl.getParameters().remove(HdfsTable.TBL_PROP_LAST_COMPUTE_STATS_TIME) != null;
> updateTableStatsViaIceberg() is invoked when a COMPUTE STATS is finalized a
Will 'impala.lastComputeStatsTime' removal raise HMS event? If yes, what will happen if such event raised?
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 1
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 13 Feb 2025 20:32:14 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Posted by "Mihaly Szjatinya (Code Review)" <ge...@cloudera.org>.
Mihaly Szjatinya has uploaded a new patch set (#8). ( http://gerrit.cloudera.org:8080/22474 )
Change subject: IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
......................................................................
IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Removes 'impala.lastComputeStatsTime' property from table on DROP STATS
catalog operation. Does not affect the incremental variant:
'DROP INCREMENTAL STATS t PARTITION (partition_spec)'
Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-compute-stats.test
2 files changed, 40 insertions(+), 24 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/74/22474/8
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 8
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/22474 )
Change subject: IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
......................................................................
IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Removes 'impala.lastComputeStatsTime' property from table on DROP STATS
catalog operation. Does not affect the incremental variant:
'DROP INCREMENTAL STATS t PARTITION (partition_spec)'
Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Reviewed-on: http://gerrit.cloudera.org:8080/22474
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-compute-stats.test
2 files changed, 40 insertions(+), 24 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 10
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/22474 )
Change subject: IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/22474/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/22474/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2820
PS3, Line 2820: boolean droppedLastCompute =
: msTbl.getParameters().remove(HdfsTable.TBL_PROP_LAST_COMPUTE_STATS_TIME) != null;
: if (droppedLastCompute) {
: applyAlterTable(msTbl, false, null, catalogTimeline);
: }
Why impala.lastComputeStatsTime is dropped, and the two others are not, for integrated iceberg table?
Please clarify in commit message.
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 3
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 06 Mar 2025 20:23:38 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Posted by "Mihaly Szjatinya (Code Review)" <ge...@cloudera.org>.
Mihaly Szjatinya has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/22474 )
Change subject: IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
......................................................................
IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Removes 'impala.lastComputeStatsTime' property from table on DROP STATS
catalog operation. Does not affect the incremental variant:
'DROP INCREMENTAL STATS t PARTITION (partition_spec)'
Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-compute-stats.test
2 files changed, 13 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/74/22474/4
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 4
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22474 )
Change subject: IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/18505/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 4
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 07 Mar 2025 14:02:42 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/22474 )
Change subject: IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
......................................................................
Patch Set 5: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/22474/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/22474/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2820
PS3, Line 2820: InProgressTableModification modification =
: new InProgressTableModification(catalog_, table);
: modification.addCatalogServiceIdentifiersToTable();
: modification.registerInflightEvent();
: o
> Yeah, I think it's OK to keep ROW_COUNT and TOTAL_SIZE untouched as they ar
Ack
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 5
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 07 Mar 2025 15:35:06 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] WIP IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/22474 )
Change subject: WIP IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
......................................................................
Patch Set 2: Code-Review+1
(4 comments)
http://gerrit.cloudera.org:8080/#/c/22474/1//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/22474/1//COMMIT_MSG@7
PS1, Line 7: WIP IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
:
> Ack
Done
http://gerrit.cloudera.org:8080/#/c/22474/1//COMMIT_MSG@11
PS1, Line 11: TATS t PARTITION (partition_spec)'
:
> I haven't found one. We can't do it here since iceberg does not support 'in
Ack
http://gerrit.cloudera.org:8080/#/c/22474/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/22474/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2824
PS1, Line 2824: boolean droppedLastCompute =
: msTbl.getParameters().remove(HdfsTable.TBL_PROP_LAST_COMPUTE_STATS_TIME) != null;
> Seems like it doesn't in this case. There is 'TestEventProcessingWithImpala
Ack
http://gerrit.cloudera.org:8080/#/c/22474/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-compute-stats.test
File testdata/workloads/functional-query/queries/QueryTest/iceberg-compute-stats.test:
http://gerrit.cloudera.org:8080/#/c/22474/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-compute-stats.test@266
PS1, Line 266: '','numRows ','2
> Ok, not directly related, but can do.
Done
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 2
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Mon, 03 Mar 2025 13:02:55 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/22474 )
Change subject: IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
......................................................................
Patch Set 8: Code-Review+2
Thanks for fixing this, LGTM!
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 8
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 14 Mar 2025 12:44:36 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/22474 )
Change subject: IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
......................................................................
Patch Set 6:
(1 comment)
gerrit-auto-critic failed. You can reproduce it locally using command:
python3 bin/jenkins/critique-gerrit-review.py --dryrun
To run it, you might need a virtual env with Python3's venv installed.
http://gerrit.cloudera.org:8080/#/c/22474/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/22474/6/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2888
PS6, Line 2888: private void dropIcebergTableStats(Table table, InProgressTableModification modification,
line too long (91 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 6
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 07 Mar 2025 18:25:55 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Posted by "Zoltan Borok-Nagy (Code Review)" <ge...@cloudera.org>.
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/22474 )
Change subject: IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/22474/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/22474/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2823
PS3, Line 2823: applyAlterTable(msTbl, false, null, catalogTimeline);
We should never directly modify the msTbl of integrated Iceberg tables, we always need to go through via Iceberg API. So in this case you should use IcebergCatalogOpExecutor.unsetTblProperties();
You might also need to invoke IcebergCatalogOpExecutor.addCatalogVersionToTxn(), so Impala's event processor will be able to detect it as self-event.
Another example is updateTableStatsViaIceberg() in this class.
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 3
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Fri, 07 Mar 2025 10:05:54 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Posted by "Mihaly Szjatinya (Code Review)" <ge...@cloudera.org>.
Mihaly Szjatinya has posted comments on this change. ( http://gerrit.cloudera.org:8080/22474 )
Change subject: IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/22474/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/22474/2//COMMIT_MSG@7
PS2, Line 7: IMPA
> Is it still WIP?
No, removed
http://gerrit.cloudera.org:8080/#/c/22474/2/testdata/workloads/functional-query/queries/QueryTest/iceberg-compute-stats.test
File testdata/workloads/functional-query/queries/QueryTest/iceberg-compute-stats.test:
http://gerrit.cloudera.org:8080/#/c/22474/2/testdata/workloads/functional-query/queries/QueryTest/iceberg-compute-stats.test@77
PS2, Line 77: ---- QUERY
: # Table-level stats are not affected by DROP STATS.
: describe formatted ice_alltypes;
: ---- RESULTS: VERIFY_IS_SUBSET
: '','numFiles ','1 '
: '','numRows ','2 '
: row_
> Do you also want to resolve this for Iceberg tables in HiveCatalog?
Pls check the new patch, it works and doesn't break any other test, but there is a comment which makes it uncertain; https://github.com/apache/Impala/blob/master/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java#L2818
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 3
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Thu, 06 Mar 2025 20:18:03 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Posted by "Mihaly Szjatinya (Code Review)" <ge...@cloudera.org>.
Mihaly Szjatinya has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/22474 )
Change subject: IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
......................................................................
IMPALA-11597: Unset impala.lastComputeStatsTime during DROP STATS
Removes 'impala.lastComputeStatsTime' property from table on DROP STATS
catalog operation. Does not affect the incremental variant:
'DROP INCREMENTAL STATS t PARTITION (partition_spec)'
Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
---
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M testdata/workloads/functional-query/queries/QueryTest/iceberg-compute-stats.test
2 files changed, 9 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/74/22474/3
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 3
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
[Impala-ASF-CR] WIP IMPALA-11597: DROP STATS should unset table property impala.lastComputeStatsTime
Posted by "Mihaly Szjatinya (Code Review)" <ge...@cloudera.org>.
Mihaly Szjatinya has posted comments on this change. ( http://gerrit.cloudera.org:8080/22474 )
Change subject: WIP IMPALA-11597: DROP STATS should unset table property impala.lastComputeStatsTime
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/22474/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/22474/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2824
PS1, Line 2824: boolean droppedLastCompute =
: msTbl.getParameters().remove(HdfsTable.TBL_PROP_LAST_COMPUTE_STATS_TIME) != null;
> Looks like there is a detail missing here that is not explained.
You mean in terms of why this issue is needed? That's probably better to discuss in Jira, there is short explanation there to start with.
Or why it is done exactly at this point in code? Pls clarify.
--
To view, visit http://gerrit.cloudera.org:8080/22474
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id743bc9779141df7a26a95a0ea201ef6fc6aeff9
Gerrit-Change-Number: 22474
Gerrit-PatchSet: 1
Gerrit-Owner: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Mihaly Szjatinya <ms...@pm.me>
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Wed, 12 Feb 2025 22:21:24 +0000
Gerrit-HasComments: Yes