Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/9096 )
Change subject: IMPALA-3916: Reserve SQL:2016 reserved words ...................................................................... Patch Set 5: (2 comments) http://gerrit.cloudera.org:8080/#/c/9096/5/be/src/common/init.cc File be/src/common/init.cc: http://gerrit.cloudera.org:8080/#/c/9096/5/be/src/common/init.cc@205 PS5, Line 205: CLEAN_EXIT_WITH_ERROR(strings::Substitute("Invalid flag reserved_words_version. The " remove strings:: http://gerrit.cloudera.org:8080/#/c/9096/5/fe/src/main/jflex/sql-scanner.flex File fe/src/main/jflex/sql-scanner.flex: http://gerrit.cloudera.org:8080/#/c/9096/5/fe/src/main/jflex/sql-scanner.flex@352 PS5, Line 352: static boolean isReserved(String token) { Sorry to keep going back and forth on this, but the initialization order seems tricky to reason about. I was thinking that we could pre-compute the map of reserved words in a static block, but that might not work depending on when this class is loaded. That's subtle and tricky to reason about. How about we add a static init() function to SqlParser() which gets called from BackendConfig.create(). That way we can pre-compute an effective list of reserved words, and it is explicit that invoking the parser before BackendConfig.create() is called is invalid. -- 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: 5 Gerrit-Owner: Tianyi Wang <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Philip Zeyliger <[email protected]> Gerrit-Reviewer: Tianyi Wang <[email protected]> Gerrit-Comment-Date: Wed, 31 Jan 2018 00:02:54 +0000 Gerrit-HasComments: Yes
