Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12052 )
Change subject: IMPALA-7939: Fix issue where CTE is categorized as DML statement ...................................................................... Patch Set 6: (3 comments) http://gerrit.cloudera.org:8080/#/c/12052/5/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/12052/5/shell/impala_shell.py@1148 PS5, Line 1148: # Set posix=False to preserve the quotes. > Appears you are breaking this if you use non-posix mode. I totally agree that using lexer to parse SQL is wrong. Unfortunately sqlparse support for CTE is also broken :( Anyway, I have a fix for this in PS5 that still uses shlex. I tested it against quoted strings found no issue so far. http://gerrit.cloudera.org:8080/#/c/12052/5/shell/impala_shell.py@1151 PS5, Line 1151: try: > Per docs, doesn't apply in non-posix mode. Yup, you're right. Removed. http://gerrit.cloudera.org:8080/#/c/12052/5/tests/shell/test_shell_interactive.py File tests/shell/test_shell_interactive.py: http://gerrit.cloudera.org:8080/#/c/12052/5/tests/shell/test_shell_interactive.py@744 PS5, Line 744: for keyword in ["insert", "upsert", "update", "delete", "\\'insert\\'", > Update this to include escaped quotes/strings if you fix it. Done -- To view, visit http://gerrit.cloudera.org:8080/12052 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I011b8e73a0477ac6b2357725452458f972785ae7 Gerrit-Change-Number: 12052 Gerrit-PatchSet: 6 Gerrit-Owner: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com> Gerrit-Reviewer: Gabor Kaszab <gaborkas...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Michael Brown <mi...@cloudera.com> Gerrit-Comment-Date: Mon, 10 Dec 2018 08:20:43 +0000 Gerrit-HasComments: Yes
