Joe McDonnell 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 9: Code-Review+1

(1 comment)

This is looking good to me

http://gerrit.cloudera.org:8080/#/c/23889/8/tests/query_test/test_observability.py
File tests/query_test/test_observability.py:

http://gerrit.cloudera.org:8080/#/c/23889/8/tests/query_test/test_observability.py@292
PS8, Line 292:     result = self.client.execute(query, fetch_exec_summary=True)
             :
> Thank you for suggesting this.
I think it would take some code to reconstruct the ExecSummary table. 
Basically, the server returns it as a Thrift struct. There is code in 
impala-shell that converts it to a table. The exec_summary that's returned here 
is a special piece of code to convert it to dictionaries to make it easy to 
interact with individual fields. You can see this logic in 
impala_connection.py's build_summary_table_from_thrift(). If you want to 
include the full exec summary, the regex that you were using on the profile 
would probably be easier.

For simple asserts comparing two values, pytest will give details about the two 
values. For example, if I corrupt the assert in test_scan_summary(), I get an 
error like this:

query_test/test_observability.py:109: in test_scan_summary
    assert result.exec_summary[scan_idx]['operator'] == '00:SCANfoo ' + 
FILESYSTEM_NAME
E   AssertionError: assert '00:SCAN HDFS' == '00:SCANfoo HDFS'
E     - 00:SCANfoo HDFS
E     ?        ---
E     + 00:SCAN HDFS

For test conditions like this, I consider a special error message to be 
optional. These are deterministic tests and if it is failing, we'll be able to 
reproduce it.



--
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: 9
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: Mon, 18 May 2026 19:47:18 +0000
Gerrit-HasComments: Yes

Reply via email to