Philip Zeyliger has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9096 )

Change subject: IMPALA-3916: Reserve SQL:2016 reserved words
......................................................................


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/9096/1/be/src/common/global-flags.cc
File be/src/common/global-flags.cc:

http://gerrit.cloudera.org:8080/#/c/9096/1/be/src/common/global-flags.cc@193
PS1, Line 193: DEFINE_bool(reserve_impala3_words, true, "Reserve impala 3 
reserved words.");
I think we'll want to change this list again, so we may want to leave ourselves 
some wiggle room.

Perhaps:

--reserved_words_list=[IMPALA2.11,IMPALA3.0], with default=IMPALA3.0?

By making it string-valued (or, really, enum-valued), we have the potential to 
do IMPALA4.0 or IMPALA3.3 in the future. I don't think this will complicate 
your parsing code significantly.


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

http://gerrit.cloudera.org:8080/#/c/9096/1/fe/src/main/cup/sql-parser.cup@301
PS1, Line 301: // A word is an arbitrary token composed of digits and at least 
one letter.
Is there documentation here you might want to add about how all the keyword 
stuff is handled?

Thanks!


http://gerrit.cloudera.org:8080/#/c/9096/1/fe/src/main/jflex/sql-scanner.flex
File fe/src/main/jflex/sql-scanner.flex:

http://gerrit.cloudera.org:8080/#/c/9096/1/fe/src/main/jflex/sql-scanner.flex@62
PS1, Line 62:   static final HashSet<String> sql16ReservedWords = new 
HashSet<>(Arrays.asList(
Are our lists of reserved words now effectively split between sql-parser.cup 
(in its KW_* list, lines 251-279) and here? Should we have them all in one 
place, one way or another?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If1b295e6a77e840cf1b794c2eb73e1b9d2b8ddd6
Gerrit-Change-Number: 9096
Gerrit-PatchSet: 1
Gerrit-Owner: Tianyi Wang <[email protected]>
Gerrit-Reviewer: Philip Zeyliger <[email protected]>
Gerrit-Comment-Date: Tue, 23 Jan 2018 19:49:37 +0000
Gerrit-HasComments: Yes

Reply via email to