[Impala-ASF-CR] IMPALA-5146: Fix inconsitent results at FROM UNIXTIME()
Kim Jin Chul has posted comments on this change. ( http://gerrit.cloudera.org:8080/8629 ) Change subject: IMPALA-5146: Fix inconsitent results at FROM_UNIXTIME() .. Patch Set 6: Dan and Jim, I appreciate you review. -- To view, visit http://gerrit.cloudera.org:8080/8629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie3a5e9a9cb39d32993fa2c7f725be44d8b9ce9f2 Gerrit-Change-Number: 8629 Gerrit-PatchSet: 6 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Comment-Date: Wed, 29 Nov 2017 05:24:14 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5146: Fix inconsitent results at FROM UNIXTIME()
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8629 ) Change subject: IMPALA-5146: Fix inconsitent results at FROM_UNIXTIME() .. Patch Set 5: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/8629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie3a5e9a9cb39d32993fa2c7f725be44d8b9ce9f2 Gerrit-Change-Number: 8629 Gerrit-PatchSet: 5 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Comment-Date: Wed, 29 Nov 2017 05:22:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5146: Fix inconsitent results at FROM UNIXTIME()
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/8629 ) Change subject: IMPALA-5146: Fix inconsitent results at FROM_UNIXTIME() .. IMPALA-5146: Fix inconsitent results at FROM_UNIXTIME() The FROM_UNIXTIME(epoch) and FROM_UNIXTIME(epoch, format) produce different results when epoch is out of range of TimestampValue. The former produces an empty string, while the latter gives NULL. The fix is to harmonize the results to NULL. Testing: Add unit tests to ExprTest.TimestampFunctions. Change-Id: Ie3a5e9a9cb39d32993fa2c7f725be44d8b9ce9f2 Reviewed-on: http://gerrit.cloudera.org:8080/8629 Reviewed-by: Dan Hecht Tested-by: Impala Public Jenkins --- M be/src/exprs/expr-test.cc M be/src/exprs/timestamp-functions-ir.cc M testdata/workloads/functional-query/queries/QueryTest/exprs.test 3 files changed, 6 insertions(+), 3 deletions(-) Approvals: Dan Hecht: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/8629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ie3a5e9a9cb39d32993fa2c7f725be44d8b9ce9f2 Gerrit-Change-Number: 8629 Gerrit-PatchSet: 6 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul
[Impala-ASF-CR] IMPALA-5146: Fix inconsitent results at FROM UNIXTIME()
Kim Jin Chul has posted comments on this change. ( http://gerrit.cloudera.org:8080/8629 ) Change subject: IMPALA-5146: Fix inconsitent results at FROM_UNIXTIME() .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/8629/4/testdata/workloads/functional-query/queries/QueryTest/exprs.test File testdata/workloads/functional-query/queries/QueryTest/exprs.test: http://gerrit.cloudera.org:8080/#/c/8629/4/testdata/workloads/functional-query/queries/QueryTest/exprs.test@2518 PS4, Line 2518: 'NULL' > Filed the issue: https://issues.apache.org/jira/browse/IMPALA-6260 Broken link in the previous comment: IMPALA-6260 -- To view, visit http://gerrit.cloudera.org:8080/8629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie3a5e9a9cb39d32993fa2c7f725be44d8b9ce9f2 Gerrit-Change-Number: 8629 Gerrit-PatchSet: 4 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Comment-Date: Wed, 29 Nov 2017 02:40:37 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5146: Fix inconsitent results at FROM UNIXTIME()
Kim Jin Chul has posted comments on this change. ( http://gerrit.cloudera.org:8080/8629 ) Change subject: IMPALA-5146: Fix inconsitent results at FROM_UNIXTIME() .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/8629/4/testdata/workloads/functional-query/queries/QueryTest/exprs.test File testdata/workloads/functional-query/queries/QueryTest/exprs.test: http://gerrit.cloudera.org:8080/#/c/8629/4/testdata/workloads/functional-query/queries/QueryTest/exprs.test@2518 PS4, Line 2518: 'NULL' > I think it's an artifact of a problem with the test infrastructure and stri Filed the issue: https://issues.apache.org/jira/browse/IMPALA-6260 -- To view, visit http://gerrit.cloudera.org:8080/8629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie3a5e9a9cb39d32993fa2c7f725be44d8b9ce9f2 Gerrit-Change-Number: 8629 Gerrit-PatchSet: 4 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Comment-Date: Wed, 29 Nov 2017 02:38:35 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5146: Fix inconsitent results at FROM UNIXTIME()
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8629 ) Change subject: IMPALA-5146: Fix inconsitent results at FROM_UNIXTIME() .. Patch Set 5: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1539/ -- To view, visit http://gerrit.cloudera.org:8080/8629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie3a5e9a9cb39d32993fa2c7f725be44d8b9ce9f2 Gerrit-Change-Number: 8629 Gerrit-PatchSet: 5 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Comment-Date: Wed, 29 Nov 2017 01:51:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5146: Fix inconsitent results at FROM UNIXTIME()
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/8629 ) Change subject: IMPALA-5146: Fix inconsitent results at FROM_UNIXTIME() .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/8629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie3a5e9a9cb39d32993fa2c7f725be44d8b9ce9f2 Gerrit-Change-Number: 8629 Gerrit-PatchSet: 5 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Comment-Date: Wed, 29 Nov 2017 01:51:19 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5146: Fix inconsitent results at FROM UNIXTIME()
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/8629 ) Change subject: IMPALA-5146: Fix inconsitent results at FROM_UNIXTIME() .. Patch Set 4: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/8629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie3a5e9a9cb39d32993fa2c7f725be44d8b9ce9f2 Gerrit-Change-Number: 8629 Gerrit-PatchSet: 4 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Comment-Date: Wed, 29 Nov 2017 01:51:10 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5146: Fix inconsitent results at FROM UNIXTIME()
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/8629 ) Change subject: IMPALA-5146: Fix inconsitent results at FROM_UNIXTIME() .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/8629/4/testdata/workloads/functional-query/queries/QueryTest/exprs.test File testdata/workloads/functional-query/queries/QueryTest/exprs.test: http://gerrit.cloudera.org:8080/#/c/8629/4/testdata/workloads/functional-query/queries/QueryTest/exprs.test@2518 PS4, Line 2518: 'NULL' > I am wondering why the test expects 'NULL' string instead of NULL. As you k I think it's an artifact of a problem with the test infrastructure and string typed columns. I thought we have a JIRA about it somewhere though I can't find it right now. -- To view, visit http://gerrit.cloudera.org:8080/8629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie3a5e9a9cb39d32993fa2c7f725be44d8b9ce9f2 Gerrit-Change-Number: 8629 Gerrit-PatchSet: 4 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Comment-Date: Wed, 29 Nov 2017 01:50:05 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5146: Fix inconsitent results at FROM UNIXTIME()
Kim Jin Chul has posted comments on this change. ( http://gerrit.cloudera.org:8080/8629 ) Change subject: IMPALA-5146: Fix inconsitent results at FROM_UNIXTIME() .. Patch Set 4: (2 comments) Thanks Dan for the notification. http://gerrit.cloudera.org:8080/#/c/8629/4/testdata/workloads/functional-query/queries/QueryTest/exprs.test File testdata/workloads/functional-query/queries/QueryTest/exprs.test: http://gerrit.cloudera.org:8080/#/c/8629/4/testdata/workloads/functional-query/queries/QueryTest/exprs.test@2518 PS4, Line 2518: 'NULL' I am wondering why the test expects 'NULL' string instead of NULL. As you know, 'NULL' is totally different to NULL. I think it's misleading. Is there any historical reason? I found several cases when return type is string. http://gerrit.cloudera.org:8080/#/c/8629/4/testdata/workloads/functional-query/queries/QueryTest/exprs.test@2553 PS4, Line 2553: 'NULL' : TYPES : STRING Expects 'NULL' literal string -- To view, visit http://gerrit.cloudera.org:8080/8629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie3a5e9a9cb39d32993fa2c7f725be44d8b9ce9f2 Gerrit-Change-Number: 8629 Gerrit-PatchSet: 4 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Comment-Date: Wed, 29 Nov 2017 01:42:32 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5146: Fix inconsitent results at FROM UNIXTIME()
Hello Jim Apple, Dan Hecht, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8629 to look at the new patch set (#4). Change subject: IMPALA-5146: Fix inconsitent results at FROM_UNIXTIME() .. IMPALA-5146: Fix inconsitent results at FROM_UNIXTIME() The FROM_UNIXTIME(epoch) and FROM_UNIXTIME(epoch, format) produce different results when epoch is out of range of TimestampValue. The former produces an empty string, while the latter gives NULL. The fix is to harmonize the results to NULL. Testing: Add unit tests to ExprTest.TimestampFunctions. Change-Id: Ie3a5e9a9cb39d32993fa2c7f725be44d8b9ce9f2 --- M be/src/exprs/expr-test.cc M be/src/exprs/timestamp-functions-ir.cc M testdata/workloads/functional-query/queries/QueryTest/exprs.test 3 files changed, 6 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/8629/4 -- To view, visit http://gerrit.cloudera.org:8080/8629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie3a5e9a9cb39d32993fa2c7f725be44d8b9ce9f2 Gerrit-Change-Number: 8629 Gerrit-PatchSet: 4 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul
[Impala-ASF-CR] IMPALA-5146: Fix inconsitent results at FROM UNIXTIME()
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/8629 ) Change subject: IMPALA-5146: Fix inconsitent results at FROM_UNIXTIME() .. Patch Set 3: > Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/1525/ Hi Jinchul, it looks like a test case may need updating with this change? Could you please take a look? -- To view, visit http://gerrit.cloudera.org:8080/8629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie3a5e9a9cb39d32993fa2c7f725be44d8b9ce9f2 Gerrit-Change-Number: 8629 Gerrit-PatchSet: 3 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Comment-Date: Tue, 28 Nov 2017 19:04:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5146: Fix inconsitent results at FROM UNIXTIME()
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8629 ) Change subject: IMPALA-5146: Fix inconsitent results at FROM_UNIXTIME() .. Patch Set 3: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/1525/ -- To view, visit http://gerrit.cloudera.org:8080/8629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie3a5e9a9cb39d32993fa2c7f725be44d8b9ce9f2 Gerrit-Change-Number: 8629 Gerrit-PatchSet: 3 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Comment-Date: Mon, 27 Nov 2017 22:37:54 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5146: Fix inconsitent results at FROM UNIXTIME()
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/8629 ) Change subject: IMPALA-5146: Fix inconsitent results at FROM_UNIXTIME() .. Patch Set 3: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/8629/2/be/src/exprs/timestamp-functions-ir.cc File be/src/exprs/timestamp-functions-ir.cc: http://gerrit.cloudera.org:8080/#/c/8629/2/be/src/exprs/timestamp-functions-ir.cc@82 PS2, Line 82: > I don't remember which which commit id or gerrit link. I think the fix for the issue Jim is referring to was e1ae98841. I don't think it's a problem in this case, but I'm okay with dropping the use of reference here. -- To view, visit http://gerrit.cloudera.org:8080/8629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie3a5e9a9cb39d32993fa2c7f725be44d8b9ce9f2 Gerrit-Change-Number: 8629 Gerrit-PatchSet: 3 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Comment-Date: Mon, 27 Nov 2017 18:57:19 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5146: Fix inconsitent results at FROM UNIXTIME()
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/8629 ) Change subject: IMPALA-5146: Fix inconsitent results at FROM_UNIXTIME() .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1525/ -- To view, visit http://gerrit.cloudera.org:8080/8629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie3a5e9a9cb39d32993fa2c7f725be44d8b9ce9f2 Gerrit-Change-Number: 8629 Gerrit-PatchSet: 3 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Comment-Date: Mon, 27 Nov 2017 18:57:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-5146: Fix inconsitent results at FROM UNIXTIME()
Jim Apple has posted comments on this change. ( http://gerrit.cloudera.org:8080/8629 ) Change subject: IMPALA-5146: Fix inconsitent results at FROM_UNIXTIME() .. Patch Set 3: Code-Review+1 (2 comments) Leaving a +1 in case Dan has any more comments before he +2s. http://gerrit.cloudera.org:8080/#/c/8629/2/be/src/exprs/timestamp-functions-ir.cc File be/src/exprs/timestamp-functions-ir.cc: http://gerrit.cloudera.org:8080/#/c/8629/2/be/src/exprs/timestamp-functions-ir.cc@82 PS2, Line 82: > Thanks for the information. I guess TimestampValue might have a dangling po I don't remember which which commit id or gerrit link. I don't think this will create any CPU cost, due to the return-value optimization, which was common even years ago: http://en.cppreference.com/w/cpp/language/copy_elision http://gerrit.cloudera.org:8080/#/c/8629/2/be/src/exprs/timestamp-functions-ir.cc@96 PS2, Line 96: & > Is it a problematic code also? This is also not ideal, but probably not worth fixing in this commit, since commits should mostly address a single thing. -- To view, visit http://gerrit.cloudera.org:8080/8629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie3a5e9a9cb39d32993fa2c7f725be44d8b9ce9f2 Gerrit-Change-Number: 8629 Gerrit-PatchSet: 3 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Comment-Date: Mon, 27 Nov 2017 14:49:55 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5146: Fix inconsitent results at FROM UNIXTIME()
Kim Jin Chul has posted comments on this change. ( http://gerrit.cloudera.org:8080/8629 ) Change subject: IMPALA-5146: Fix inconsitent results at FROM_UNIXTIME() .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/8629/2/be/src/exprs/timestamp-functions-ir.cc File be/src/exprs/timestamp-functions-ir.cc: http://gerrit.cloudera.org:8080/#/c/8629/2/be/src/exprs/timestamp-functions-ir.cc@82 PS2, Line 82: & > I'd suggest a non-reference here. I think Henry R. caught a bug in the last Thanks for the information. I guess TimestampValue might have a dangling pointer at a member variable such as posix_time::time_duration or gregorian::date . Would you please share commit id or gerrit link for the bug fix? Regarding unavailability of the reference, I hope we should fix TImestampValue itself to use reference type because some developers who don't know this restriction which makes a corruption unexpectedly. You know there is unexpected cpu cost for memcpy by copying values of the object if we don't use reference. I guess use of shared_ptr might solve this problem. Was it filed on Jira? http://gerrit.cloudera.org:8080/#/c/8629/2/be/src/exprs/timestamp-functions-ir.cc@96 PS2, Line 96: & Is it a problematic code also? -- To view, visit http://gerrit.cloudera.org:8080/8629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie3a5e9a9cb39d32993fa2c7f725be44d8b9ce9f2 Gerrit-Change-Number: 8629 Gerrit-PatchSet: 2 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Comment-Date: Mon, 27 Nov 2017 09:32:32 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5146: Fix inconsitent results at FROM UNIXTIME()
Hello Jim Apple, Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8629 to look at the new patch set (#3). Change subject: IMPALA-5146: Fix inconsitent results at FROM_UNIXTIME() .. IMPALA-5146: Fix inconsitent results at FROM_UNIXTIME() The FROM_UNIXTIME(epoch) and FROM_UNIXTIME(epoch, format) produce different results when epoch is out of range of TimestampValue. The former produces an empty string, while the latter gives NULL. The fix is to harmonize the results to NULL. Testing: Add unit tests to ExprTest.TimestampFunctions. Change-Id: Ie3a5e9a9cb39d32993fa2c7f725be44d8b9ce9f2 --- M be/src/exprs/expr-test.cc M be/src/exprs/timestamp-functions-ir.cc 2 files changed, 5 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/8629/3 -- To view, visit http://gerrit.cloudera.org:8080/8629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie3a5e9a9cb39d32993fa2c7f725be44d8b9ce9f2 Gerrit-Change-Number: 8629 Gerrit-PatchSet: 3 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul
[Impala-ASF-CR] IMPALA-5146: Fix inconsitent results at FROM UNIXTIME()
Jim Apple has posted comments on this change. ( http://gerrit.cloudera.org:8080/8629 ) Change subject: IMPALA-5146: Fix inconsitent results at FROM_UNIXTIME() .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/8629/2/be/src/exprs/timestamp-functions-ir.cc File be/src/exprs/timestamp-functions-ir.cc: http://gerrit.cloudera.org:8080/#/c/8629/2/be/src/exprs/timestamp-functions-ir.cc@82 PS2, Line 82: & I'd suggest a non-reference here. I think Henry R. caught a bug in the last few months in which the lifetime extension didn't work as expected, leading to memory corruption. -- To view, visit http://gerrit.cloudera.org:8080/8629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie3a5e9a9cb39d32993fa2c7f725be44d8b9ce9f2 Gerrit-Change-Number: 8629 Gerrit-PatchSet: 2 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Kim Jin Chul Gerrit-Comment-Date: Sun, 26 Nov 2017 21:57:10 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5146: Fix inconsitent results at FROM UNIXTIME()
Kim Jin Chul has posted comments on this change. ( http://gerrit.cloudera.org:8080/8629 ) Change subject: IMPALA-5146: Fix inconsitent results at FROM_UNIXTIME() .. Patch Set 1: (1 comment) Thanks for your comment http://gerrit.cloudera.org:8080/#/c/8629/1/be/src/exprs/timestamp-functions-ir.cc File be/src/exprs/timestamp-functions-ir.cc: http://gerrit.cloudera.org:8080/#/c/8629/1/be/src/exprs/timestamp-functions-ir.cc@84 PS1, Line 84: ToUnixTime > i think it would be clearer to just call tv.HasDateAndTime() directly to sh Done -- To view, visit http://gerrit.cloudera.org:8080/8629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie3a5e9a9cb39d32993fa2c7f725be44d8b9ce9f2 Gerrit-Change-Number: 8629 Gerrit-PatchSet: 1 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Dan Hecht Gerrit-Reviewer: Kim Jin Chul Gerrit-Comment-Date: Thu, 23 Nov 2017 01:58:55 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5146: Fix inconsitent results at FROM UNIXTIME()
Hello Dan Hecht, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8629 to look at the new patch set (#2). Change subject: IMPALA-5146: Fix inconsitent results at FROM_UNIXTIME() .. IMPALA-5146: Fix inconsitent results at FROM_UNIXTIME() The FROM_UNIXTIME(epoch) and FROM_UNIXTIME(epoch, format) produce different results when epoch is out of range of TimestampValue. The former produces an empty string, while the latter gives NULL. The fix is to harmonize the results to NULL. Testing: Add unit tests to ExprTest.TimestampFunctions. Change-Id: Ie3a5e9a9cb39d32993fa2c7f725be44d8b9ce9f2 --- M be/src/exprs/expr-test.cc M be/src/exprs/timestamp-functions-ir.cc 2 files changed, 5 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/8629/2 -- To view, visit http://gerrit.cloudera.org:8080/8629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie3a5e9a9cb39d32993fa2c7f725be44d8b9ce9f2 Gerrit-Change-Number: 8629 Gerrit-PatchSet: 2 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Dan Hecht
[Impala-ASF-CR] IMPALA-5146: Fix inconsitent results at FROM UNIXTIME()
Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/8629 ) Change subject: IMPALA-5146: Fix inconsitent results at FROM_UNIXTIME() .. Patch Set 1: (1 comment) Thanks for the fix! http://gerrit.cloudera.org:8080/#/c/8629/1/be/src/exprs/timestamp-functions-ir.cc File be/src/exprs/timestamp-functions-ir.cc: http://gerrit.cloudera.org:8080/#/c/8629/1/be/src/exprs/timestamp-functions-ir.cc@84 PS1, Line 84: ToUnixTime i think it would be clearer to just call tv.HasDateAndTime() directly to show the intention of this check. (also, generally for Impala style, we don't use extraneous parenthesis around the conditional, though I see that in this file the style has diverged). -- To view, visit http://gerrit.cloudera.org:8080/8629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie3a5e9a9cb39d32993fa2c7f725be44d8b9ce9f2 Gerrit-Change-Number: 8629 Gerrit-PatchSet: 1 Gerrit-Owner: Kim Jin Chul Gerrit-Reviewer: Dan Hecht Gerrit-Comment-Date: Wed, 22 Nov 2017 19:50:31 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5146: Fix inconsitent results at FROM UNIXTIME()
Kim Jin Chul has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8629 Change subject: IMPALA-5146: Fix inconsitent results at FROM_UNIXTIME() .. IMPALA-5146: Fix inconsitent results at FROM_UNIXTIME() The FROM_UNIXTIME(epoch) and FROM_UNIXTIME(epoch, format) produce different results when epoch is out of range of TimestampValue. The former produces an empty string, while the latter gives NULL. The fix is to harmonize the results to NULL. Testing: Add unit tests to ExprTest.TimestampFunctions. Change-Id: Ie3a5e9a9cb39d32993fa2c7f725be44d8b9ce9f2 --- M be/src/exprs/expr-test.cc M be/src/exprs/timestamp-functions-ir.cc 2 files changed, 6 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/29/8629/1 -- To view, visit http://gerrit.cloudera.org:8080/8629 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ie3a5e9a9cb39d32993fa2c7f725be44d8b9ce9f2 Gerrit-Change-Number: 8629 Gerrit-PatchSet: 1 Gerrit-Owner: Kim Jin Chul
