Gabor Kaszab 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 2:

(4 comments)

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:
> Last parameter 3 can be removed, MONTH_ARRAY[] elements are already 3 char
Done


http://gerrit.cloudera.org:8080/#/c/14291/1/be/src/runtime/date-parse-util.cc@172
PS1, Line 172: // Remove the escaping from the double quotes ins
> I think, this works only if the format string is between double quotes.  Wh
The first replace is used when the format is between double quotes, the text 
token is surrounded by escaped double quotes and there is a double escaped 
double quote inside.  E.g.: ... FORMAT "YYYY\"text1\\\"text2\"")

The replace below is used when the there is an escaped double quote inside the 
text token (not double-escaped).

Note that these replace calls are running only the content of the text token 
and not for the surrounding double or single quotes.

If the format is covered with single quotes than the forst replace doesn't have 
to run because the quote inside the text token won't be double-escaped.


http://gerrit.cloudera.org:8080/#/c/14291/1/be/src/runtime/date-parse-util.cc@172
PS1, Line 172:        // Remove the escaping from the double quotes inside the 
text token.
             :         // If the format is surrounded by double quot
> Please add some comments about these calls.
Done


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: r'''select cast('1985-11-21' as timestamp '''
             :         r'''format '""YYYY""-""MM""-""DD""')''')
> This and other strings would be easier to read if you used python's raw-str
Thanks for the comment, this is in fact way more readable with 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: 2
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: Thu, 26 Sep 2019 13:49:16 +0000
Gerrit-HasComments: Yes

Reply via email to