From 15c82efd6994affd1d5654d13bc8911a9faff659 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Tue, 1 Jul 2014 19:02:21 -0400
Subject: [PATCH] Refactor CREATE/ALTER DATABASE syntax so options need not be
keywords.
Most of the existing option names are keywords anyway, but we can get rid
of LC_COLLATE and LC_CTYPE as keywords known to the lexer/grammar. This
immediately reduces the size of the grammar tables by about 8KB, and will
save more when we add additional CREATE/ALTER DATABASE options in future.
A side effect of the implementation is that the CONNECTION LIMIT option
can now also be spelled CONNECTION_LIMIT. We choose not to document this,
however.
Vik Fearing, based on a suggestion by me; reviewed by Pavel Stehule
---
src/backend/commands/dbcommands.c | 54 +++++++------
src/backend/parser/gram.y | 123 +++++++++++-------------------
src/include/parser/kwlist.h | 2 -
3 files changed, 76 insertions(+), 103 deletions(-)
diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index 5705889f31d..dd92aff89dc 100644
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -39,6 +39,7 @@
#include "catalog/pg_tablespace.h"
#include "commands/comment.h"
#include "commands/dbcommands.h"
+#include "commands/defrem.h"
#include "commands/seclabel.h"
#include "commands/tablespace.h"
#include "mb/pg_wchar.h"
@@ -188,7 +189,7 @@ createdb(const CreatedbStmt *stmt)
errmsg("conflicting or redundant options")));
dctype = defel;
}
- else if (strcmp(defel->defname, "connectionlimit") == 0)
+ else if (strcmp(defel->defname, "connection_limit") == 0)
{
if (dconnlimit)
ereport(ERROR,
@@ -204,21 +205,22 @@ createdb(const CreatedbStmt *stmt)
errhint("Consider using tablespaces instead.")));
}
else
- elog(ERROR, "option \"%s\" not recognized",
- defel->defname);
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("option \"%s\" not recognized", defel->defname)));
}
if (downer && downer->arg)
- dbowner = strVal(downer->arg);
+ dbowner = defGetString(downer);
if (dtemplate && dtemplate->arg)
- dbtemplate = strVal(dtemplate->arg);
+ dbtemplate = defGetString(dtemplate);
if (dencoding && dencoding->arg)
{
const char *encoding_name;
if (IsA(dencoding->arg, Integer))
{
- encoding = intVal(dencoding->arg);
+ encoding = defGetInt32(dencoding);
encoding_name = pg_encoding_to_char(encoding);
if (strcmp(encoding_name, "") == 0 ||
pg_valid_server_encoding(encoding_name) < 0)
@@ -227,9 +229,9 @@ createdb(const CreatedbStmt *stmt)
errmsg("%d is not a valid encoding code",
encoding)));
}
- else if (IsA(dencoding->arg, String))
+ else
{
- encoding_name = strVal(dencoding->arg);
+ encoding_name = defGetString(dencoding);
encoding = pg_valid_server_encoding(encoding_name);
if (encoding < 0)
ereport(ERROR,
@@ -237,18 +239,15 @@ createdb(const CreatedbStmt *stmt)
errmsg("%s is not a valid encoding name",
encoding_name)));
}
- else
- elog(ERROR, "unrecognized node type: %d",
- nodeTag(dencoding->arg));
}
if (dcollate && dcollate->arg)
- dbcollate = strVal(dcollate->arg);
+ dbcollate = defGetString(dcollate);
if (dctype && dctype->arg)
- dbctype = strVal(dctype->arg);
+ dbctype = defGetString(dctype);
if (dconnlimit && dconnlimit->arg)
{
- dbconnlimit = intVal(dconnlimit->arg);
+ dbconnlimit = defGetInt32(dconnlimit);
if (dbconnlimit < -1)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
@@ -379,7 +378,7 @@ createdb(const CreatedbStmt *stmt)
char *tablespacename;
AclResult aclresult;
- tablespacename = strVal(dtablespacename->arg);
+ tablespacename = defGetString(dtablespacename);
dst_deftablespace = get_tablespace_oid(tablespacename, false);
/* check permissions */
aclresult = pg_tablespace_aclcheck(dst_deftablespace, GetUserId(),
@@ -1341,7 +1340,7 @@ AlterDatabase(AlterDatabaseStmt *stmt, bool isTopLevel)
{
DefElem *defel = (DefElem *) lfirst(option);
- if (strcmp(defel->defname, "connectionlimit") == 0)
+ if (strcmp(defel->defname, "connection_limit") == 0)
{
if (dconnlimit)
ereport(ERROR,
@@ -1358,23 +1357,32 @@ AlterDatabase(AlterDatabaseStmt *stmt, bool isTopLevel)
dtablespace = defel;
}
else
- elog(ERROR, "option \"%s\" not recognized",
- defel->defname);
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("option \"%s\" not recognized", defel->defname)));
}
if (dtablespace)
{
- /* currently, can't be specified along with any other options */
- Assert(!dconnlimit);
+ /*
+ * While the SET TABLESPACE syntax doesn't allow any other options,
+ * somebody could write "WITH TABLESPACE ...". Forbid any other
+ * options from being specified in that case.
+ */
+ if (list_length(stmt->options) != 1)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("option \"%s\" cannot be specified with other options",
+ dtablespace->defname)));
/* this case isn't allowed within a transaction block */
PreventTransactionChain(isTopLevel, "ALTER DATABASE SET TABLESPACE");
- movedb(stmt->dbname, strVal(dtablespace->arg));
+ movedb(stmt->dbname, defGetString(dtablespace));
return InvalidOid;
}
- if (dconnlimit)
+ if (dconnlimit && dconnlimit->arg)
{
- connlimit = intVal(dconnlimit->arg);
+ connlimit = defGetInt32(dconnlimit);
if (connlimit < -1)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 605c9b4aadf..ba7d091dc79 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -264,10 +264,10 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
%type <dbehavior> opt_drop_behavior
-%type <list> createdb_opt_list alterdb_opt_list copy_opt_list
+%type <list> createdb_opt_list createdb_opt_items copy_opt_list
transaction_mode_list
create_extension_opt_list alter_extension_opt_list
-%type <defelt> createdb_opt_item alterdb_opt_item copy_opt_item
+%type <defelt> createdb_opt_item copy_opt_item
transaction_mode_item
create_extension_opt_item alter_extension_opt_item
@@ -462,6 +462,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
%type <list> var_list
%type <str> ColId ColLabel var_name type_function_name param_name
%type <str> NonReservedWord NonReservedWord_or_Sconst
+%type <str> createdb_opt_name
%type <node> var_value zone_value
%type <keyword> unreserved_keyword type_func_name_keyword
@@ -564,7 +565,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
KEY
- LABEL LANGUAGE LARGE_P LAST_P LATERAL_P LC_COLLATE_P LC_CTYPE_P
+ LABEL LANGUAGE LARGE_P LAST_P LATERAL_P
LEADING LEAKPROOF LEAST LEFT LEVEL LIKE LIMIT LISTEN LOAD LOCAL
LOCALTIME LOCALTIMESTAMP LOCATION LOCK_P
@@ -8320,77 +8321,51 @@ CreatedbStmt:
;
createdb_opt_list:
- createdb_opt_list createdb_opt_item { $$ = lappend($1, $2); }
+ createdb_opt_items { $$ = $1; }
| /* EMPTY */ { $$ = NIL; }
;
+createdb_opt_items:
+ createdb_opt_item { $$ = list_make1($1); }
+ | createdb_opt_items createdb_opt_item { $$ = lappend($1, $2); }
+ ;
+
createdb_opt_item:
- TABLESPACE opt_equal name
- {
- $$ = makeDefElem("tablespace", (Node *)makeString($3));
- }
- | TABLESPACE opt_equal DEFAULT
- {
- $$ = makeDefElem("tablespace", NULL);
- }
- | LOCATION opt_equal Sconst
- {
- $$ = makeDefElem("location", (Node *)makeString($3));
- }
- | LOCATION opt_equal DEFAULT
- {
- $$ = makeDefElem("location", NULL);
- }
- | TEMPLATE opt_equal name
- {
- $$ = makeDefElem("template", (Node *)makeString($3));
- }
- | TEMPLATE opt_equal DEFAULT
- {
- $$ = makeDefElem("template", NULL);
- }
- | ENCODING opt_equal Sconst
- {
- $$ = makeDefElem("encoding", (Node *)makeString($3));
- }
- | ENCODING opt_equal Iconst
- {
- $$ = makeDefElem("encoding", (Node *)makeInteger($3));
- }
- | ENCODING opt_equal DEFAULT
- {
- $$ = makeDefElem("encoding", NULL);
- }
- | LC_COLLATE_P opt_equal Sconst
- {
- $$ = makeDefElem("lc_collate", (Node *)makeString($3));
- }
- | LC_COLLATE_P opt_equal DEFAULT
- {
- $$ = makeDefElem("lc_collate", NULL);
- }
- | LC_CTYPE_P opt_equal Sconst
+ createdb_opt_name opt_equal SignedIconst
{
- $$ = makeDefElem("lc_ctype", (Node *)makeString($3));
+ $$ = makeDefElem($1, (Node *)makeInteger($3));
}
- | LC_CTYPE_P opt_equal DEFAULT
+ | createdb_opt_name opt_equal opt_boolean_or_string
{
- $$ = makeDefElem("lc_ctype", NULL);
+ $$ = makeDefElem($1, (Node *)makeString($3));
}
- | CONNECTION LIMIT opt_equal SignedIconst
+ | createdb_opt_name opt_equal DEFAULT
{
- $$ = makeDefElem("connectionlimit", (Node *)makeInteger($4));
- }
- | OWNER opt_equal name
- {
- $$ = makeDefElem("owner", (Node *)makeString($3));
- }
- | OWNER opt_equal DEFAULT
- {
- $$ = makeDefElem("owner", NULL);
+ $$ = makeDefElem($1, NULL);
}
;
+/*
+ * Ideally we'd use ColId here, but that causes shift/reduce conflicts against
+ * the ALTER DATABASE SET/RESET syntaxes. Instead call out specific keywords
+ * we need, and allow IDENT so that database option names don't have to be
+ * parser keywords unless they are already keywords for other reasons.
+ *
+ * XXX this coding technique is fragile since if someone makes a formerly
+ * non-keyword option name into a keyword and forgets to add it here, the
+ * option will silently break. Best defense is to provide a regression test
+ * exercising every such option, at least at the syntax level.
+ */
+createdb_opt_name:
+ IDENT { $$ = $1; }
+ | CONNECTION LIMIT { $$ = pstrdup("connection_limit"); }
+ | ENCODING { $$ = pstrdup($1); }
+ | LOCATION { $$ = pstrdup($1); }
+ | OWNER { $$ = pstrdup($1); }
+ | TABLESPACE { $$ = pstrdup($1); }
+ | TEMPLATE { $$ = pstrdup($1); }
+ ;
+
/*
* Though the equals sign doesn't match other WITH options, pg_dump uses
* equals for backward compatibility, and it doesn't seem worth removing it.
@@ -8407,13 +8382,20 @@ opt_equal: '=' {}
*****************************************************************************/
AlterDatabaseStmt:
- ALTER DATABASE database_name opt_with alterdb_opt_list
+ ALTER DATABASE database_name WITH createdb_opt_list
{
AlterDatabaseStmt *n = makeNode(AlterDatabaseStmt);
n->dbname = $3;
n->options = $5;
$$ = (Node *)n;
}
+ | ALTER DATABASE database_name createdb_opt_list
+ {
+ AlterDatabaseStmt *n = makeNode(AlterDatabaseStmt);
+ n->dbname = $3;
+ n->options = $4;
+ $$ = (Node *)n;
+ }
| ALTER DATABASE database_name SET TABLESPACE name
{
AlterDatabaseStmt *n = makeNode(AlterDatabaseStmt);
@@ -8435,19 +8417,6 @@ AlterDatabaseSetStmt:
;
-alterdb_opt_list:
- alterdb_opt_list alterdb_opt_item { $$ = lappend($1, $2); }
- | /* EMPTY */ { $$ = NIL; }
- ;
-
-alterdb_opt_item:
- CONNECTION LIMIT opt_equal SignedIconst
- {
- $$ = makeDefElem("connectionlimit", (Node *)makeInteger($4));
- }
- ;
-
-
/*****************************************************************************
*
* DROP DATABASE [ IF EXISTS ]
@@ -12958,8 +12927,6 @@ unreserved_keyword:
| LANGUAGE
| LARGE_P
| LAST_P
- | LC_COLLATE_P
- | LC_CTYPE_P
| LEAKPROOF
| LEVEL
| LISTEN
diff --git a/src/include/parser/kwlist.h b/src/include/parser/kwlist.h
index 61fae22f0a0..04e98109635 100644
--- a/src/include/parser/kwlist.h
+++ b/src/include/parser/kwlist.h
@@ -215,8 +215,6 @@ PG_KEYWORD("language", LANGUAGE, UNRESERVED_KEYWORD)
PG_KEYWORD("large", LARGE_P, UNRESERVED_KEYWORD)
PG_KEYWORD("last", LAST_P, UNRESERVED_KEYWORD)
PG_KEYWORD("lateral", LATERAL_P, RESERVED_KEYWORD)
-PG_KEYWORD("lc_collate", LC_COLLATE_P, UNRESERVED_KEYWORD)
-PG_KEYWORD("lc_ctype", LC_CTYPE_P, UNRESERVED_KEYWORD)
PG_KEYWORD("leading", LEADING, RESERVED_KEYWORD)
PG_KEYWORD("leakproof", LEAKPROOF, UNRESERVED_KEYWORD)
PG_KEYWORD("least", LEAST, COL_NAME_KEYWORD)
--
GitLab