Matthew Jacobs has posted comments on this change. Change subject: IMPALA-2546: Fix handeling of invalid timezones ......................................................................
Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/7216/1//COMMIT_MSG Commit Message: PS1, Line 7: handeling typo http://gerrit.cloudera.org:8080/#/c/7216/1/be/src/exprs/expr-test.cc File be/src/exprs/expr-test.cc: PS1, Line 5115: TestError("from_utc_timestamp('2015-10-13 09:15:34.101', 'FOOBAR')"); : TestError("to_utc_timestamp('2015-10-13 09:15:34.101', 'FOOBAR')"); this looks good, but it'd also be good to verify the error message is what we expect it to be so we know we're hitting the expected code path. you could modify this test code to optionally take an error, but our end-to-end tests already handle this functionality and we shouldn't try to rebuild everything in expr-test. Can you add these test cases to testdata/workloads/functional-query/queries/QueryTest/exprs.test ? There are some examples where there is an expected error, search for CATCH http://gerrit.cloudera.org:8080/#/c/7216/1/be/src/exprs/timestamp-functions.cc File be/src/exprs/timestamp-functions.cc: Line 110: context->AddWarning(msg.c_str()); while you're adding the end-to-end test cases, it'd be nice to add a test to check these warnings as well. See testdata/workloads/functional-query/queries/DataErrorsTest/hdfs-scan-node-errors.test for an example of a test that has a query that completes successfully but a warning is set (the 'ERRORS' section). -- To view, visit http://gerrit.cloudera.org:8080/7216 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I587a16bb8e835a6eea02e1f87fd4033686e84d0f Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Bikramjeet Vig <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-HasComments: Yes
