sql/parser: unreserve INDEX and NOTHING from the RHS of SET statements #31731
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… 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.
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.
bobvawter
approved these changes
Oct 23, 2018
|
TFYR! bors r+ |
Merge conflict (retrying...) |
craig bot
pushed a commit
that referenced
this pull request
Oct 23, 2018
31637: pgwire: add telemetry for fetch limits r=knz a=knz Requested by @awoods187 The JDBC driver and perhaps others commonly try to use the "fetch limit" parameter, which is yet unsupported in CockroachDB (#4035). This patch adds telemetry to gauge demand. Release note (sql change): attempts by client apps to use the unsupported "fetch limit" parameter (e.g. via JDBC) will now be captured in telemetry if statistics reporting is enabled, to gauge support for this feature. 31725: sql/parser: re-allow FAMILY, MINVALUE, MAXVALUE, NOTHING and INDEX in table names r=knz a=knz Fixes #31589. 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. 31731: sql/parser: unreserve INDEX and NOTHING from the RHS of SET statements r=knz a=knz First commit from #31725. 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. Co-authored-by: Raphael 'kena' Poss <[email protected]>
Build succeeded |
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]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
First commit from #31725.
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.