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

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


Patch Set 6: Code-Review+2

(3 comments)

http://gerrit.cloudera.org:8080/#/c/12009/5/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/5/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@116
PS5, Line 116: an identifier
> "ident" is a common abbreviation, and the name of the parameter, but I thin
I meant to refer to the method argument @ident here. But it is fine. Not too 
strong.


http://gerrit.cloudera.org:8080/#/c/12009/5/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@124
PS5, Line 124: upper-case text, but that
> Turns out ANTLRNoCaseStringStream is a non-static inner class inside the Pa
cool, thanks for checking.


http://gerrit.cloudera.org:8080/#/c/12009/5/fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java@148
PS5, Line 148: /
             :   public static boolean impalaNeedsQuotes(String ident) {
             :     return SqlScanner.isReserved(ident) ||
             :       // Impala's scanner recognizes the ".123" portion of 
"db.123e45" as a decimal,
             :       // so while the quoting is not necessary
> Made the || change.
ya, 123e45 is a valid decimal literal but not .123 part like you mentioned (e 
is exponential). Fix the comment?

[localhost:21000] default> select typeof(.123e45);

| typeof(1.23e+44) |
+------------------+
| DOUBLE           |
+------------------+

I guess this is true anything simple like `12345`



--
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: 6
Gerrit-Owner: Paul Rogers <prog...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bhara...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fwij...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Paul Rogers <prog...@cloudera.com>
Gerrit-Comment-Date: Thu, 03 Jan 2019 19:32:19 +0000
Gerrit-HasComments: Yes

Reply via email to