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

diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c
index 5705889f31d349c33966c4bbe84763a99400c04b..dd92aff89dc7848aeca8a06142f46d8372304630 100644 (file)
--- 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 605c9b4aadfaffb67b5b99771d8d644212165621..ba7d091dc793c079481a9a0fab05a110c8e98ce7 100644 (file)
--- 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 61fae22f0a0e1b396660dd52ca33df2d2df86931..04e98109635fa08a742557e59f285b25e796a1ca 100644 (file)
--- 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)