Skip to content
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: unreserve MINVALUE and MAXVALUE #31730

Merged
merged 3 commits into from
Oct 23, 2018
Merged

Conversation

knz
Copy link
Contributor

@knz knz commented Oct 23, 2018

First two commits from #31725 and #31731.

The keywords MINVALUE and MAXVALUE had been marked as reserved away
from being a valid column name to accommodate the CockroachDB-specific
"PARTITIONING" syntax.

This is unfortunate because these two words are perfectly fine English
names for things people may want to have as attributes in their
relations.

This patch removes the limitation by unreserving the two keywords and
handling the special words in the planning code for the partitioning
feature. In every other context the two words are not treated
specially and handled like any possible column name / identifier.

Release note (sql change): It is now again possible to use the
keywords MINVALUE and MAXVALUE as column names, for compatibility with
PostgreSQL.

… table names

CockroachDB introduced non-standard extensions to its SQL dialect very
early in its history, before concerns of compatibility with existing
PostgreSQL clients became a priority. When these features were added,
new keywords were liberally marked as "reserved", so as to "make the
grammar work", and without noticing / care for the fact that this
change would make existing valid SQL queries/clients encounter new
errors.

An example of this:

1. let's make "column families" a thing

2. the syntax `create table(..., constraint xxx family(a,b,c))` is not
   good enough (although this would not require reserved keywords), we
   really want also `create table (..., family (a,b,c))` to be
   possible.

3. oh, the grammar won't let us because "family" is a possible column
   name? No matter! let's mark "FAMILY" as a reserved name for
   column/function names.

   - No concern for the fact that "family" is a  perfectly valid
	 English name for things that people want to make an attribute of
	 in inventory / classification tables.

   - No concern for the fact that reserved column/function names are
	 also reserved for table names.

4. (much later) Clients complaining about the fact they can't call
   their columns or tables `family` without quoting.

Ditto "INDEX", "MINVALUE", "MAXVALUE", and perhaps others.

Moral of the story: DO NOT MAKE NEW RESERVED KEYWORDS UNLESS YOU'RE
VERY VERY VERY SURE THAT THERE IS NO LEGITIMATE USE FOR THEM IN CLIENT
APPS EVER.

(An example perhaps: the word "NOTHING" was also marked as reserved,
but it's much more unlikely this word will ever be used for something
useful.)

This patch restores the use of FAMILY, INDEX, NOTHING, MINVALUE and
MAXVALUE in table and function names, by introducing an awkward dance
in the grammar of keyword non-terminals and database object names.

They remain reserved as colum names because of the non-standard
CockroachDB extensions.

Release note (sql change): It is now again possible to use the
keywords FAMILY, MINVALUE, MAXVALUE, INDEX or NOTHING as table names,
for compatibility with PostgreSQL.
@knz knz requested review from a team and bobvawter October 23, 2018 10:41
The SET statement in the pg dialect is special because it
auto-converts identifiers on its RHS to symbolic values or strings. In
particular it is meant to support a diversity of special keywords as
pseudo-values.

This patch ensures that INDEX and NOTHING are accepted on the RHS.

Release note (sql change): the names "index" and "nothing" are again
accepted in the right-hand-side of the assignment in SET statements,
for compatibility with PostgreSQL.
The keywords MINVALUE and MAXVALUE had been marked as reserved away
from being a valid column name to accommodate the CockroachDB-specific
"PARTITIONING" syntax.

This is unfortunate because these two words are perfectly fine English
names for things people may want to have as attributes in their
relations.

This patch removes the limitation by unreserving the two keywords and
handling the special words in the planning code for the partitioning
feature. In every other context the two words are not treated
specially and handled like any possible column name / identifier.

Release note (sql change): It is now again possible to use the
keywords MINVALUE and MAXVALUE as column names, for compatibility with
PostgreSQL.
@knz
Copy link
Contributor Author

knz commented Oct 23, 2018

will want also a green light by Nikhil or Dan before merging this one.

@knz
Copy link
Contributor Author

knz commented Oct 23, 2018

cool thanks

bors r+

craig bot pushed a commit that referenced this pull request Oct 23, 2018
31730:  sql: unreserve MINVALUE and MAXVALUE r=knz a=knz

First two commits from  #31725 and  #31731.

The keywords MINVALUE and MAXVALUE had been marked as reserved away
from being a valid column name to accommodate the CockroachDB-specific
"PARTITIONING" syntax.

This is unfortunate because these two words are perfectly fine English
names for things people may want to have as attributes in their
relations.

This patch removes the limitation by unreserving the two keywords and
handling the special words in the planning code for the partitioning
feature. In every other context the two words are not treated
specially and handled like any possible column name / identifier.

Release note (sql change): It is now again possible to use the
keywords MINVALUE and MAXVALUE as column names, for compatibility with
PostgreSQL.

Co-authored-by: Raphael 'kena' Poss <[email protected]>
@craig craig bot merged commit 795ed06 into cockroachdb:master Oct 23, 2018
@knz knz deleted the 20181023-minmax-val branch February 14, 2019 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants