Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/9195 )
Change subject: IMPALA-6337: Fix infinite loop in Impala shell ...................................................................... Patch Set 12: (2 comments) > Patch Set 12: > > > Patch Set 12: > > > > > Patch Set 11: > > > > > > Fredy, > > > > > > Can you leave pointers in the patch code to indicate that it's fixed > > > upstream (and where-ish)? > > > > Created a new patchseet with the indication that the issue is fixed > > upstream and a link to the fix. > > It looks like we're sticking with v0.1.14 with this fix. Are we sure we don't > want any of the fixes list in the CHANGELOG up to v0.1.19? One of them is a > actually a fix by caseyching, a former Impala engineer. > > From https://github.com/andialbrecht/sqlparse/blob/master/CHANGELOG > > Release 0.1.16 (Jul 26, 2015) > ----------------------------- > > Bug Fixes > > * Fix a regression in get_alias() introduced in 0.1.15 (issue185). > * Fix a bug in the splitter regarding DECLARE (issue193). > * sqlformat command line tool doesn't duplicat newlines anymore (issue191). > * Don't mix up MySQL comments starting with hash and MSSQL > temp tables (issue192). > * Statement.get_type() now ignores comments at the beginning of > a statement (issue186). > > > Release 0.1.15 (Apr 15, 2015) > ----------------------------- > > Bug Fixes > > * Fix a regression for identifiers with square bracktes > notation (issue153, by darikg). > * Add missing SQL types (issue154, issue155, issue156, by jukebox). > * Fix parsing of multi-line comments (issue172, by JacekPliszka). > * Fix parsing of escaped backslashes (issue174, by caseyching). > * Fix parsing of identifiers starting with underscore (issue175). > * Fix misinterpretation of IN keyword (issue183). > > Enhancements > > * Improve formatting of HAVING statements. > * Improve parsing of inline comments (issue163). > * Group comments to parent object (issue128, issue160). > * Add double precision builtin (issue169, by darikg). > * Add support for square bracket array indexing (issue170, issue176, > issue177 by darikg). > * Improve grouping of aliased elements (issue167, by darikg). > * Support comments starting with '#' character (issue178). Yeah I think it's a good idea to upgrade sqlparse to the last version that supports Python 2.6 especially if we intend to use a vendored sqlparse. IMO it's cleaner to do the upgrade in a separate patch and the apply the bug fix in another patch. We can either upgrade first and apply the bug fix or apply the bug fix first and then upgrade. Either way works for me. Thoughts? http://gerrit.cloudera.org:8080/#/c/9195/12/shell/ext-py/sqlparse-0.1.14/sqlparse/lexer.py File shell/ext-py/sqlparse-0.1.14/sqlparse/lexer.py: http://gerrit.cloudera.org:8080/#/c/9195/12/shell/ext-py/sqlparse-0.1.14/sqlparse/lexer.py@197 PS12, Line 197: Symbol > I know you didn't change this part of the line, but do you know why this is In standard SQL, '' is actually used for strings. In some databases "" are used for symbols for example select * from "functional"."alltypes" and others use "" for strings. In Impala we use "" for strings: https://github.com/apache/impala/blob/master/fe/src/main/jflex/sql-scanner.flex#L404 http://gerrit.cloudera.org:8080/#/c/9195/9/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/9195/9/shell/impala_shell.py@601 PS9, Line 601: except AttributeError: Checking if the bug can yield to an infinite loop can be tricky to do. Furthermore, bailing out with an error can reject valid SQL statements, especially for multiple SQL statements, e.g. > select * functional.alltypes; select " "; -- To view, visit http://gerrit.cloudera.org:8080/9195 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9142f21a888189d351f00ce09baeba123bc0959b Gerrit-Change-Number: 9195 Gerrit-PatchSet: 12 Gerrit-Owner: Fredy Wijaya <[email protected]> Gerrit-Reviewer: David Knupp <[email protected]> Gerrit-Reviewer: Fredy Wijaya <[email protected]> Gerrit-Reviewer: Philip Zeyliger <[email protected]> Gerrit-Reviewer: Taras Bobrovytsky <[email protected]> Gerrit-Reviewer: Vuk Ercegovac <[email protected]> Gerrit-Comment-Date: Sat, 05 May 2018 15:34:25 +0000 Gerrit-HasComments: Yes
