Yida Wu has posted comments on this change. ( http://gerrit.cloudera.org:8080/23075 )
Change subject: IMPALA-13892: Add support for printing STRUCTs ...................................................................... Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/23075/3/be/src/util/debug-util.cc File be/src/util/debug-util.cc: http://gerrit.cloudera.org:8080/#/c/23075/3/be/src/util/debug-util.cc@196 PS3, Line 196: void PrintStruct(const Tuple* t, const SlotDescriptor& slot_d, stringstream& out) { Do we need a maximum recursion depth for recursive calls to PrintSlot()? http://gerrit.cloudera.org:8080/#/c/23075/3/be/src/util/debug-util.cc@197 PS3, Line 197: DCHECK(slot_d.type().IsStructType()); It looks like t shouldn't be nullptr here, would it be good to add a DCHECK(t != nullptr)? http://gerrit.cloudera.org:8080/#/c/23075/3/tests/custom_cluster/test_tuple_cache.py File tests/custom_cluster/test_tuple_cache.py: http://gerrit.cloudera.org:8080/#/c/23075/3/tests/custom_cluster/test_tuple_cache.py@456 PS3, Line 456: assert result2.success Can we add an assertion for the profile counter to ensure the tuple cache hits and being used? -- To view, visit http://gerrit.cloudera.org:8080/23075 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib9479754c2766a9dd6483ba065e26a4d3a22e7e9 Gerrit-Change-Number: 23075 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Kurt Deschler <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Yida Wu <[email protected]> Gerrit-Comment-Date: Wed, 25 Jun 2025 22:52:17 +0000 Gerrit-HasComments: Yes
