Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/14080 )
Change subject: IMPALA-8755: Backend support for Z-ordering ...................................................................... Patch Set 20: Code-Review+1 (5 comments) Found some nits, but I think it's almost done :) http://gerrit.cloudera.org:8080/#/c/14080/20/be/src/util/tuple-row-compare-test.cc File be/src/util/tuple-row-compare-test.cc: http://gerrit.cloudera.org:8080/#/c/14080/20/be/src/util/tuple-row-compare-test.cc@164 PS20, Line 164: have nit: double have http://gerrit.cloudera.org:8080/#/c/14080/20/be/src/util/tuple-row-compare-test.cc@167 PS20, Line 167: sizeof(char*) + sizeof(int32_t*) * 2 nit: please add comment about the layout of tuple_row_mem. http://gerrit.cloudera.org:8080/#/c/14080/20/be/src/util/tuple-row-compare-test.cc@172 PS20, Line 172: tuple_mem->SetNotNull(NullIndicatorOffset(0,1)); Don't we need to set both slots as not nulls? http://gerrit.cloudera.org:8080/#/c/14080/20/be/src/util/tuple-row-compare-test.cc@180 PS20, Line 180: DCHECK nit: use DCHECK_EQ instead http://gerrit.cloudera.org:8080/#/c/14080/20/be/src/util/tuple-row-compare.h File be/src/util/tuple-row-compare.h: http://gerrit.cloudera.org:8080/#/c/14080/20/be/src/util/tuple-row-compare.h@187 PS20, Line 187: The basic concept of getting the shared representation nit: The shared representation has an important property that could be mentioned. Namely that we transform the original a and b values to their "shared representation" a' and b' in a way that if a < b then a' is lexically less than b' regarding to their bits. Thus for ints INT_MIN would be 0, INT_MIN+1 would be 1, and so on, and in the end INT_MAX would be 111..111. -- To view, visit http://gerrit.cloudera.org:8080/14080 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0200748ce3e65ebc5d3530f794c0f80aa335a2ab Gerrit-Change-Number: 14080 Gerrit-PatchSet: 20 Gerrit-Owner: Norbert Luksa <norbert.lu...@cloudera.com> Gerrit-Reviewer: Anonymous Coward (520) Gerrit-Reviewer: Daniel Becker <daniel.bec...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Norbert Luksa <norbert.lu...@cloudera.com> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Mon, 27 Jan 2020 14:44:45 +0000 Gerrit-HasComments: Yes
