Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/8790 )
Change subject: IMPALA-6069: Fix CodegenAnyVal's handling of 'nan' ...................................................................... Patch Set 2: (2 comments) > Nice catch ! Do we have a similar problem with the FCMP instruction > in LlvmCodeGen::CodegenMinMax() ? Yes, but it looks like its more complicated than just switching the llvm function we call, so with 2.11 about to be branched I filed IMPALA-6295 so we can deal with it separately. http://gerrit.cloudera.org:8080/#/c/8790/1/be/src/codegen/codegen-anyval.cc File be/src/codegen/codegen-anyval.cc: http://gerrit.cloudera.org:8080/#/c/8790/1/be/src/codegen/codegen-anyval.cc@679 PS1, Line 679: // Use the ordering version "OEQ" to ensure that 'nan' != 'nan'. > Can you add a comment explaining why this variant is used? It's subtle. I l Done http://gerrit.cloudera.org:8080/#/c/8790/1/testdata/workloads/functional-query/queries/QueryTest/joins.test File testdata/workloads/functional-query/queries/QueryTest/joins.test: http://gerrit.cloudera.org:8080/#/c/8790/1/testdata/workloads/functional-query/queries/QueryTest/joins.test@775 PS1, Line 775: # Test that 'nan' != 'nan' when joining. > Can we test a few more NaN variants just to get additional coverage? I thin Done -- To view, visit http://gerrit.cloudera.org:8080/8790 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1bb8e5074b3c939927dedc46bc9db63ca24486a1 Gerrit-Change-Number: 8790 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Tauber-Marshall <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Fri, 08 Dec 2017 00:45:37 +0000 Gerrit-HasComments: Yes
