Todd Lipcon has posted comments on this change. Change subject: KUDU-1386 NaN float and double values are not handled correctly ......................................................................
Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/3142/1/src/kudu/common/column_predicate.cc File src/kudu/common/column_predicate.cc: Line 278: column_.type_info()->Compare(cell, this->lower_) == GREATER; I'm worried about the perf impact of this -- it's doubling the number of Compare() calls, which is actually a perf hotspot already. Is there any other trick we could use to avoid doubling the branches? http://gerrit.cloudera.org:8080/#/c/3142/1/src/kudu/common/types.h File src/kudu/common/types.h: Line 53: NONCOMPARABLE = 12 I'm not sold on NaN being not comparable. Postgres says: "Note: IEEE754 specifies that NaN should not compare equal to any other floating-point value (including NaN). In order to allow floating-point values to be sorted and used in tree-based indexes, PostgreSQL treats NaN values as equal, and greater than all non-NaN values." which I think is a reasonable position for us to take as well (both for compatibility with postgres and because their reasoning around the convenience of totally ordered types for indexing is sound) -- To view, visit http://gerrit.cloudera.org:8080/3142 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I194dcddeb8eabcc67699661b9cc9362a99f2f4ae Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Will Berkeley <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
