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

Change subject: IMPALA-7905: Hive keywords not quoted for identifiers
......................................................................


Patch Set 1:

(2 comments)

Just couple nits, but it looks good overall.

http://gerrit.cloudera.org:8080/#/c/12009/1/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java
File fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java:

http://gerrit.cloudera.org:8080/#/c/12009/1/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@126
PS1, Line 126: Token t = hiveLexer.nextToken();
Will hiveLexer.nextToken() returns null when there's no token? We should either 
do a null check or add Preconditions if we're sure it never returns a null. 
Similarly with L132.


http://gerrit.cloudera.org:8080/#/c/12009/1/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@149
PS1, Line 149: // Impala's scanner recognizes the ".123" portion of 
"db.123_tbl" as a decimal,
             :     // so while the quoting is not necessary for the given 
identifier itself, the quotes
             :     // are needed if this identifier will be preceded by a ".".
This is no longer an issue in Impala: https://gerrit.cloudera.org/c/11927/. I'm 
okay with quoting it since db.3e12 still needs to be quoted.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I06cc20b052a3a66535a171c36b4b31477c0ba6d0
Gerrit-Change-Number: 12009
Gerrit-PatchSet: 1
Gerrit-Owner: Paul Rogers <[email protected]>
Gerrit-Reviewer: Fredy Wijaya <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Paul Rogers <[email protected]>
Gerrit-Comment-Date: Thu, 13 Dec 2018 19:47:15 +0000
Gerrit-HasComments: Yes

Reply via email to