Michal Ostrowski has posted comments on this change. ( http://gerrit.cloudera.org:8080/11535 )
Change subject: IMPALA-6661 Make NaN values equal for grouping purposes. ...................................................................... Patch Set 16: (3 comments) > Patch Set 16: > > (3 comments) > > A few more comments as I learn this code. http://gerrit.cloudera.org:8080/#/c/11535/16//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/11535/16//COMMIT_MSG@17 PS16, Line 17: additional logic to consider NaN values as equal. > The comment mentions equality. Is the rule that NaN = NaN is always false? NaN = NaN is always false. NaN <=> NaN is always true. The "DISTINCT" implementation does not need to change. Testing before/after shows that after this change the behavior is correct. http://gerrit.cloudera.org:8080/#/c/11535/16/be/src/runtime/raw-value.cc File be/src/runtime/raw-value.cc: http://gerrit.cloudera.org:8080/#/c/11535/16/be/src/runtime/raw-value.cc@33 PS16, Line 33: const float RawValue::CANONICAL_FLOAT_NAN = nanf(""); > NaN is not a single bit pattern (which is why you've defined a canonical fo The only requirement is that within a particular process we are consistent. But, as I understand it these functions should be consistent anyways. http://gerrit.cloudera.org:8080/#/c/11535/16/testdata/workloads/functional-query/queries/QueryTest/exprs.test File testdata/workloads/functional-query/queries/QueryTest/exprs.test: http://gerrit.cloudera.org:8080/#/c/11535/16/testdata/workloads/functional-query/queries/QueryTest/exprs.test@3074 PS16, Line 3074: ---- QUERY > The thought is that we might want to ensure all paths work to handle the Na The issue in this bug is that we want "GROUP BY" to consider NaN==NaN, but not "WHERE NaN op floatCal". The JOIN behavior is implicitly doing a "GROUP BY" with respect ot the hash table. -- To view, visit http://gerrit.cloudera.org:8080/11535 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I996c4a2e1934fd887046ed0c55457b7285375086 Gerrit-Change-Number: 11535 Gerrit-PatchSet: 16 Gerrit-Owner: Michal Ostrowski <[email protected]> Gerrit-Reviewer: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Michal Ostrowski <[email protected]> Gerrit-Reviewer: Paul Rogers <[email protected]> Gerrit-Reviewer: Thomas Marshall <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Tue, 23 Oct 2018 02:15:23 +0000 Gerrit-HasComments: Yes
