IGNITE-20196 Sql. Review list of reserved keywords#2452
IGNITE-20196 Sql. Review list of reserved keywords#2452korlov42 merged 4 commits intoapache:mainfrom
Conversation
| nonReservedKeywords: [ | ||
| "SEMI" | ||
| # items in this list become non-reserved. | ||
| nonReservedKeywordsToAdd: [ |
There was a problem hiding this comment.
can you explain a bit - what nonReservedKeywordsToAdd means in practice ? How ddl\dml commands interact with them ?
There was a problem hiding this comment.
This patch changes the configuration of FMPP task in order to take an advantage of a default configuration introduced in CALCITE-4238. Having default configuration, any sub parser (as Ignite parser, for instance) may omit redundant configuration, and concentrate only on extensions to the grammar. One of such extensions is a list of keywords that should not be reserved. That is, keywords that can be freely used as identifiers of a table/column/index/whatever without need to be quoted.
A "nonReservedKeywordsToAdd" is an extension point to convert any keyword to non-reserved. Any keyword from this list, as well as any keyword from default_config.fmpp#nonReservedKeywords, can be used as an identifier without quoting.
There was a problem hiding this comment.
thanks, got it ! But i have strange feeling that we can miss here something, look for example :
CASCADE is reserved by standard but ignite allow to create a table with such a name, can we obtain a troubles in future if we want to process this keyword ?
also i find that errors with existing reserved keywords are non informative :
CREATE TABLE INDEX brings :
Caused by: org.apache.ignite.internal.generated.query.calcite.sql.ParseException: Incorrect syntax near the keyword 'TABLE' at line 1, column 8.
Was expecting one of:
"OR" ...
"TABLE" ...
i suppose we need additional issue here, wdyt?
There was a problem hiding this comment.
CASCADE is reserved by standard
I haven't found segregation on reserved vs non-reserved keywords in SQL standard (to be honest, I've spent not much time on this). If you know particular section, I appreciate if you point it out to me
i suppose we need additional issue here, wdyt?
IN fact, we already have such issue. I'm going to address error message in IGNITE-20104
There was a problem hiding this comment.
5.2 <token> and <separator>
section
There was a problem hiding this comment.
wow, that's cool, thanks!
I tried to search for reserved word, but search results were pointing out on a random place at document. Turns out search just got broken because of there were too much options since every page contains All rights reserved at the bottom...
regarding to CASCADE: according to SQL standard 2016, CASCADE is non reserved.
regarding to can we obtain a troubles in future if we want to process this keyword ?: technically, yes. But the list of non reserved keywords is aligned with Postgres and H2, thus I believe there won't be any problem
There was a problem hiding this comment.
I check 2 random reserved words form 2016 standard and it`s passed : SYSTEM_TIME, TRUNCATE probably we need additional issue ? btw they are passed on main too. Plz fix conflicts and move forward.
There was a problem hiding this comment.
I check 2 random reserved words form 2016 standard and it`s passed
I'm going to revise list of (non-)reserved keywords in IGNITE-20104, thus I believe it's better to continue this discussion in the next patch :)
# Conflicts: # modules/runner/src/integrationTest/java/org/apache/ignite/internal/configuration/storage/ItRebalanceDistributedTest.java
3 participants
https://issues.apache.org/jira/browse/IGNITE-20196
Thank you for submitting the pull request.
To streamline the review process of the patch and ensure better code quality
we ask both an author and a reviewer to verify the following:
The Review Checklist
- There is a single JIRA ticket related to the pull request.
- The web-link to the pull request is attached to the JIRA ticket.
- The JIRA ticket has the Patch Available state.
- The description of the JIRA ticket explains WHAT was made, WHY and HOW.
- The pull request title is treated as the final commit message. The following pattern must be used: IGNITE-XXXX Change summary where XXXX - number of JIRA issue.
Notes