Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16599 )
Change subject: IMPALA-10288: Implement DESCRIBE HISTORY for Iceberg tables ...................................................................... Patch Set 3: (1 comment) THis is really cool! I had a concern about reserved words that hopefully isn't too big a deal to address. http://gerrit.cloudera.org:8080/#/c/16599/3/fe/src/main/jflex/sql-scanner.flex File fe/src/main/jflex/sql-scanner.flex: http://gerrit.cloudera.org:8080/#/c/16599/3/fe/src/main/jflex/sql-scanner.flex@152 PS3, Line 152: keywordMap.put("history", SqlParserSymbols.KW_HISTORY); We should be careful with adding new reserved words (it's best to avoid it) because it can cause problems if there are unquoted symbols in existing queries. https://impala.apache.org/docs/build/html/topics/impala_reserved_words.html I think you could avoid adding the keyword if you used an existing keyword (change? commit? versioning?), or if you made it an IDENT token and compared it to "history" on the Java side. -- To view, visit http://gerrit.cloudera.org:8080/16599 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I56a4b92c27e8e4a79109696cbae62735a00750e5 Gerrit-Change-Number: 16599 Gerrit-PatchSet: 3 Gerrit-Owner: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: wangsheng <[email protected]> Gerrit-Comment-Date: Fri, 30 Oct 2020 23:23:19 +0000 Gerrit-HasComments: Yes
