Gabor Kaszab has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16179 )

Change subject: IMPALA-9941: ExprTest.CastExprs fails when running with ASAN fix
......................................................................


Patch Set 3:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/16179/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/16179/3//COMMIT_MSG@13
PS3, Line 13: Testxing
typo


http://gerrit.cloudera.org:8080/#/c/16179/3/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:

http://gerrit.cloudera.org:8080/#/c/16179/3/be/src/exprs/expr-test.cc@3311
PS3, Line 3311: TestTimestampValue("cast('  \t\r\n      01:05:01   \t\r\n     ' 
as timestamp)",
              :       TimestampValue::ParseSimpleDateFormat("01:05:01"));
Does this produce a valid TS?


http://gerrit.cloudera.org:8080/#/c/16179/3/be/src/exprs/expr-test.cc@3313
PS3, Line 3313: //  TestIsNull("cast(' \t\r\n 01:05:01.123456789   \t\r\n     ' 
as timestamp)",
              : //      TYPE_TIMESTAMP);
Please don't leave commented code.


http://gerrit.cloudera.org:8080/#/c/16179/3/be/src/exprs/expr-test.cc@3327
PS3, Line 3327: TestIsNull("cast('  \t\r\n      1:5:1    \t\r\n    ' as 
timestamp)", TYPE_TIMESTAMP);
              :   TestIsNull(
              :       "cast('  \t\r\n      1:5:1.12345678    \t\r\n    ' as 
timestamp)", TYPE_TIMESTAMP);
These have nothing to do with IMPALA-6995, please move them to a different 
section of the test file.


http://gerrit.cloudera.org:8080/#/c/16179/3/be/src/runtime/datetime-simple-date-format-parser.cc
File be/src/runtime/datetime-simple-date-format-parser.cc:

http://gerrit.cloudera.org:8080/#/c/16179/3/be/src/runtime/datetime-simple-date-format-parser.cc@346
PS3, Line 346: str[4]
For me the root cause is still a bit unclear. The ASAN build said that for 
certain inputs here we try to read from memory that was actually released. I 
wonder how come we get to this point when we had released 'str' and where it is 
actually released?



--
To view, visit http://gerrit.cloudera.org:8080/16179
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I75bd47b6627a2e338c46dc354f7e67d34c1abbcf
Gerrit-Change-Number: 16179
Gerrit-PatchSet: 3
Gerrit-Owner: Adam Tamas <ta...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <gaborkas...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Comment-Date: Mon, 13 Jul 2020 09:24:18 +0000
Gerrit-HasComments: Yes

Reply via email to