Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10044 )

Change subject: IMPALA-6790: Upgrade sqlparse to 0.1.19
......................................................................


Patch Set 1:

> Patch Set 1:
>
> > We also have the source code for sqlparse checked into the repo for
>  > the shell. Should we also upgrade that version? The argument for
>  > that is that it will minimise divergence between the shell's
>  > behaviour and our test's behaviour.
>
> Looking through the tests, I don't think our tests actually use sqlparse. The 
> only python file that includes sqlparse is 
> tests/comparison/model_translator.py. It uses it for pretty printing SQL. As 
> far as I can tell, nothing uses that pretty printing.
>
> Dataload uses sqlparse as a glorified split on ';', so we could also just 
> stop using it there.

It would be good if we could upgrade to sqlparse 0.2.4 to make it easier for us 
to upgrade sqlparse in the future. I have a fix in sqlparse (not yet released): 
https://github.com/andialbrecht/sqlparse/commit/002f37dde390ab2f189c579f5fb18e191f7eed89
 that can fix this issue: https://issues.apache.org/jira/browse/IMPALA-6337. I 
believe there are quite a number of bug fixes in sqlparse, which could benefit 
us in the long run. My 2 cents.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic5289f86b78f1d77d91a8fa47d63b7a7eaa3af38
Gerrit-Change-Number: 10044
Gerrit-PatchSet: 1
Gerrit-Owner: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Fredy Wijaya <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Philip Zeyliger <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-Comment-Date: Fri, 13 Apr 2018 18:09:03 +0000
Gerrit-HasComments: No

Reply via email to