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

Change subject: IMPALA-8497: dealing with query ends with '\n'
......................................................................


Patch Set 5:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/13293/5//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/13293/5//COMMIT_MSG@7
PS5, Line 7: dealing with query ends with '\n'
nit: Fix ArrayIndexOutOfBoundsException for queries that end with '\n'


http://gerrit.cloudera.org:8080/#/c/13293/5//COMMIT_MSG@9
PS5, Line 9: sql
nit: the query


http://gerrit.cloudera.org:8080/#/c/13293/5//COMMIT_MSG@10
PS5, Line 10: syntax error
nit: a syntax error


http://gerrit.cloudera.org:8080/#/c/13293/5//COMMIT_MSG@11
PS5, Line 11: This situation usually occured when sql submit by hive jdbc in 
code,
            : user sometimes have their own BI tools,and may submit sql ends
            : with '\n' to impala server.
nit: The bug only affects the queries submitted directly to Impala outside 
Impala shell.


http://gerrit.cloudera.org:8080/#/c/13293/5/fe/src/main/cup/sql-parser.cup
File fe/src/main/cup/sql-parser.cup:

http://gerrit.cloudera.org:8080/#/c/13293/5/fe/src/main/cup/sql-parser.cup@182
PS5, Line 182: int errorIndex = stmt.endsWith("\n") ? errorToken_.left - 2 : 
errorToken_.left - 1;
i don't think this code is quite correct. I think the code will throw 
ArrayIndexOutOfBoundsException when there are multiple \n at the end?


http://gerrit.cloudera.org:8080/#/c/13293/5/fe/src/test/java/org/apache/impala/analysis/ParserTest.java
File fe/src/test/java/org/apache/impala/analysis/ParserTest.java:

http://gerrit.cloudera.org:8080/#/c/13293/5/fe/src/test/java/org/apache/impala/analysis/ParserTest.java@3440
PS5, Line 3440: "SELECT\n"
can we have another test case for "SELECT\n\n"?


http://gerrit.cloudera.org:8080/#/c/13293/5/fe/src/test/java/org/apache/impala/analysis/ParserTest.java@3445
PS5, Line 3445: Expected: ALL, CASE, CAST, DEFAULT, DISTINCT, EXISTS, FALSE, "
you missed the DATE, i.e.

Expected: ALL, CASE, CAST, DATE, ALL, CASE, CAST, DATE,



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3f034b351d0468a77773f6482e27ddef818b34d8
Gerrit-Change-Number: 13293
Gerrit-PatchSet: 5
Gerrit-Owner: wangsheng <[email protected]>
Gerrit-Reviewer: Austin Nobis <[email protected]>
Gerrit-Reviewer: Fredy Wijaya <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Comment-Date: Tue, 14 May 2019 21:55:33 +0000
Gerrit-HasComments: Yes

Reply via email to