Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/23075 )
Change subject: IMPALA-13892: Add support for printing STRUCTs ...................................................................... Patch Set 3: (6 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()? The frontend enforces a maximum depth of 100 for nesting of complex types, so I think we can rely on that. https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/catalog/Type.java#L45-L49 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 Done http://gerrit.cloudera.org:8080/#/c/23075/3/be/src/util/debug-util.cc@238 PS3, Line 238: TupleRow > can this be const? Done http://gerrit.cloudera.org:8080/#/c/23075/3/be/src/util/debug-util.cc@247 PS3, Line 247: TupleRow > can this be const? Done http://gerrit.cloudera.org:8080/#/c/23075/3/be/src/util/debug-util.cc@260 PS3, Line 260: RowBatch > can this be const? Done 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 h Added a counter to tell us that correctness verification is being used and checked that counter here. -- 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 <joemcdonn...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: Kurt Deschler <kdesc...@cloudera.com> Gerrit-Reviewer: Michael Smith <michael.sm...@cloudera.com> Gerrit-Reviewer: Yida Wu <wydbaggio...@gmail.com> Gerrit-Comment-Date: Thu, 26 Jun 2025 23:24:22 +0000 Gerrit-HasComments: Yes
