Noemi Pap-Takacs has posted comments on this change. ( http://gerrit.cloudera.org:8080/24274 )
Change subject: IMPALA-14798: Fix IllegalStateException on COMPUTE STATS on Iceberg tables ...................................................................... Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/24274/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/24274/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2135 PS3, Line 2135: if (msTbl.getParameters().containsKey(CATALOG_SERVICE_ID)) { : props.put(CATALOG_SERVICE_ID, msTbl.getParameters().get(CATALOG_SERVICE_ID)); : } : if (msTbl.getParameters().containsKey(CATALOG_VERSION)) { : props.put(CATALOG_VERSION, msTbl.getParameters().get(CATALOG_VERSION)); : } > I agree with you conceptually. From my understanding, they do act as a pair What is missing now is the Preconditions check iff isEventProcessingEnabled(). Then we must ensure that both are present. If event processing is disabled, then I agree, we should keep what is already there. Your logic now ensures this, but misses the enforcement of the properties when event processing is enabled. http://gerrit.cloudera.org:8080/#/c/24274/3/tests/custom_cluster/test_metadata_no_events_processing.py File tests/custom_cluster/test_metadata_no_events_processing.py: http://gerrit.cloudera.org:8080/#/c/24274/3/tests/custom_cluster/test_metadata_no_events_processing.py@342 PS3, Line 342: self.run_stmt_in_hive("create table %s (id int) stored by iceberg;" % tbl) > Within the end-to-end tests, I looked for a way to verify this but could no That's exactly what we want to see, that CATALOG_SERVICE_ID is missing. Just assert that it is not there - throw an error if it is. If both CATALOG_SERVICE_ID and CATALOG_VERSION is present that means that we cannot test what we want: how Impala behaves in their absence. Then the test could be false positive. -- To view, visit http://gerrit.cloudera.org:8080/24274 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I71c36872bf84c44af8283bab11ce97e60a3e598c Gerrit-Change-Number: 24274 Gerrit-PatchSet: 3 Gerrit-Owner: Surya Hebbar <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Noemi Pap-Takacs <[email protected]> Gerrit-Reviewer: Surya Hebbar <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Fri, 19 Jun 2026 08:53:10 +0000 Gerrit-HasComments: Yes
