-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
sql/parser: Add FAMILY as reserved keyword to avoid ambiguity #7069
sql/parser: Add FAMILY as reserved keyword to avoid ambiguity #7069
Conversation
mechanics but I'm not sure about the choice of
|
Any reason to not make |
@petermattis we discussed that. I argued that as a general matter of PL design we should aim to minimize the number of reserved keywords. Also, "family" seems a rather common name that users are likely to use for columns, doesn't it? |
@knz I agree that we should minimize the number of reserved keywords, but we also want to minimize the number of constructs and PS A lot of people habitually quote SQL identifiers using double quotes (e.g. |
Well if you know from experience that people tend to double quote identifiers naturally, my main argument goes away then. Then the only point remaining is that we should highlight in our docs that we have more keywords. |
A major benefit in my mind of making |
There are benefits to homogeneity but the higher level issue is to minimize inconvenience. Unless users are ready to accept more keywords (which they are generally not enclined to in other PLs but peter thinks for SQL it's OK) in general changing a regular identifier to a reserved keyword causes more inconvenience than additional grammar rules. |
Fixes cockroachdb#7061. This change adds `FAMILY` as a reserved keyword. This is necessary to resolve ambiguity with our grammar. The ambiguity is an artifact of `FAMILY` not being a reserved keyword. Without adding to our reserved keywords or using multiple character lookahead, the previous syntax was throwing a shift/reduce error. The ambiguity caused by this error is shown with the following situation: ``` CREATE TABLE t (x type_t (opts)); RENAME t.x TO t.family; SHOW CREATE TABLE t; +-------+---------------------------------------+ | Table | CreateTable | +-------+---------------------------------------+ | t | CREATE TABLE t (family type_t (opts)) | +-------+---------------------------------------+ ``` Now that `FAMILY` is a reserved keyword, `family` will not be a valid column name, and therefore the rename will not be possible.
9e8944e
to
39ce7de
Compare
Discussed with @knz in person and decided that the reserved keyword approach was ok. PTAL. |
Another possibility: we decided on the term
|
AFAIR, we switched to family largely because group was already used for something, which hasn't changed. So my preference is to stick with family.
|
TFTRs. I'll stick with @paperstreet's preference. If we want to change this later, we can always use the |
Our grammar diagram lists |
Fixes #7061.
This change adds
FAMILY
as a reserved keyword.This is necessary to resolve ambiguity with our grammar. The ambiguity
is an artifact of
FAMILY
not being a reserved keyword. Without adding toour reserved keywords or using multiple character lookahead, the
previous syntax was throwing a shift/reduce error. The ambiguity caused
by this error is shown with the following situation:
Now that
FAMILY
is a reserved keyword,family
will not be a validcolumn name, and therefore the rename will not be possible.
@knz
This change is