[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9195 ) Change subject: IMPALA-6337: Fix infinite loop in Impala shell .. IMPALA-6337: Fix infinite loop in Impala shell This patch fixes a bug in sqlparse where sqlparse incorrectly splits a statement that has a new line inside double quotes. The bug in sqlparse causes Impala shell to go to infinite loop when a statement contains a new line inside double quotes. The patch in sqlparse is based on the upstream fix at https://github.com/andialbrecht/sqlparse/pull/396 Testing: - Added new end-to-end shell tests - Ran end-to-end shell tests Change-Id: I9142f21a888189d351f00ce09baeba123bc0959b Reviewed-on: http://gerrit.cloudera.org:8080/9195 Reviewed-by: David KnuppTested-by: Impala Public Jenkins --- M shell/ext-py/sqlparse-0.1.19/sqlparse/lexer.py M shell/ext-py/sqlparse-0.1.19/tests/test_split.py M tests/shell/test_shell_interactive.py 3 files changed, 25 insertions(+), 1 deletion(-) Approvals: David Knupp: Looks good to me, approved Impala Public Jenkins: Verified -- 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: merged Gerrit-Change-Id: I9142f21a888189d351f00ce09baeba123bc0959b Gerrit-Change-Number: 9195 Gerrit-PatchSet: 14 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9195 ) Change subject: IMPALA-6337: Fix infinite loop in Impala shell .. Patch Set 13: Verified+1 -- 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: 13 Gerrit-Owner: Fredy WijayaGerrit-Reviewer: David Knupp Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Sat, 12 May 2018 19:29:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9195 ) Change subject: IMPALA-6337: Fix infinite loop in Impala shell .. Patch Set 13: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2461/ -- 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: 13 Gerrit-Owner: Fredy WijayaGerrit-Reviewer: David Knupp Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Sat, 12 May 2018 15:58:18 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
David Knupp has posted comments on this change. ( http://gerrit.cloudera.org:8080/9195 ) Change subject: IMPALA-6337: Fix infinite loop in Impala shell .. Patch Set 13: Code-Review+2 Thanks for upgrading sqlparse, Fredy! -- 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: 13 Gerrit-Owner: Fredy WijayaGerrit-Reviewer: David Knupp Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Sat, 12 May 2018 15:57:11 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
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 13: > Patch Set 12: > > (1 comment) > > > 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? > > I think upgrading first makes the most sense to me. > > And thanks for clarifying my other questions. The backported bugfix is now based on the upgraded sqlparse-0.1.19. Please re-review this CR again. -- 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: 13 Gerrit-Owner: Fredy WijayaGerrit-Reviewer: David Knupp Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 10 May 2018 23:26:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
Fredy Wijaya has uploaded a new patch set (#13). ( http://gerrit.cloudera.org:8080/9195 ) Change subject: IMPALA-6337: Fix infinite loop in Impala shell .. IMPALA-6337: Fix infinite loop in Impala shell This patch fixes a bug in sqlparse where sqlparse incorrectly splits a statement that has a new line inside double quotes. The bug in sqlparse causes Impala shell to go to infinite loop when a statement contains a new line inside double quotes. The patch in sqlparse is based on the upstream fix at https://github.com/andialbrecht/sqlparse/pull/396 Testing: - Added new end-to-end shell tests - Ran end-to-end shell tests Change-Id: I9142f21a888189d351f00ce09baeba123bc0959b --- M shell/ext-py/sqlparse-0.1.19/sqlparse/lexer.py M shell/ext-py/sqlparse-0.1.19/tests/test_split.py M tests/shell/test_shell_interactive.py 3 files changed, 25 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/9195/13 -- 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: newpatchset Gerrit-Change-Id: I9142f21a888189d351f00ce09baeba123bc0959b Gerrit-Change-Number: 9195 Gerrit-PatchSet: 13 Gerrit-Owner: Fredy WijayaGerrit-Reviewer: David Knupp Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
David Knupp 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: (1 comment) > 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? I think upgrading first makes the most sense to me. And thanks for clarifying my other questions. 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. Furt Done -- 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 WijayaGerrit-Reviewer: David Knupp Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 08 May 2018 16:17:11 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
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 WijayaGerrit-Reviewer: David Knupp Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Sat, 05 May 2018 15:34:25 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
David Knupp 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: > 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). -- 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 WijayaGerrit-Reviewer: David Knupp Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 04 May 2018 21:40:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
David Knupp 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 10: > > (2 comments) Since we're going ahead with this change, can you reply to this older comment? Do you think there's any merit to the idea of doing error checking in our own code and bailing with an error message, rather than shipping a patched version of sqlparse to workaround the issue? 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 String.Symbol, when the nearly identical line above specifies String.Single? 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: > The problem seems to be in the stated assumption: "Error tokens are assumed Since we're going ahead with this change now, can you reply to this older comment? Do you think there's any merit to the idea of doing error checking in our own code and bailing with an error message, rather than shipping a patched version of sqlparse to workaround the issue? -- 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 WijayaGerrit-Reviewer: David Knupp Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 04 May 2018 21:37:52 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
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: > 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. -- 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 WijayaGerrit-Reviewer: David Knupp Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 04 May 2018 02:48:25 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
Fredy Wijaya has uploaded a new patch set (#12). ( http://gerrit.cloudera.org:8080/9195 ) Change subject: IMPALA-6337: Fix infinite loop in Impala shell .. IMPALA-6337: Fix infinite loop in Impala shell This patch fixes a bug in sqlparse where sqlparse incorrectly splits a statement that has a new line inside double quotes. The bug in sqlparse causes Impala shell to go to infinite loop when a statement contains a new line inside double quotes. The patch in sqlparse is based on the upstream fix at https://github.com/andialbrecht/sqlparse/pull/396 Testing: - Added new end-to-end shell tests - Ran end-to-end shell tests Change-Id: I9142f21a888189d351f00ce09baeba123bc0959b --- M shell/ext-py/sqlparse-0.1.14/sqlparse/lexer.py M shell/ext-py/sqlparse-0.1.14/tests/test_split.py M tests/shell/test_shell_interactive.py 3 files changed, 25 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/9195/12 -- 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: newpatchset Gerrit-Change-Id: I9142f21a888189d351f00ce09baeba123bc0959b Gerrit-Change-Number: 9195 Gerrit-PatchSet: 12 Gerrit-Owner: Fredy WijayaGerrit-Reviewer: David Knupp Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/9195 ) Change subject: IMPALA-6337: Fix infinite loop in Impala shell .. Patch Set 11: Fredy, Can you leave pointers in the patch code to indicate that it's fixed upstream (and where-ish)? -- 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: 11 Gerrit-Owner: Fredy WijayaGerrit-Reviewer: David Knupp Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 04 May 2018 00:00:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/9195 ) Change subject: IMPALA-6337: Fix infinite loop in Impala shell .. Patch Set 11: > how much longer will we realistically commit ourselves to being compatible > with python 2.6? Fore the foreseeable future. I'm aware of many folks using Impala on RH6 and friends and planning to stay there. -- 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: 11 Gerrit-Owner: Fredy WijayaGerrit-Reviewer: David Knupp Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 03 May 2018 23:59:33 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
David Knupp has posted comments on this change. ( http://gerrit.cloudera.org:8080/9195 ) Change subject: IMPALA-6337: Fix infinite loop in Impala shell .. Patch Set 11: > Patch Set 11: > > > Patch Set 11: > > > > If there's a bug in sqlparse, have we filed it with sqlparse and tried to > > fix it upstream? > > > > I think David's concern is with having patches rather than upgrading. I > > think we can upgrade if we need to. Maintaining a "vendored"/shaded copy > > would be annoying but also possible if we absolutely need the pathes. > > The fix is already upstream, but it's not released yet and it may require at > minimum Python 2.7 for us to upgrade, which I don't know if it's an option > for us. The sqlparse version that we use is pretty old. Phil's right about my concerns. I further questions would be: - how much longer will we realistically commit ourselves to being compatible with python 2.6? - if we in fact are going to bundle a "vendored" copy of sqlparse, should we base it on the version we assume elsewhere -- 0.1.19 (the last version to support python 2.6)? -- 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: 11 Gerrit-Owner: Fredy WijayaGerrit-Reviewer: David Knupp Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 03 May 2018 23:28:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
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 11: > Patch Set 11: > > If there's a bug in sqlparse, have we filed it with sqlparse and tried to fix > it upstream? > > I think David's concern is with having patches rather than upgrading. I think > we can upgrade if we need to. Maintaining a "vendored"/shaded copy would be > annoying but also possible if we absolutely need the pathes. The fix is already upstream, but it's not released yet and it may require at minimum Python 2.7 for us to upgrade, which I don't know if it's an option for us. The sqlparse version that we use is pretty old. -- 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: 11 Gerrit-Owner: Fredy WijayaGerrit-Reviewer: David Knupp Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 03 May 2018 21:45:47 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
Philip Zeyliger has posted comments on this change. ( http://gerrit.cloudera.org:8080/9195 ) Change subject: IMPALA-6337: Fix infinite loop in Impala shell .. Patch Set 11: If there's a bug in sqlparse, have we filed it with sqlparse and tried to fix it upstream? I think David's concern is with having patches rather than upgrading. I think we can upgrade if we need to. Maintaining a "vendored"/shaded copy would be annoying but also possible if we absolutely need the pathes. -- 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: 11 Gerrit-Owner: Fredy WijayaGerrit-Reviewer: David Knupp Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 03 May 2018 21:36:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
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 11: > Patch Set 11: > > > Patch Set 11: > > > > > Do you know if this bug exists in 0.1.19? We've actually switched to > > > > that version elsewhere in the Impala python infra. > > > https://github.com/apache/impala/blob/master/infra/python/deps/requirements.txt#L65 > > > > Yes, the bug still exists in 0.1.19. However, depending how we use it, > > there may not be a real issue in Python infra. The infinite loop in Impala > > shell is a specific case because of the way we use sqlparse.split. > > Here's my concern. Work has already started on turning the impala-shell into > an actual python package for distributing through PyPI. See: > > https://gerrit.cloudera.org/c/9771/ > https://issues.apache.org/jira/browse/IMPALA-1071 > https://issues.apache.org/jira/browse/IMPALA-6808 > > With that change, we probably won't be relying on an internally-bundled > version of sqlparse anymore, especially if we're uploading impala-shell to > PyPI. Since we've hit several issues with sqlparse and upgrading may not be an easy option, do you think it's better to bundle our own sqlparse even with distributable impala-shell similar to how we deal with native-toolchain? -- 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: 11 Gerrit-Owner: Fredy WijayaGerrit-Reviewer: David Knupp Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 03 May 2018 01:27:18 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
David Knupp has posted comments on this change. ( http://gerrit.cloudera.org:8080/9195 ) Change subject: IMPALA-6337: Fix infinite loop in Impala shell .. Patch Set 11: > Patch Set 11: > > > Do you know if this bug exists in 0.1.19? We've actually switched to > that > > version elsewhere in the Impala python infra. > > https://github.com/apache/impala/blob/master/infra/python/deps/requirements.txt#L65 > > Yes, the bug still exists in 0.1.19. However, depending how we use it, there > may not be a real issue in Python infra. The infinite loop in Impala shell is > a specific case because of the way we use sqlparse.split. Here's my concern. Work has already started on turning the impala-shell into an actual python package for distributing through PyPI. See: https://gerrit.cloudera.org/c/9771/ https://issues.apache.org/jira/browse/IMPALA-1071 https://issues.apache.org/jira/browse/IMPALA-6808 With that change, we probably won't be relying on an internally-bundled version of sqlparse anymore, especially if we're uploading impala-shell to PyPI. -- 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: 11 Gerrit-Owner: Fredy WijayaGerrit-Reviewer: David Knupp Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 03 May 2018 00:41:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
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 11: > Do you know if this bug exists in 0.1.19? We've actually switched to > that > version elsewhere in the Impala python infra. > https://github.com/apache/impala/blob/master/infra/python/deps/requirements.txt#L65 Yes, the bug still exists in 0.1.19. However, depending how we use it, there may not be a real issue in Python infra. The infinite loop in Impala shell is a specific case because of the way we use sqlparse.split. -- 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: 11 Gerrit-Owner: Fredy WijayaGerrit-Reviewer: David Knupp Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 01 May 2018 13:25:39 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
David Knupp has posted comments on this change. ( http://gerrit.cloudera.org:8080/9195 ) Change subject: IMPALA-6337: Fix infinite loop in Impala shell .. Patch Set 11: > Patch Set 11: > > I just realized we can roll out a bug fix in shell/ext-py/sqlparse-0.0.14. > The patch in sqlparse-0.1.14 is based on > https://github.com/andialbrecht/sqlparse/pull/396 with slight modification to > work on sqlparse-0.0.14. Do you know if this bug exists in 0.1.19? We've actually switched to that version elsewhere in the Impala python infra. https://github.com/apache/impala/blob/master/infra/python/deps/requirements.txt#L65 -- 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: 11 Gerrit-Owner: Fredy WijayaGerrit-Reviewer: David Knupp Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 01 May 2018 08:21:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
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 11: I just realized we can roll out a bug fix in shell/ext-py/sqlparse-0.0.14. The patch in sqlparse-0.1.14 is based on https://github.com/andialbrecht/sqlparse/pull/396 with slight modification to work on sqlparse-0.0.14. -- 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: 11 Gerrit-Owner: Fredy WijayaGerrit-Reviewer: David Knupp Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 01 May 2018 04:02:03 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
Fredy Wijaya has uploaded a new patch set (#11). ( http://gerrit.cloudera.org:8080/9195 ) Change subject: IMPALA-6337: Fix infinite loop in Impala shell .. IMPALA-6337: Fix infinite loop in Impala shell This patch fixes a bug in sqlparse where sqlparse incorrectly splits a statement that has a new line inside double quotes. The bug in sqlparse causes Impala shell to go to infinite loop when a statement contains a new line inside double quotes. Testing: - Added new end-to-end shell tests - Ran end-to-end shell tests Change-Id: I9142f21a888189d351f00ce09baeba123bc0959b --- M shell/ext-py/sqlparse-0.1.14/sqlparse/lexer.py M shell/ext-py/sqlparse-0.1.14/tests/test_split.py M tests/shell/test_shell_interactive.py 3 files changed, 24 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/9195/11 -- 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: newpatchset Gerrit-Change-Id: I9142f21a888189d351f00ce09baeba123bc0959b Gerrit-Change-Number: 9195 Gerrit-PatchSet: 11 Gerrit-Owner: Fredy WijayaGerrit-Reviewer: David Knupp Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
Fredy Wijaya has restored this change. ( http://gerrit.cloudera.org:8080/9195 ) Change subject: IMPALA-6337: Fix infinite loop in Impala shell .. Restored -- 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: restore Gerrit-Change-Id: I9142f21a888189d351f00ce09baeba123bc0959b Gerrit-Change-Number: 9195 Gerrit-PatchSet: 10 Gerrit-Owner: Fredy WijayaGerrit-Reviewer: David Knupp Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/9195 ) Change subject: IMPALA-6337: Fix infinite loop in Impala shell .. Patch Set 10: if we're not going forward with this change, pls abandon it. -- 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: 10 Gerrit-Owner: Fredy WijayaGerrit-Reviewer: David Knupp Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Mon, 02 Apr 2018 16:50:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
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 10: > Patch Set 10: > > (2 comments) I decided to fix the bug in sqlparse instead: https://github.com/andialbrecht/sqlparse/pull/396 -- 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: 10 Gerrit-Owner: Fredy WijayaGerrit-Reviewer: David Knupp Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 09 Mar 2018 20:48:29 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
David Knupp has posted comments on this change. ( http://gerrit.cloudera.org:8080/9195 ) Change subject: IMPALA-6337: Fix infinite loop in Impala shell .. Patch Set 10: (2 comments) 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@588 PS9, Line 588: for index in xrange(0, len(parsed_statements)): > It probably is faster, but I don't think performance should be a concern in For what it's worth, I think enumerate is either as fast or faster than xrange. Python built-ins tend to be highly optimized. And either way, I think that this is clearly a case where the benefit of enhanced readability far outweighs any infinitesimal performance gains. http://gerrit.cloudera.org:8080/#/c/9195/9/shell/impala_shell.py@601 PS9, Line 601: if not found_error: > After spending some time think about this issue, I don't think there's an e The problem seems to be in the stated assumption: "Error tokens are assumed to be generated as pairs." Rather than trying to workaround the issue, I wonder if this may be a case where the best course of action is not to try to infer the intent of malformed SQL, but rather to bail gracefully with explicit messages. E.g., one could add something like: @staticmethod def is_valid_sql(sql): """Use sqlparse module to validate that sql doesn't contain any errors. Returns: True if no errors found, else return False. """ parsed = sqlparse.parse(sql) for parsed_stmt in parsed: if any(tok.ttype == sqlparse.tokens.Error for tok in parsed_stmt.tokens): return False return True And then use this in precmd() like: parsed_cmds = sqlparse.split(args) if not all(ImpalaShell.is_valid_sql(cmd) for cmd in parsed_cmds): # Probably output args + a message to STDOUT, and then take # action appropriate to whether self.interactive is True or False else: if len(parsed_cmds) > 1: # etc... -- 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: 10 Gerrit-Owner: Fredy WijayaGerrit-Reviewer: David Knupp Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 07 Mar 2018 19:19:44 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
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 10: (1 comment) 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: if not found_error: > Ack After spending some time think about this issue, I don't think there's an easy way to workaround the issue other than having the sqlparse library fixed. I'm thinking of abandoning this review. -- 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: 10 Gerrit-Owner: Fredy WijayaGerrit-Reviewer: David Knupp Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 07 Mar 2018 16:14:32 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
Taras Bobrovytsky has posted comments on this change. ( http://gerrit.cloudera.org:8080/9195 ) Change subject: IMPALA-6337: Fix infinite loop in Impala shell .. Patch Set 9: (1 comment) 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@588 PS9, Line 588: for index in xrange(0, len(parsed_statements)): > Isn't xrange is faster in Python 2.7 compared to enumerate? It probably is faster, but I don't think performance should be a concern in this case. If you want to keep xrange(), then you can get rid of the first argument. xrange(len(parsed_statements)) -- 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: 9 Gerrit-Owner: Fredy WijayaGerrit-Reviewer: David Knupp Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 07 Mar 2018 00:24:20 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
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 10: (2 comments) 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@588 PS9, Line 588: for index in xrange(0, len(parsed_statements)): > enumerate* Isn't xrange is faster in Python 2.7 compared to enumerate? http://gerrit.cloudera.org:8080/#/c/9195/9/shell/impala_shell.py@601 PS9, Line 601: if not found_error: > I tried it, it looks like it throws away the middle "not error" statement. Ack -- 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: 10 Gerrit-Owner: Fredy WijayaGerrit-Reviewer: David Knupp Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 07 Mar 2018 00:20:16 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
Taras Bobrovytsky has posted comments on this change. ( http://gerrit.cloudera.org:8080/9195 ) Change subject: IMPALA-6337: Fix infinite loop in Impala shell .. Patch Set 9: (1 comment) 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: if not found_error: > Ok, but what will happen if the list looks like the following: I tried it, it looks like it throws away the middle "not error" statement. I think that's weird behavior. I tried it with this query in impala-shell: select 1; \; select 2; \; select 3; "Select 2" vanishes -- 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: 9 Gerrit-Owner: Fredy WijayaGerrit-Reviewer: David Knupp Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Wed, 07 Mar 2018 00:16:47 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
Taras Bobrovytsky has posted comments on this change. ( http://gerrit.cloudera.org:8080/9195 ) Change subject: IMPALA-6337: Fix infinite loop in Impala shell .. Patch Set 9: (1 comment) 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@588 PS9, Line 588: for index in xrange(0, len(parsed_statements)): > for index, statement in enumate(parsed_statements) enumerate* -- 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: 9 Gerrit-Owner: Fredy WijayaGerrit-Reviewer: David Knupp Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 06 Mar 2018 23:45:14 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
Taras Bobrovytsky has posted comments on this change. ( http://gerrit.cloudera.org:8080/9195 ) Change subject: IMPALA-6337: Fix infinite loop in Impala shell .. Patch Set 9: (1 comment) 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@588 PS9, Line 588: for index in xrange(0, len(parsed_statements)): for index, statement in enumate(parsed_statements) -- 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: 9 Gerrit-Owner: Fredy WijayaGerrit-Reviewer: David Knupp Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 06 Mar 2018 23:44:28 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
Taras Bobrovytsky has posted comments on this change. ( http://gerrit.cloudera.org:8080/9195 ) Change subject: IMPALA-6337: Fix infinite loop in Impala shell .. Patch Set 10: (1 comment) 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: if not found_error: > When found_error is true at the very end (implied by having non empty joine Ok, but what will happen if the list looks like the following: not error, error, not error, error, not error Also, maybe add this to the test cases? -- 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: 10 Gerrit-Owner: Fredy WijayaGerrit-Reviewer: David Knupp Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 06 Mar 2018 23:42:32 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
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 10: Code-Review+1 Carrying Vuk's +1. -- 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: 10 Gerrit-Owner: Fredy WijayaGerrit-Reviewer: David Knupp Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 06 Mar 2018 23:33:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
Fredy Wijaya has uploaded a new patch set (#10). ( http://gerrit.cloudera.org:8080/9195 ) Change subject: IMPALA-6337: Fix infinite loop in Impala shell .. IMPALA-6337: Fix infinite loop in Impala shell The patch uses a workaround for a bug in the sqlparse.split() function by joining the statements that contain error tokens into a single statement Testing: - Ran end-to-end shell tests Change-Id: I9142f21a888189d351f00ce09baeba123bc0959b --- M shell/impala_shell.py M tests/shell/test_shell_interactive.py 2 files changed, 59 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/9195/10 -- 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: newpatchset Gerrit-Change-Id: I9142f21a888189d351f00ce09baeba123bc0959b Gerrit-Change-Number: 9195 Gerrit-PatchSet: 10 Gerrit-Owner: Fredy WijayaGerrit-Reviewer: David Knupp Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
Taras Bobrovytsky has posted comments on this change. ( http://gerrit.cloudera.org:8080/9195 ) Change subject: IMPALA-6337: Fix infinite loop in Impala shell .. Patch Set 9: (2 comments) 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@590 PS9, Line 590: filter It's not very Pythonic to use the filter function. The following is much more Pythonic: error_count = len([tok for tok in statement.tokens if tok.ttype == tokens.Error]) http://gerrit.cloudera.org:8080/#/c/9195/9/shell/impala_shell.py@601 PS9, Line 601: if not found_error: What if found error is true here? Then something must have gone wrong, right? Maybe return split_statements in that case? -- 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: 9 Gerrit-Owner: Fredy WijayaGerrit-Reviewer: David Knupp Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Taras Bobrovytsky Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Tue, 06 Mar 2018 22:12:21 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/9195 ) Change subject: IMPALA-6337: Fix infinite loop in Impala shell .. Patch Set 9: Code-Review+1 lgtm. would anyone with more experience with the shell/python care to have a look at this patch? -- 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: 9 Gerrit-Owner: Fredy WijayaGerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 16 Feb 2018 20:02:17 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
Fredy Wijaya has uploaded a new patch set (#9). ( http://gerrit.cloudera.org:8080/9195 ) Change subject: IMPALA-6337: Fix infinite loop in Impala shell .. IMPALA-6337: Fix infinite loop in Impala shell The patch uses a workaround for a bug in the sqlparse.split() function by joining the statements that contain error tokens into a single statement Testing: - Ran end-to-end shell tests Change-Id: I9142f21a888189d351f00ce09baeba123bc0959b --- M shell/impala_shell.py M tests/shell/test_shell_interactive.py 2 files changed, 59 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/9195/9 -- 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: newpatchset Gerrit-Change-Id: I9142f21a888189d351f00ce09baeba123bc0959b Gerrit-Change-Number: 9195 Gerrit-PatchSet: 9 Gerrit-Owner: Fredy WijayaGerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/9195 ) Change subject: IMPALA-6337: Fix infinite loop in Impala shell .. Patch Set 8: (2 comments) http://gerrit.cloudera.org:8080/#/c/9195/8/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/9195/8/shell/impala_shell.py@580 PS8, Line 580: select repeated 'select'? http://gerrit.cloudera.org:8080/#/c/9195/8/tests/shell/test_shell_interactive.py File tests/shell/test_shell_interactive.py: http://gerrit.cloudera.org:8080/#/c/9195/8/tests/shell/test_shell_interactive.py@482 PS8, Line 482: # IMPALA-6337: Fix infinite loop. add the simpler example from the jira -- 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: 8 Gerrit-Owner: Fredy WijayaGerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Fri, 16 Feb 2018 19:26:58 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
Fredy Wijaya has uploaded a new patch set (#8). ( http://gerrit.cloudera.org:8080/9195 ) Change subject: IMPALA-6337: Fix infinite loop in Impala shell .. IMPALA-6337: Fix infinite loop in Impala shell The patch uses a workaround for a bug in the sqlparse.split() function by joining the statements that contain error tokens into a single statement Testing: - Ran end-to-end shell tests Change-Id: I9142f21a888189d351f00ce09baeba123bc0959b --- M shell/impala_shell.py M tests/shell/test_shell_interactive.py 2 files changed, 59 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/9195/8 -- 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: newpatchset Gerrit-Change-Id: I9142f21a888189d351f00ce09baeba123bc0959b Gerrit-Change-Number: 9195 Gerrit-PatchSet: 8 Gerrit-Owner: Fredy WijayaGerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
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 7: (2 comments) http://gerrit.cloudera.org:8080/#/c/9195/3/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/9195/3/shell/impala_shell.py@579 PS3, Line 579: > this is returned only when errors come in pairs. otherwise, the joined_stat Done http://gerrit.cloudera.org:8080/#/c/9195/3/tests/shell/test_shell_interactive.py File tests/shell/test_shell_interactive.py: http://gerrit.cloudera.org:8080/#/c/9195/3/tests/shell/test_shell_interactive.py@482 PS3, Line 482: # IMPALA-6337: Fix infinite loop. > start off with simplest first, perhaps include the simpler repro from the j Done -- 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: 7 Gerrit-Owner: Fredy WijayaGerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 15 Feb 2018 19:14:29 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
Fredy Wijaya has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/9195 ) Change subject: IMPALA-6337: Fix infinite loop in Impala shell .. IMPALA-6337: Fix infinite loop in Impala shell The patch uses a workaround for a bug in the sqlparse.split() function by joining the statements that contain error tokens into a single statement Testing: - Ran end-to-end shell tests Change-Id: I9142f21a888189d351f00ce09baeba123bc0959b --- M shell/impala_shell.py M tests/shell/test_shell_interactive.py 2 files changed, 59 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/9195/7 -- 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: newpatchset Gerrit-Change-Id: I9142f21a888189d351f00ce09baeba123bc0959b Gerrit-Change-Number: 9195 Gerrit-PatchSet: 7 Gerrit-Owner: Fredy WijayaGerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/9195 ) Change subject: IMPALA-6337: Fix infinite loop in Impala shell .. Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/9195/3/shell/impala_shell.py File shell/impala_shell.py: http://gerrit.cloudera.org:8080/#/c/9195/3/shell/impala_shell.py@579 PS3, Line 579: joined_statement this is returned only when errors come in pairs. otherwise, the joined_statement will be dropped. pls clarify the assumptions about what parsed_statements contain. http://gerrit.cloudera.org:8080/#/c/9195/3/tests/shell/test_shell_interactive.py File tests/shell/test_shell_interactive.py: http://gerrit.cloudera.org:8080/#/c/9195/3/tests/shell/test_shell_interactive.py@482 PS3, Line 482: # IMPALA-6337: Fix infinite loop. start off with simplest first, perhaps include the simpler repro from the jira. -- 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: 3 Gerrit-Owner: Fredy WijayaGerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 15 Feb 2018 18:13:59 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
Fredy Wijaya has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/9195 ) Change subject: IMPALA-6337: Fix infinite loop in Impala shell .. IMPALA-6337: Fix infinite loop in Impala shell The patch uses a workaround for a bug in the sqlparse.split() function by joining the statements that contain error tokens into a single statement Testing: - Ran end-to-end shell tests Change-Id: I9142f21a888189d351f00ce09baeba123bc0959b --- M shell/impala_shell.py M tests/shell/test_shell_interactive.py 2 files changed, 42 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/9195/3 -- 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: newpatchset Gerrit-Change-Id: I9142f21a888189d351f00ce09baeba123bc0959b Gerrit-Change-Number: 9195 Gerrit-PatchSet: 3 Gerrit-Owner: Fredy Wijaya
[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell
Fredy Wijaya has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/9195 ) Change subject: IMPALA-6337: Fix infinite loop in Impala shell .. IMPALA-6337: Fix infinite loop in Impala shell The patch uses a workaround for a bug in the sqlparse.split() function by joining the statements that contain error tokens into a single statement Change-Id: I9142f21a888189d351f00ce09baeba123bc0959b --- M shell/impala_shell.py M tests/shell/test_shell_interactive.py 2 files changed, 42 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/95/9195/2 -- 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: newpatchset Gerrit-Change-Id: I9142f21a888189d351f00ce09baeba123bc0959b Gerrit-Change-Number: 9195 Gerrit-PatchSet: 2 Gerrit-Owner: Fredy Wijaya