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:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/14291/2/tests/query_test/test_cast_with_format.py
File tests/query_test/test_cast_with_format.py:

http://gerrit.cloudera.org:8080/#/c/14291/2/tests/query_test/test_cast_with_format.py@578
PS2, Line 578: test_text_token
> I'm confused about how a backslash character should be represented inside t
Thanks for spotting this. The code is not really prepared to handle backslashes 
inside the text token. What makes this complicated is that when parsing we 
don't know if the content of the text token is escaped or double escaped 
(latter can happen when the surrounding double quotes of the text token are 
escaped themselves.) so we won't know how many backslashes to skip.
I'll give this a second though, or if I can't figure out anything we might want 
to emphasize that backslashes are not supported only for escaping double quotes.


http://gerrit.cloudera.org:8080/#/c/14291/2/tests/query_test/test_cast_with_format.py@580
PS2, Line 580: '''
             :         r'''
> You can probably remove these from the end of the L580 and the beginning of
It might work, but I wanted to not include a new line char in the query string. 
I don't see much benefit of removing these.



--
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 <[email protected]>
Gerrit-Reviewer: Attila Jeges <[email protected]>
Gerrit-Reviewer: Gabor Kaszab <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Comment-Date: Fri, 27 Sep 2019 11:36:46 +0000
Gerrit-HasComments: Yes

Reply via email to