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 1: (4 comments) I took a quick look. I'll dive into it more tomorrow. http://gerrit.cloudera.org:8080/#/c/14291/1/be/src/runtime/date-parse-util.cc File be/src/runtime/date-parse-util.cc: http://gerrit.cloudera.org:8080/#/c/14291/1/be/src/runtime/date-parse-util.cc@158 PS1, Line 158: , 3) Last parameter 3 can be removed, MONTH_ARRAY[] elements are already 3 char length. http://gerrit.cloudera.org:8080/#/c/14291/1/be/src/runtime/date-parse-util.cc@172 PS1, Line 172: boost::replace_all(text_token, "\\\\\\\"", "\""); I think, this works only if the format string is between double quotes. What happens if format string is between apostrophe characters? http://gerrit.cloudera.org:8080/#/c/14291/1/be/src/runtime/date-parse-util.cc@172 PS1, Line 172: boost::replace_all(text_token, "\\\\\\\"", "\""); : boost::replace_all(text_token, "\\\"", "\""); Please add some comments about these calls. http://gerrit.cloudera.org:8080/#/c/14291/1/tests/query_test/test_cast_with_format.py File tests/query_test/test_cast_with_format.py: http://gerrit.cloudera.org:8080/#/c/14291/1/tests/query_test/test_cast_with_format.py@595 PS1, Line 595: "select cast('1985-11-21' as timestamp " : "format '\"\"YYYY\"\"-\"\"MM\"\"-\"\"DD\"\"')" This and other strings would be easier to read if you used python's raw-string notation. For instance, instead of: "select cast('1985-11-21' as timestamp format '\"\"YYYY\"\"-\"\"MM\"\"-\"\"DD\"\"')" you could write: r'''select cast('1985-11-21' as timestamp format '""YYYY""-""MM""-""DD""')''' no need to escape " ' \ characters inside raw strings. -- 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: 1 Gerrit-Owner: Gabor Kaszab <gaborkas...@cloudera.com> Gerrit-Reviewer: Attila Jeges <atti...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Comment-Date: Tue, 24 Sep 2019 16:08:13 +0000 Gerrit-HasComments: Yes
