Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/9239 )
Change subject: IMPALA-6258: Uninitialized tuple pointers in row batch for empty rows ...................................................................... Patch Set 1: (4 comments) Thanks for the comments! http://gerrit.cloudera.org:8080/#/c/9239/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/9239/1//COMMIT_MSG@32 PS1, Line 32: > It would be good to do a microbenchmark to make sure we haven't regressed p Extended the commit with a measurement. Do you think we should develop a count(*) optimisation for kudu like we do for Parquet? http://gerrit.cloudera.org:8080/#/c/9239/1/be/src/runtime/tuple.h File be/src/runtime/tuple.h: http://gerrit.cloudera.org:8080/#/c/9239/1/be/src/runtime/tuple.h@225 PS1, Line 225: void SetNull(const NullIndicatorOffset& offset) { > I'm not sure about adding DCHECKs to all these functions. They're definitel Removed the DCHECKs, pointer value is 42 now, such a low address should segfault (it does segfault on my system). http://gerrit.cloudera.org:8080/#/c/9239/1/be/src/runtime/tuple.cc File be/src/runtime/tuple.cc: http://gerrit.cloudera.org:8080/#/c/9239/1/be/src/runtime/tuple.cc@49 PS1, Line 49: Tuple* Tuple::Poison() { > Can we avoid this function call indirection? MemPool just uses a bogus cons Now it is just a pointer. http://gerrit.cloudera.org:8080/#/c/9239/1/be/src/runtime/tuple.cc@87 PS1, Line 87: if (UNLIKELY(this == Poison())) return Poison(); > We can skip this case. MemPool::Allocate() actually already returns a poiso Yeah I saw that, just thought maybe it is better to keep our own poison (that sounds wrong:) ). -- To view, visit http://gerrit.cloudera.org:8080/9239 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I298122aaaa7e62eb5971508e0698e189519755de Gerrit-Change-Number: 9239 Gerrit-PatchSet: 1 Gerrit-Owner: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Thu, 08 Feb 2018 17:28:12 +0000 Gerrit-HasComments: Yes
