Attila Jeges has posted comments on this change. ( http://gerrit.cloudera.org:8080/14291 )
Change subject: IMPALA-8704: ISO:SQL:2016 datetime patterns - Milestone 2 ...................................................................... Patch Set 6: (2 comments) http://gerrit.cloudera.org:8080/#/c/14291/6/be/src/runtime/datetime-iso-sql-format-tokenizer.cc File be/src/runtime/datetime-iso-sql-format-tokenizer.cc: http://gerrit.cloudera.org:8080/#/c/14291/6/be/src/runtime/datetime-iso-sql-format-tokenizer.cc@240 PS6, Line 240: strncmp(current_pos, "\\\"", 2) == 0) > It is zero-terminated You're correct, it is null-terminated for iso-sql format strings, because the DateTimeFormatContext instance is always created using a null-terminated string (in CastFormatExpr::OpenEvaluator()). On the other hand, DateTimeFormatContext class does not guarantee that DateTimeFormatContext::fmt is null-terminated. E.g. TimestampFunctions::UnixAndFromUnixPrepare() creates a DateTimeFormatContext instance where fmt is not null-terminated. For this reason, we are very careful in SimpleDateFormatTokenizer::Tokenize() not to assume that format strings will end with a null-byte. Maybe add a comment at the beginning of this file to explain that iso-sql format strings are always null-terminated? http://gerrit.cloudera.org:8080/#/c/14291/6/tests/query_test/test_cast_with_format.py File tests/query_test/test_cast_with_format.py: http://gerrit.cloudera.org:8080/#/c/14291/6/tests/query_test/test_cast_with_format.py@738 PS6, Line 738: select cast("1985-\a\b\f\n\r\t\v\'12-09" as ''' : r'''date format 'YYYY-"\a\b\f\n\r\t\v\'"MM-DD') This test is about special escape sequences but non-special chars in the text token are escaped too. Is that intentional? -- To view, visit http://gerrit.cloudera.org:8080/14291 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I30d2f6656054371476aaa8bd0d51f572b9369855 Gerrit-Change-Number: 14291 Gerrit-PatchSet: 6 Gerrit-Owner: Gabor Kaszab <gaborkas...@cloudera.com> Gerrit-Reviewer: Attila Jeges <atti...@cloudera.com> Gerrit-Reviewer: Gabor Kaszab <gaborkas...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Comment-Date: Wed, 16 Oct 2019 18:02:04 +0000 Gerrit-HasComments: Yes
