Dimitris Tsirogiannis has posted comments on this change. Change subject: IMPALA-3726: Add support for Kudu-specific column options ......................................................................
Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/5026/1/fe/src/main/cup/sql-parser.cup File fe/src/main/cup/sql-parser.cup: Line 250: KW_DATE, KW_DATETIME, KW_DECIMAL, KW_DEFAULT, KW_DELETE, KW_DELIMITED, KW_DESC, > Making "default" a keyword sounds bad because of the "default" database. I It's really annoying but I think we should definitely consider making it a KEYWORD for C6. Line 1466: | opt_default_val:default_val > I think we might be able to get away with the usual hack for not making def Changed it to using IDENT. It does remove some flexibility wrt where NOT NULL is specified relative to the other options. http://gerrit.cloudera.org:8080/#/c/5026/1/fe/src/test/java/org/apache/impala/analysis/ParserTest.java File fe/src/test/java/org/apache/impala/analysis/ParserTest.java: Line 934: ParsesOk("select a from `default`.`t`"); > :( Removed. -- To view, visit http://gerrit.cloudera.org:8080/5026 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I727b9ae1b7b2387db752b58081398dd3f3449c02 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-HasComments: Yes
