[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell

2018-05-12 Thread Impala Public Jenkins (Code Review)
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 Knupp 
Tested-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

2018-05-12 Thread Impala Public Jenkins (Code Review)
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 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 
Gerrit-Comment-Date: Sat, 12 May 2018 19:29:31 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell

2018-05-12 Thread Impala Public Jenkins (Code Review)
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 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 
Gerrit-Comment-Date: Sat, 12 May 2018 15:58:18 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell

2018-05-12 Thread David Knupp (Code Review)
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 Wijaya 
Gerrit-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

2018-05-10 Thread Fredy Wijaya (Code Review)
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 Wijaya 
Gerrit-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

2018-05-10 Thread Fredy Wijaya (Code Review)
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 Wijaya 
Gerrit-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

2018-05-08 Thread David Knupp (Code Review)
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 Wijaya 
Gerrit-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

2018-05-05 Thread Fredy Wijaya (Code Review)
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 
Gerrit-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

2018-05-04 Thread David Knupp (Code Review)
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 Wijaya 
Gerrit-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

2018-05-04 Thread David Knupp (Code Review)
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 Wijaya 
Gerrit-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

2018-05-03 Thread Fredy Wijaya (Code Review)
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 Wijaya 
Gerrit-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

2018-05-03 Thread Fredy Wijaya (Code Review)
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 Wijaya 
Gerrit-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

2018-05-03 Thread Philip Zeyliger (Code Review)
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 Wijaya 
Gerrit-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

2018-05-03 Thread Philip Zeyliger (Code Review)
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 Wijaya 
Gerrit-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

2018-05-03 Thread David Knupp (Code Review)
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 Wijaya 
Gerrit-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

2018-05-03 Thread Fredy Wijaya (Code Review)
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 Wijaya 
Gerrit-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

2018-05-03 Thread Philip Zeyliger (Code Review)
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 Wijaya 
Gerrit-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

2018-05-02 Thread Fredy Wijaya (Code Review)
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 Wijaya 
Gerrit-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

2018-05-02 Thread David Knupp (Code Review)
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 Wijaya 
Gerrit-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

2018-05-01 Thread Fredy Wijaya (Code Review)
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 Wijaya 
Gerrit-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

2018-05-01 Thread David Knupp (Code Review)
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 Wijaya 
Gerrit-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

2018-04-30 Thread Fredy Wijaya (Code Review)
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 Wijaya 
Gerrit-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

2018-04-30 Thread Fredy Wijaya (Code Review)
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 Wijaya 
Gerrit-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

2018-04-30 Thread Fredy Wijaya (Code Review)
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 Wijaya 
Gerrit-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

2018-04-02 Thread Vuk Ercegovac (Code Review)
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 Wijaya 
Gerrit-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

2018-03-09 Thread Fredy Wijaya (Code Review)
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 Wijaya 
Gerrit-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

2018-03-07 Thread David Knupp (Code Review)
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 Wijaya 
Gerrit-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

2018-03-07 Thread Fredy Wijaya (Code Review)
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 Wijaya 
Gerrit-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

2018-03-06 Thread Taras Bobrovytsky (Code Review)
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 Wijaya 
Gerrit-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

2018-03-06 Thread Fredy Wijaya (Code Review)
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 Wijaya 
Gerrit-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

2018-03-06 Thread Taras Bobrovytsky (Code Review)
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 Wijaya 
Gerrit-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

2018-03-06 Thread Taras Bobrovytsky (Code Review)
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 Wijaya 
Gerrit-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

2018-03-06 Thread Taras Bobrovytsky (Code Review)
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 Wijaya 
Gerrit-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

2018-03-06 Thread Taras Bobrovytsky (Code Review)
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 Wijaya 
Gerrit-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

2018-03-06 Thread Fredy Wijaya (Code Review)
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 Wijaya 
Gerrit-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

2018-03-06 Thread Fredy Wijaya (Code Review)
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 Wijaya 
Gerrit-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

2018-03-06 Thread Taras Bobrovytsky (Code Review)
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 Wijaya 
Gerrit-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

2018-02-16 Thread Vuk Ercegovac (Code Review)
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 Wijaya 
Gerrit-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

2018-02-16 Thread Fredy Wijaya (Code Review)
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 Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac 


[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell

2018-02-16 Thread Vuk Ercegovac (Code Review)
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 Wijaya 
Gerrit-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

2018-02-15 Thread Fredy Wijaya (Code Review)
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 Wijaya 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac 


[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell

2018-02-15 Thread Fredy Wijaya (Code Review)
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 Wijaya 
Gerrit-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

2018-02-15 Thread Fredy Wijaya (Code Review)
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 Wijaya 
Gerrit-Reviewer: Vuk Ercegovac 


[Impala-ASF-CR] IMPALA-6337: Fix infinite loop in Impala shell

2018-02-15 Thread Vuk Ercegovac (Code Review)
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 Wijaya 
Gerrit-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

2018-02-15 Thread Fredy Wijaya (Code Review)
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

2018-02-02 Thread Fredy Wijaya (Code Review)
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