[Impala-ASF-CR] IMPALA-4493: fix string-compare-test when using clang
Internal Jenkins has submitted this change and it was merged. Change subject: IMPALA-4493: fix string-compare-test when using clang .. IMPALA-4493: fix string-compare-test when using clang Only the 0 value or sign bit is specified in the return value for strncmp(), so fix up the test accordingly. Testing: - verified the new test still reproduces IMPALA-4436 - verify the new test passes under ASAN build Change-Id: I5d82ac2bff33fdbf66275fcfc6558c4bc29de5e7 Reviewed-on: http://gerrit.cloudera.org:8080/5110 Reviewed-by: Jim AppleTested-by: Internal Jenkins --- M be/src/runtime/string-compare-test.cc 1 file changed, 9 insertions(+), 2 deletions(-) Approvals: Jim Apple: Looks good to me, approved Internal Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/5110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: merged Gerrit-Change-Id: I5d82ac2bff33fdbf66275fcfc6558c4bc29de5e7 Gerrit-PatchSet: 3 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dan Hecht Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-4493: fix string-compare-test when using clang
Internal Jenkins has posted comments on this change. Change subject: IMPALA-4493: fix string-compare-test when using clang .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/5110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5d82ac2bff33fdbf66275fcfc6558c4bc29de5e7 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dan HechtGerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4493: fix string-compare-test when using clang
Jim Apple has posted comments on this change. Change subject: IMPALA-4493: fix string-compare-test when using clang .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/5110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5d82ac2bff33fdbf66275fcfc6558c4bc29de5e7 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dan HechtGerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4493: fix string-compare-test when using clang
Internal Jenkins has posted comments on this change. Change subject: IMPALA-4493: fix string-compare-test when using clang .. Patch Set 1: Verified-1 Build failed: http://sandbox.jenkins.cloudera.com/job/impala-external-gerrit-verify-merge-ASF/490/ -- To view, visit http://gerrit.cloudera.org:8080/5110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5d82ac2bff33fdbf66275fcfc6558c4bc29de5e7 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dan HechtGerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4493: fix string-compare-test when using clang
Dan Hecht has posted comments on this change. Change subject: IMPALA-4493: fix string-compare-test when using clang .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/5110/1/be/src/runtime/string-compare-test.cc File be/src/runtime/string-compare-test.cc: Line 34: EXPECT_EQ(stringcompare_r, 0); > But the args to EXPECT_EQ don't include the args to RunTestCase: the actual Oops, sorry, didn't read that carefully enough. Done. -- To view, visit http://gerrit.cloudera.org:8080/5110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5d82ac2bff33fdbf66275fcfc6558c4bc29de5e7 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dan HechtGerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4493: fix string-compare-test when using clang
Hello Tim Armstrong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/5110 to look at the new patch set (#2). Change subject: IMPALA-4493: fix string-compare-test when using clang .. IMPALA-4493: fix string-compare-test when using clang Only the 0 value or sign bit is specified in the return value for strncmp(), so fix up the test accordingly. Testing: - verified the new test still reproduces IMPALA-4436 - verify the new test passes under ASAN build Change-Id: I5d82ac2bff33fdbf66275fcfc6558c4bc29de5e7 --- M be/src/runtime/string-compare-test.cc 1 file changed, 9 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/10/5110/2 -- To view, visit http://gerrit.cloudera.org:8080/5110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5d82ac2bff33fdbf66275fcfc6558c4bc29de5e7 Gerrit-PatchSet: 2 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dan HechtGerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-4493: fix string-compare-test when using clang
Jim Apple has posted comments on this change. Change subject: IMPALA-4493: fix string-compare-test when using clang .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/5110/1/be/src/runtime/string-compare-test.cc File be/src/runtime/string-compare-test.cc: Line 34: EXPECT_EQ(stringcompare_r, 0); > EXPECT_EQ already prints its args (stringcompare_r), and the exact value of But the args to EXPECT_EQ don't include the args to RunTestCase: the actual strings being compared. -- To view, visit http://gerrit.cloudera.org:8080/5110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5d82ac2bff33fdbf66275fcfc6558c4bc29de5e7 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dan HechtGerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4493: fix string-compare-test when using clang
Dan Hecht has posted comments on this change. Change subject: IMPALA-4493: fix string-compare-test when using clang .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/5110/1/be/src/runtime/string-compare-test.cc File be/src/runtime/string-compare-test.cc: Line 34: EXPECT_EQ(stringcompare_r, 0); > Failures here would be easier to diagnose with the addition of " << l << ' EXPECT_EQ already prints its args (stringcompare_r), and the exact value of strncmp_r isn't interesting (only whether it is 0, neg, pos) which is implied by the EXPECT case. -- To view, visit http://gerrit.cloudera.org:8080/5110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5d82ac2bff33fdbf66275fcfc6558c4bc29de5e7 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dan HechtGerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4493: fix string-compare-test when using clang
Jim Apple has posted comments on this change. Change subject: IMPALA-4493: fix string-compare-test when using clang .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/5110/1/be/src/runtime/string-compare-test.cc File be/src/runtime/string-compare-test.cc: Line 34: EXPECT_EQ(stringcompare_r, 0); Failures here would be easier to diagnose with the addition of " << l << ' ' << r" -- To view, visit http://gerrit.cloudera.org:8080/5110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5d82ac2bff33fdbf66275fcfc6558c4bc29de5e7 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dan HechtGerrit-Reviewer: Jim Apple Gerrit-Reviewer: Tim Armstrong Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-4493: fix string-compare-test when using clang
Tim Armstrong has posted comments on this change. Change subject: IMPALA-4493: fix string-compare-test when using clang .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/5110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5d82ac2bff33fdbf66275fcfc6558c4bc29de5e7 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dan HechtGerrit-Reviewer: Tim Armstrong Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-4493: fix string-compare-test when using clang
Dan Hecht has uploaded a new change for review. http://gerrit.cloudera.org:8080/5110 Change subject: IMPALA-4493: fix string-compare-test when using clang .. IMPALA-4493: fix string-compare-test when using clang Only the 0 value or sign bit is specified in the return value for strncmp(), so fix up the test accordingly. Testing: - verified the new test still reproduces IMPALA-4436 - verify the new test passes under ASAN build Change-Id: I5d82ac2bff33fdbf66275fcfc6558c4bc29de5e7 --- M be/src/runtime/string-compare-test.cc 1 file changed, 9 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/10/5110/1 -- To view, visit http://gerrit.cloudera.org:8080/5110 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I5d82ac2bff33fdbf66275fcfc6558c4bc29de5e7 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dan Hecht