Norbert Luksa has posted comments on this change. ( http://gerrit.cloudera.org:8080/14080 )
Change subject: IMPALA-8755: Backend support for Z-ordering ...................................................................... Patch Set 21: (6 comments) 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@42 PS20, Line 42: desc > nit: add underscore suffix Done http://gerrit.cloudera.org:8080/#/c/14080/20/be/src/util/tuple-row-compare-test.cc@164 PS20, Line 164: > nit: double have Done http://gerrit.cloudera.org:8080/#/c/14080/20/be/src/util/tuple-row-compare-test.cc@167 PS20, Line 167: teComperator(ColumnType(TYPE_BOOLEAN > nit: please add comment about the layout of tuple_row_mem. Done http://gerrit.cloudera.org:8080/#/c/14080/20/be/src/util/tuple-row-compare-test.cc@172 PS20, Line 172: This function is responsible for only the char > Don't we need to set both slots as not nulls? As discussed offline, we do not even have to set these, since by default the slots are not nullable. However this pointed out that we do not test nulls, so added a case for the IntIntTest for them. http://gerrit.cloudera.org:8080/#/c/14080/20/be/src/util/tuple-row-compare-test.cc@180 PS20, Line 180: memcpy > nit: use DCHECK_EQ instead Done 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: We transform the original a and b values to their "sha > nit: The shared representation has an important property that could be ment Done, copied your description to the comment. -- 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: 21 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: Thu, 30 Jan 2020 15:51:03 +0000 Gerrit-HasComments: Yes
