Surya Hebbar has posted comments on this change. ( http://gerrit.cloudera.org:8080/23889 )
Change subject: IMPALA-12027: Support additional details for DataSink nodes in ExecSummary ...................................................................... Patch Set 7: (7 comments) Thank you for the review, Noemi, Csaba and Joe! http://gerrit.cloudera.org:8080/#/c/23889/6/fe/src/main/java/org/apache/impala/planner/MultiDataSink.java File fe/src/main/java/org/apache/impala/planner/MultiDataSink.java: http://gerrit.cloudera.org:8080/#/c/23889/6/fe/src/main/java/org/apache/impala/planner/MultiDataSink.java@21 PS6, Line 21: import java.util.List; > unused import Done http://gerrit.cloudera.org:8080/#/c/23889/6/fe/src/main/java/org/apache/impala/planner/MultiDataSink.java@23 PS6, Line 23: import org.apache.imp > unused import Done http://gerrit.cloudera.org:8080/#/c/23889/6/fe/src/main/java/org/apache/impala/planner/MultiDataSink.java@32 PS6, Line 32: /** > Use com.google.common.base.Preconditions Thank you for the explanation, I did not know this. http://gerrit.cloudera.org:8080/#/c/23889/6/tests/query_test/test_observability.py File tests/query_test/test_observability.py: http://gerrit.cloudera.org:8080/#/c/23889/6/tests/query_test/test_observability.py@a278 PS6, Line 278: > Is it intentional to remove this line? If so, why remove it? Sorry for missing this in diff. I had removed it by mistake. It's essential to wait for admission control in order to populate exec_summary. http://gerrit.cloudera.org:8080/#/c/23889/6/tests/query_test/test_observability.py@289 PS6, Line 289: """Test that the exec summary contains the correct table details""" : query = "create table if not exists %s.iceberg_test stored as iceberg "\ : % unique_database + "as select * from functional_parquet.iceberg_v2_no_deletes" : profile = self.execute_query(query > This CTAS statement will have an insert sink, which we could also test that Done. I have added the check for insert sink label detail. Thank you for the suggestion. http://gerrit.cloudera.org:8080/#/c/23889/6/tests/query_test/test_observability.py@304 PS6, Line 304: 3" % uni > nit: typo Done http://gerrit.cloudera.org:8080/#/c/23889/6/tests/query_test/test_observability.py@305 PS6, Line 305: profile = self.execute_query(query).runtime_profile : exec_summary = re.search(f"ExecSummary:(.*?)Errors", profile, re.DOTALL).group(1) > Could you please assert that the table name is displayed for the sink and n I have added assertion to ensure the table details belong to the respective type of 'SINK NODE' or 'SCAN NODE'. (regex search without re.DOTALL ensures that both terms are on the same line) -- To view, visit http://gerrit.cloudera.org:8080/23889 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2652dd896f72c5c6bbe7e76facdede2a237808d5 Gerrit-Change-Number: 23889 Gerrit-PatchSet: 7 Gerrit-Owner: Surya Hebbar <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Noemi Pap-Takacs <[email protected]> Gerrit-Reviewer: Surya Hebbar <[email protected]> Gerrit-Comment-Date: Wed, 13 May 2026 10:09:03 +0000 Gerrit-HasComments: Yes
