On 05/26/2014 08:19 PM, Vik Fearing wrote:
> On 05/26/2014 07:10 AM, Stephen Frost wrote:
>> * Tom Lane (t...@sss.pgh.pa.us) wrote:
>>> I don't really object to doing an unlocked check for another such
>>> database, but I'm not convinced that additional locking to try to
>>> prevent a race is worth its keep.
>> +1 on the nannyism, and +1 to ignoring the race.
> Okay, I'll submit a new patch with racy nannyism and some grammar
> changes that Tom and I have been discussing in private.

Attached are two patches.

The first is a refactoring of the createdb/alterdb grammars mostly by
Tom which makes all of the options non-keywords that don't otherwise
need to be.  Not only does this remove the two unreserved keywords I had
added (ALLOW and CONNECTIONS) but also removes two existing ones
(LC_COLLATE and LC_TYPE), reducing gram.o by about half a percent by
Tom's calculations.  That's much better than increasing it like my
original patch did.

The problem is we foolishly adopted a two-word option name (CONNECTION
LIMIT) which complicates the grammar.  My aping of that for IS TEMPLATE
and ALLOW CONNECTIONS only aggravated the situation.  And so I changed
all the documentation (and pg_dumpall etc) to use CONNECTION_LIMIT
instead.  We might hopefully one day deprecate the with-space version so
the sooner the documentation recommends the without-space version, the
better.  The old syntax is of course still valid.

I also changed the documentation to say connection_limit instead of
connlimit.  Documentation is for humans, something like "connlimit" (and
later as we'll see allowconn) is for programmers.  It also indirectly
reminds us that we should not add another multi-word option like I
initially did.

Now that anything goes grammar-wise, the elog for unknown option is now
ereport.

I am hoping this gets backpatched.

-------

The second patch adds my original functionality, except this time the
syntax is IS_TEMPLATE and ALLOW_CONNECTIONS (both one word, neither
being keywords).  It also changes all the catalog updates we used to do
because we didn't have this patch.

As for the nannyism, the point was to find another database having
datallowconn=true but since we have to be connected to a database to
issue this command, the simplest is just to disallow the current
database (credit Andres on IRC) so that's what I've done as explained
with this in-code comment:

        /*
        * In order to avoid getting locked out and having to go through
standalone
        * mode, we refuse to disallow connections on the database we're
currently
        * connected to.  Lockout can still happen with concurrent
sessions but the
        * likeliness of that is not high enough to worry about.
        */

I also changed the C variable connlimit to dbconnlimit in
AlterDatabase() to be consistent with its analog in createdb().

-------

The third and non-existent patch is about regression tests.  Tom put in
gram.y that we should have some now that the grammar doesn't regulate
it, and I wanted some anyway in my original patch; but I don't know what
they should look like or where they should go so I'm asking for help on
that.

-- 
Vik

*** a/contrib/sepgsql/expected/alter.out
--- b/contrib/sepgsql/expected/alter.out
***************
*** 110,116 **** SET search_path = regtest_schema, regtest_schema_2, public;
  --
  -- misc ALTER commands
  --
! ALTER DATABASE regtest_sepgsql_test_database CONNECTION LIMIT 999;
  LOG:  SELinux: allowed { setattr } scontext=unconfined_u:unconfined_r:unconfined_t:s0 tcontext=unconfined_u:object_r:sepgsql_db_t:s0 tclass=db_database name="regtest_sepgsql_test_database"
  ALTER DATABASE regtest_sepgsql_test_database SET search_path TO regtest_schema, public; -- not supported yet
  ALTER TABLE regtest_table ADD COLUMN d float;
--- 110,116 ----
  --
  -- misc ALTER commands
  --
! ALTER DATABASE regtest_sepgsql_test_database CONNECTION_LIMIT 999;
  LOG:  SELinux: allowed { setattr } scontext=unconfined_u:unconfined_r:unconfined_t:s0 tcontext=unconfined_u:object_r:sepgsql_db_t:s0 tclass=db_database name="regtest_sepgsql_test_database"
  ALTER DATABASE regtest_sepgsql_test_database SET search_path TO regtest_schema, public; -- not supported yet
  ALTER TABLE regtest_table ADD COLUMN d float;
*** a/contrib/sepgsql/sql/alter.sql
--- b/contrib/sepgsql/sql/alter.sql
***************
*** 85,91 **** SET search_path = regtest_schema, regtest_schema_2, public;
  --
  -- misc ALTER commands
  --
! ALTER DATABASE regtest_sepgsql_test_database CONNECTION LIMIT 999;
  ALTER DATABASE regtest_sepgsql_test_database SET search_path TO regtest_schema, public; -- not supported yet
  
  ALTER TABLE regtest_table ADD COLUMN d float;
--- 85,91 ----
  --
  -- misc ALTER commands
  --
! ALTER DATABASE regtest_sepgsql_test_database CONNECTION_LIMIT 999;
  ALTER DATABASE regtest_sepgsql_test_database SET search_path TO regtest_schema, public; -- not supported yet
  
  ALTER TABLE regtest_table ADD COLUMN d float;
*** a/doc/src/sgml/ref/alter_database.sgml
--- b/doc/src/sgml/ref/alter_database.sgml
***************
*** 25,31 **** ALTER DATABASE <replaceable class="PARAMETER">name</replaceable> [ [ WITH ] <rep
  
  <phrase>where <replaceable class="PARAMETER">option</replaceable> can be:</phrase>
  
!     CONNECTION LIMIT <replaceable class="PARAMETER">connlimit</replaceable>
  
  ALTER DATABASE <replaceable class="PARAMETER">name</replaceable> RENAME TO <replaceable>new_name</replaceable>
  
--- 25,31 ----
  
  <phrase>where <replaceable class="PARAMETER">option</replaceable> can be:</phrase>
  
!     CONNECTION_LIMIT <replaceable class="PARAMETER">connection_limit</replaceable>
  
  ALTER DATABASE <replaceable class="PARAMETER">name</replaceable> RENAME TO <replaceable>new_name</replaceable>
  
***************
*** 107,113 **** ALTER DATABASE <replaceable class="PARAMETER">name</replaceable> RESET ALL
       </varlistentry>
  
       <varlistentry>
!       <term><replaceable class="parameter">connlimit</replaceable></term>
        <listitem>
         <para>
          How many concurrent connections can be made
--- 107,113 ----
       </varlistentry>
  
       <varlistentry>
!       <term><replaceable class="parameter">connection_limit</replaceable></term>
        <listitem>
         <para>
          How many concurrent connections can be made
*** a/doc/src/sgml/ref/create_database.sgml
--- b/doc/src/sgml/ref/create_database.sgml
***************
*** 28,34 **** CREATE DATABASE <replaceable class="PARAMETER">name</replaceable>
             [ LC_COLLATE [=] <replaceable class="parameter">lc_collate</replaceable> ]
             [ LC_CTYPE [=] <replaceable class="parameter">lc_ctype</replaceable> ]
             [ TABLESPACE [=] <replaceable class="parameter">tablespace_name</replaceable> ]
!            [ CONNECTION LIMIT [=] <replaceable class="parameter">connlimit</replaceable> ] ]
  </synopsis>
   </refsynopsisdiv>
  
--- 28,34 ----
             [ LC_COLLATE [=] <replaceable class="parameter">lc_collate</replaceable> ]
             [ LC_CTYPE [=] <replaceable class="parameter">lc_ctype</replaceable> ]
             [ TABLESPACE [=] <replaceable class="parameter">tablespace_name</replaceable> ]
!            [ CONNECTION_LIMIT [=] <replaceable class="parameter">connection_limit</replaceable> ] ]
  </synopsis>
   </refsynopsisdiv>
  
***************
*** 148,154 **** CREATE DATABASE <replaceable class="PARAMETER">name</replaceable>
       </varlistentry>
  
       <varlistentry>
!       <term><replaceable class="parameter">connlimit</replaceable></term>
        <listitem>
         <para>
          How many concurrent connections can be made
--- 148,154 ----
       </varlistentry>
  
       <varlistentry>
!       <term><replaceable class="parameter">connection_limit</replaceable></term>
        <listitem>
         <para>
          How many concurrent connections can be made
***************
*** 231,237 **** CREATE DATABASE <replaceable class="PARAMETER">name</replaceable>
    </para>
  
    <para>
!    The <literal>CONNECTION LIMIT</> option is only enforced approximately;
     if two new sessions start at about the same time when just one
     connection <quote>slot</> remains for the database, it is possible that
     both will fail.  Also, the limit is not enforced against superusers.
--- 231,237 ----
    </para>
  
    <para>
!    The <literal>CONNECTION_LIMIT</> option is only enforced approximately;
     if two new sessions start at about the same time when just one
     connection <quote>slot</> remains for the database, it is possible that
     both will fail.  Also, the limit is not enforced against superusers.
*** a/src/backend/commands/dbcommands.c
--- b/src/backend/commands/dbcommands.c
***************
*** 39,44 ****
--- 39,45 ----
  #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,194 **** createdb(const CreatedbStmt *stmt)
  						 errmsg("conflicting or redundant options")));
  			dctype = defel;
  		}
! 		else if (strcmp(defel->defname, "connectionlimit") == 0)
  		{
  			if (dconnlimit)
  				ereport(ERROR,
--- 189,195 ----
  						 errmsg("conflicting or redundant options")));
  			dctype = defel;
  		}
! 		else if (strcmp(defel->defname, "connection_limit") == 0)
  		{
  			if (dconnlimit)
  				ereport(ERROR,
***************
*** 204,224 **** createdb(const CreatedbStmt *stmt)
  					 errhint("Consider using tablespaces instead.")));
  		}
  		else
! 			elog(ERROR, "option \"%s\" not recognized",
! 				 defel->defname);
  	}
  
! 	if (downer && downer->arg)
! 		dbowner = strVal(downer->arg);
! 	if (dtemplate && dtemplate->arg)
! 		dbtemplate = strVal(dtemplate->arg);
! 	if (dencoding && dencoding->arg)
  	{
  		const char *encoding_name;
  
  		if (IsA(dencoding->arg, Integer))
  		{
! 			encoding = intVal(dencoding->arg);
  			encoding_name = pg_encoding_to_char(encoding);
  			if (strcmp(encoding_name, "") == 0 ||
  				pg_valid_server_encoding(encoding_name) < 0)
--- 205,226 ----
  					 errhint("Consider using tablespaces instead.")));
  		}
  		else
! 			ereport(ERROR,
! 					(errcode(ERRCODE_SYNTAX_ERROR),
! 					 errmsg("option \"%s\" not recognized", defel->defname)));
  	}
  
! 	if (downer)
! 		dbowner = defGetString(downer);
! 	if (dtemplate)
! 		dbtemplate = defGetString(dtemplate);
! 	if (dencoding)
  	{
  		const char *encoding_name;
  
  		if (IsA(dencoding->arg, Integer))
  		{
! 			encoding = defGetInt32(dencoding);
  			encoding_name = pg_encoding_to_char(encoding);
  			if (strcmp(encoding_name, "") == 0 ||
  				pg_valid_server_encoding(encoding_name) < 0)
***************
*** 229,235 **** createdb(const CreatedbStmt *stmt)
  		}
  		else if (IsA(dencoding->arg, String))
  		{
! 			encoding_name = strVal(dencoding->arg);
  			encoding = pg_valid_server_encoding(encoding_name);
  			if (encoding < 0)
  				ereport(ERROR,
--- 231,237 ----
  		}
  		else if (IsA(dencoding->arg, String))
  		{
! 			encoding_name = defGetString(dencoding);
  			encoding = pg_valid_server_encoding(encoding_name);
  			if (encoding < 0)
  				ereport(ERROR,
***************
*** 241,254 **** createdb(const CreatedbStmt *stmt)
  			elog(ERROR, "unrecognized node type: %d",
  				 nodeTag(dencoding->arg));
  	}
! 	if (dcollate && dcollate->arg)
! 		dbcollate = strVal(dcollate->arg);
! 	if (dctype && dctype->arg)
! 		dbctype = strVal(dctype->arg);
  
! 	if (dconnlimit && dconnlimit->arg)
  	{
! 		dbconnlimit = intVal(dconnlimit->arg);
  		if (dbconnlimit < -1)
  			ereport(ERROR,
  					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
--- 243,256 ----
  			elog(ERROR, "unrecognized node type: %d",
  				 nodeTag(dencoding->arg));
  	}
! 	if (dcollate)
! 		dbcollate = defGetString(dcollate);
! 	if (dctype)
! 		dbctype = defGetString(dctype);
  
! 	if (dconnlimit)
  	{
! 		dbconnlimit = defGetInt32(dconnlimit);
  		if (dbconnlimit < -1)
  			ereport(ERROR,
  					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
***************
*** 1341,1347 **** AlterDatabase(AlterDatabaseStmt *stmt, bool isTopLevel)
  	{
  		DefElem    *defel = (DefElem *) lfirst(option);
  
! 		if (strcmp(defel->defname, "connectionlimit") == 0)
  		{
  			if (dconnlimit)
  				ereport(ERROR,
--- 1343,1349 ----
  	{
  		DefElem    *defel = (DefElem *) lfirst(option);
  
! 		if (strcmp(defel->defname, "connection_limit") == 0)
  		{
  			if (dconnlimit)
  				ereport(ERROR,
***************
*** 1374,1380 **** AlterDatabase(AlterDatabaseStmt *stmt, bool isTopLevel)
  
  	if (dconnlimit)
  	{
! 		connlimit = intVal(dconnlimit->arg);
  		if (connlimit < -1)
  			ereport(ERROR,
  					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
--- 1376,1382 ----
  
  	if (dconnlimit)
  	{
! 		connlimit = defGetInt32(dconnlimit);
  		if (connlimit < -1)
  			ereport(ERROR,
  					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
***************
*** 261,270 **** 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
  				transaction_mode_list
  				create_extension_opt_list alter_extension_opt_list
! %type <defelt>	createdb_opt_item alterdb_opt_item copy_opt_item
  				transaction_mode_item
  				create_extension_opt_item alter_extension_opt_item
  
--- 261,270 ----
  
  %type <dbehavior>	opt_drop_behavior
  
! %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 copy_opt_item
  				transaction_mode_item
  				create_extension_opt_item alter_extension_opt_item
  
***************
*** 459,464 **** static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
--- 459,465 ----
  %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
***************
*** 561,567 **** static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
  
  	KEY
  
! 	LABEL LANGUAGE LARGE_P LAST_P LATERAL_P LC_COLLATE_P LC_CTYPE_P
  	LEADING LEAKPROOF LEAST LEFT LEVEL LIKE LIMIT LISTEN LOAD LOCAL
  	LOCALTIME LOCALTIMESTAMP LOCATION LOCK_P
  
--- 562,568 ----
  
  	KEY
  
! 	LABEL LANGUAGE LARGE_P LAST_P LATERAL_P
  	LEADING LEAKPROOF LEAST LEFT LEVEL LIKE LIMIT LISTEN LOAD LOCAL
  	LOCALTIME LOCALTIMESTAMP LOCATION LOCK_P
  
***************
*** 8317,8394 **** CreatedbStmt:
  		;
  
  createdb_opt_list:
! 			createdb_opt_list createdb_opt_item		{ $$ = lappend($1, $2); }
  			| /* EMPTY */							{ $$ = NIL; }
  		;
  
  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
  				{
! 					$$ = makeDefElem("lc_ctype", (Node *)makeString($3));
  				}
! 			| LC_CTYPE_P opt_equal DEFAULT
  				{
! 					$$ = makeDefElem("lc_ctype", NULL);
  				}
! 			| CONNECTION LIMIT opt_equal SignedIconst
  				{
! 					$$ = makeDefElem("connectionlimit", (Node *)makeInteger($4));
! 				}
! 			| OWNER opt_equal name
! 				{
! 					$$ = makeDefElem("owner", (Node *)makeString($3));
! 				}
! 			| OWNER opt_equal DEFAULT
! 				{
! 					$$ = makeDefElem("owner", NULL);
  				}
  		;
  
  /*
   *	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.
   */
--- 8318,8369 ----
  		;
  
  createdb_opt_list:
! 			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:
! 			createdb_opt_name opt_equal SignedIconst
  				{
! 					$$ = makeDefElem($1, (Node *)makeInteger($3));
  				}
! 			| createdb_opt_name opt_equal opt_boolean_or_string
  				{
! 					$$ = makeDefElem($1, (Node *)makeString($3));
  				}
! 			| createdb_opt_name opt_equal DEFAULT
  				{
! 					$$ = 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.
   */
***************
*** 8404,8416 **** opt_equal:	'='										{}
   *****************************************************************************/
  
  AlterDatabaseStmt:
! 			ALTER DATABASE database_name opt_with alterdb_opt_list
  				 {
  					AlterDatabaseStmt *n = makeNode(AlterDatabaseStmt);
  					n->dbname = $3;
  					n->options = $5;
  					$$ = (Node *)n;
  				 }
  			| ALTER DATABASE database_name SET TABLESPACE name
  				 {
  					AlterDatabaseStmt *n = makeNode(AlterDatabaseStmt);
--- 8379,8398 ----
   *****************************************************************************/
  
  AlterDatabaseStmt:
! 			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);
***************
*** 8432,8450 **** 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 ]
--- 8414,8419 ----
***************
*** 12912,12919 **** unreserved_keyword:
  			| LANGUAGE
  			| LARGE_P
  			| LAST_P
- 			| LC_COLLATE_P
- 			| LC_CTYPE_P
  			| LEAKPROOF
  			| LEVEL
  			| LISTEN
--- 12881,12886 ----
*** a/src/bin/pg_dump/pg_dumpall.c
--- b/src/bin/pg_dump/pg_dumpall.c
***************
*** 1375,1381 **** dumpCreateDB(PGconn *conn)
  								  fmtId(dbtablespace));
  
  			if (strcmp(dbconnlimit, "-1") != 0)
! 				appendPQExpBuffer(buf, " CONNECTION LIMIT = %s",
  								  dbconnlimit);
  
  			appendPQExpBufferStr(buf, ";\n");
--- 1375,1381 ----
  								  fmtId(dbtablespace));
  
  			if (strcmp(dbconnlimit, "-1") != 0)
! 				appendPQExpBuffer(buf, " CONNECTION_LIMIT = %s",
  								  dbconnlimit);
  
  			appendPQExpBufferStr(buf, ";\n");
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
***************
*** 1004,1010 **** psql_completion(const char *text, int start, int end)
  			 pg_strcasecmp(prev2_wd, "DATABASE") == 0)
  	{
  		static const char *const list_ALTERDATABASE[] =
! 		{"RESET", "SET", "OWNER TO", "RENAME TO", "CONNECTION LIMIT", NULL};
  
  		COMPLETE_WITH_LIST(list_ALTERDATABASE);
  	}
--- 1004,1010 ----
  			 pg_strcasecmp(prev2_wd, "DATABASE") == 0)
  	{
  		static const char *const list_ALTERDATABASE[] =
! 		{"RESET", "SET", "OWNER TO", "RENAME TO", "CONNECTION_LIMIT", NULL};
  
  		COMPLETE_WITH_LIST(list_ALTERDATABASE);
  	}
***************
*** 2045,2051 **** psql_completion(const char *text, int start, int end)
  			 pg_strcasecmp(prev2_wd, "DATABASE") == 0)
  	{
  		static const char *const list_DATABASE[] =
! 		{"OWNER", "TEMPLATE", "ENCODING", "TABLESPACE", "CONNECTION LIMIT",
  		NULL};
  
  		COMPLETE_WITH_LIST(list_DATABASE);
--- 2045,2051 ----
  			 pg_strcasecmp(prev2_wd, "DATABASE") == 0)
  	{
  		static const char *const list_DATABASE[] =
! 		{"OWNER", "TEMPLATE", "ENCODING", "TABLESPACE", "CONNECTION_LIMIT",
  		NULL};
  
  		COMPLETE_WITH_LIST(list_DATABASE);
*** a/src/include/parser/kwlist.h
--- b/src/include/parser/kwlist.h
***************
*** 215,222 **** 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)
--- 215,220 ----
*** a/contrib/pg_upgrade/pg_upgrade.c
--- b/contrib/pg_upgrade/pg_upgrade.c
***************
*** 519,527 **** set_frozenxids(void)
  		 */
  		if (strcmp(datallowconn, "f") == 0)
  			PQclear(executeQueryOrDie(conn_template1,
! 									  "UPDATE pg_catalog.pg_database "
! 									  "SET	datallowconn = true "
! 									  "WHERE datname = '%s'", datname));
  
  		conn = connectToServer(&new_cluster, datname);
  
--- 519,526 ----
  		 */
  		if (strcmp(datallowconn, "f") == 0)
  			PQclear(executeQueryOrDie(conn_template1,
! 									  "ALTER DATABASE %s ALLOW_CONNECTIONS = true",
! 									  datname));
  
  		conn = connectToServer(&new_cluster, datname);
  
***************
*** 537,545 **** set_frozenxids(void)
  		/* Reset datallowconn flag */
  		if (strcmp(datallowconn, "f") == 0)
  			PQclear(executeQueryOrDie(conn_template1,
! 									  "UPDATE pg_catalog.pg_database "
! 									  "SET	datallowconn = false "
! 									  "WHERE datname = '%s'", datname));
  	}
  
  	PQclear(dbres);
--- 536,543 ----
  		/* Reset datallowconn flag */
  		if (strcmp(datallowconn, "f") == 0)
  			PQclear(executeQueryOrDie(conn_template1,
! 									  "ALTER DATABASE %s ALLOW_CONNECTIONS = false",
! 									  datname));
  	}
  
  	PQclear(dbres);
*** a/doc/src/sgml/ref/alter_database.sgml
--- b/doc/src/sgml/ref/alter_database.sgml
***************
*** 25,30 **** ALTER DATABASE <replaceable class="PARAMETER">name</replaceable> [ [ WITH ] <rep
--- 25,32 ----
  
  <phrase>where <replaceable class="PARAMETER">option</replaceable> can be:</phrase>
  
+     IS_TEMPLATE <replaceable class="PARAMETER">istemplate</replaceable>
+     ALLOW_CONNECTIONS <replaceable class="PARAMETER">allowconn</replaceable>
      CONNECTION_LIMIT <replaceable class="PARAMETER">connection_limit</replaceable>
  
  ALTER DATABASE <replaceable class="PARAMETER">name</replaceable> RENAME TO <replaceable>new_name</replaceable>
***************
*** 107,112 **** ALTER DATABASE <replaceable class="PARAMETER">name</replaceable> RESET ALL
--- 109,134 ----
       </varlistentry>
  
       <varlistentry>
+        <term><replaceable class="parameter">istemplate</replaceable></term>
+        <listitem>
+         <para>
+          If true, then this database can be cloned by any user with CREATEDB
+          privileges; if false, then only superusers or the owner of the
+          database can clone it.
+         </para>
+        </listitem>
+       </varlistentry>
+  
+       <varlistentry>
+        <term><replaceable class="parameter">allowconn</replaceable></term>
+        <listitem>
+         <para>
+          If false then no one can connect to this database.
+         </para>
+        </listitem>
+       </varlistentry>
+  
+       <varlistentry>
        <term><replaceable class="parameter">connection_limit</replaceable></term>
        <listitem>
         <para>
*** a/doc/src/sgml/ref/create_database.sgml
--- b/doc/src/sgml/ref/create_database.sgml
***************
*** 28,33 **** CREATE DATABASE <replaceable class="PARAMETER">name</replaceable>
--- 28,35 ----
             [ LC_COLLATE [=] <replaceable class="parameter">lc_collate</replaceable> ]
             [ LC_CTYPE [=] <replaceable class="parameter">lc_ctype</replaceable> ]
             [ TABLESPACE [=] <replaceable class="parameter">tablespace_name</replaceable> ]
+            [ IS_TEMPLATE [=] <replaceable class="parameter">istemplate</replaceable>]
+            [ ALLOW_CONNECTIONS [=] <replaceable class="parameter">allowconn</replaceable>]
             [ CONNECTION_LIMIT [=] <replaceable class="parameter">connection_limit</replaceable> ] ]
  </synopsis>
   </refsynopsisdiv>
***************
*** 148,153 **** CREATE DATABASE <replaceable class="PARAMETER">name</replaceable>
--- 150,175 ----
       </varlistentry>
  
       <varlistentry>
+        <term><replaceable class="parameter">istemplate</replaceable></term>
+        <listitem>
+         <para>
+          If true, then this database can be cloned by any user with CREATEDB
+          privileges; if false, then only superusers or the owner of the
+          database can clone it.
+         </para>
+        </listitem>
+       </varlistentry>
+  
+       <varlistentry>
+        <term><replaceable class="parameter">allowconn</replaceable></term>
+        <listitem>
+         <para>
+          If false then no one can connect to this database.
+         </para>
+        </listitem>
+       </varlistentry>
+  
+       <varlistentry>
        <term><replaceable class="parameter">connection_limit</replaceable></term>
        <listitem>
         <para>
*** a/src/backend/commands/dbcommands.c
--- b/src/backend/commands/dbcommands.c
***************
*** 123,128 **** createdb(const CreatedbStmt *stmt)
--- 123,130 ----
  	DefElem    *dencoding = NULL;
  	DefElem    *dcollate = NULL;
  	DefElem    *dctype = NULL;
+ 	DefElem	   *distemplate = NULL;
+ 	DefElem	   *dallowconnections = NULL;
  	DefElem    *dconnlimit = NULL;
  	char	   *dbname = stmt->dbname;
  	char	   *dbowner = NULL;
***************
*** 131,136 **** createdb(const CreatedbStmt *stmt)
--- 133,140 ----
  	char	   *dbctype = NULL;
  	char	   *canonname;
  	int			encoding = -1;
+ 	bool		dbistemplate = false;
+ 	bool		dballowconnections = true;
  	int			dbconnlimit = -1;
  	int			notherbackends;
  	int			npreparedxacts;
***************
*** 189,194 **** createdb(const CreatedbStmt *stmt)
--- 193,214 ----
  						 errmsg("conflicting or redundant options")));
  			dctype = defel;
  		}
+ 		else if (strcmp(defel->defname, "is_template") == 0)
+ 		{
+ 			if (distemplate)
+ 				ereport(ERROR,
+ 						(errcode(ERRCODE_SYNTAX_ERROR),
+ 						 errmsg("conflicting or redundant options")));
+ 			distemplate = defel;
+ 		}
+ 		else if (strcmp(defel->defname, "allow_connections") == 0)
+ 		{
+ 			if (dallowconnections)
+ 				ereport(ERROR,
+ 						(errcode(ERRCODE_SYNTAX_ERROR),
+ 						 errmsg("conflicting or redundant options")));
+ 			dallowconnections = defel;
+ 		}
  		else if (strcmp(defel->defname, "connection_limit") == 0)
  		{
  			if (dconnlimit)
***************
*** 248,253 **** createdb(const CreatedbStmt *stmt)
--- 268,277 ----
  	if (dctype)
  		dbctype = defGetString(dctype);
  
+ 	if (distemplate)
+ 		dbistemplate = defGetBoolean(distemplate);
+ 	if (dallowconnections)
+ 		dballowconnections = defGetBoolean(dallowconnections);
  	if (dconnlimit)
  	{
  		dbconnlimit = defGetInt32(dconnlimit);
***************
*** 490,497 **** createdb(const CreatedbStmt *stmt)
  		DirectFunctionCall1(namein, CStringGetDatum(dbcollate));
  	new_record[Anum_pg_database_datctype - 1] =
  		DirectFunctionCall1(namein, CStringGetDatum(dbctype));
! 	new_record[Anum_pg_database_datistemplate - 1] = BoolGetDatum(false);
! 	new_record[Anum_pg_database_datallowconn - 1] = BoolGetDatum(true);
  	new_record[Anum_pg_database_datconnlimit - 1] = Int32GetDatum(dbconnlimit);
  	new_record[Anum_pg_database_datlastsysoid - 1] = ObjectIdGetDatum(src_lastsysoid);
  	new_record[Anum_pg_database_datfrozenxid - 1] = TransactionIdGetDatum(src_frozenxid);
--- 514,521 ----
  		DirectFunctionCall1(namein, CStringGetDatum(dbcollate));
  	new_record[Anum_pg_database_datctype - 1] =
  		DirectFunctionCall1(namein, CStringGetDatum(dbctype));
! 	new_record[Anum_pg_database_datistemplate - 1] = BoolGetDatum(dbistemplate);
! 	new_record[Anum_pg_database_datallowconn - 1] = BoolGetDatum(dballowconnections);
  	new_record[Anum_pg_database_datconnlimit - 1] = Int32GetDatum(dbconnlimit);
  	new_record[Anum_pg_database_datlastsysoid - 1] = ObjectIdGetDatum(src_lastsysoid);
  	new_record[Anum_pg_database_datfrozenxid - 1] = TransactionIdGetDatum(src_frozenxid);
***************
*** 1331,1337 **** AlterDatabase(AlterDatabaseStmt *stmt, bool isTopLevel)
  	ScanKeyData scankey;
  	SysScanDesc scan;
  	ListCell   *option;
! 	int			connlimit = -1;
  	DefElem    *dconnlimit = NULL;
  	DefElem    *dtablespace = NULL;
  	Datum		new_record[Natts_pg_database];
--- 1355,1365 ----
  	ScanKeyData scankey;
  	SysScanDesc scan;
  	ListCell   *option;
! 	bool		dbistemplate = false;
! 	bool		dballowconnections = true;
! 	int			dbconnlimit = -1;
! 	DefElem	   *distemplate = NULL;
! 	DefElem	   *dallowconnections = NULL;
  	DefElem    *dconnlimit = NULL;
  	DefElem    *dtablespace = NULL;
  	Datum		new_record[Natts_pg_database];
***************
*** 1343,1349 **** AlterDatabase(AlterDatabaseStmt *stmt, bool isTopLevel)
  	{
  		DefElem    *defel = (DefElem *) lfirst(option);
  
! 		if (strcmp(defel->defname, "connection_limit") == 0)
  		{
  			if (dconnlimit)
  				ereport(ERROR,
--- 1371,1393 ----
  	{
  		DefElem    *defel = (DefElem *) lfirst(option);
  
! 		if (strcmp(defel->defname, "is_template") == 0)
! 		{
! 			if (distemplate)
! 				ereport(ERROR,
! 						(errcode(ERRCODE_SYNTAX_ERROR),
! 						 errmsg("conflicting or redundant options")));
! 			distemplate = defel;
! 		}
! 		else if (strcmp(defel->defname, "allow_connections") == 0)
! 		{
! 			if (dallowconnections)
! 				ereport(ERROR,
! 						(errcode(ERRCODE_SYNTAX_ERROR),
! 						 errmsg("conflicting or redundant options")));
! 			dallowconnections = defel;
! 		}
! 		else if (strcmp(defel->defname, "connection_limit") == 0)
  		{
  			if (dconnlimit)
  				ereport(ERROR,
***************
*** 1367,1386 **** AlterDatabase(AlterDatabaseStmt *stmt, bool isTopLevel)
  	if (dtablespace)
  	{
  		/* currently, can't be specified along with any other options */
! 		Assert(!dconnlimit);
  		/* this case isn't allowed within a transaction block */
  		PreventTransactionChain(isTopLevel, "ALTER DATABASE SET TABLESPACE");
  		movedb(stmt->dbname, strVal(dtablespace->arg));
  		return InvalidOid;
  	}
  
  	if (dconnlimit)
  	{
! 		connlimit = defGetInt32(dconnlimit);
! 		if (connlimit < -1)
  			ereport(ERROR,
  					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! 					 errmsg("invalid connection limit: %d", connlimit)));
  	}
  
  	/*
--- 1411,1434 ----
  	if (dtablespace)
  	{
  		/* currently, can't be specified along with any other options */
! 		Assert(!distemplate && !dallowconnections && !dconnlimit);
  		/* this case isn't allowed within a transaction block */
  		PreventTransactionChain(isTopLevel, "ALTER DATABASE SET TABLESPACE");
  		movedb(stmt->dbname, strVal(dtablespace->arg));
  		return InvalidOid;
  	}
  
+ 	if (distemplate)
+ 		dbistemplate = defGetBoolean(distemplate);
+ 	if (dallowconnections)
+ 		dballowconnections = defGetBoolean(dallowconnections);
  	if (dconnlimit)
  	{
! 		dbconnlimit = defGetInt32(dconnlimit);
! 		if (dbconnlimit < -1)
  			ereport(ERROR,
  					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! 					 errmsg("invalid connection limit: %d", dbconnlimit)));
  	}
  
  	/*
***************
*** 1408,1422 **** AlterDatabase(AlterDatabaseStmt *stmt, bool isTopLevel)
  					   stmt->dbname);
  
  	/*
  	 * Build an updated tuple, perusing the information just obtained
  	 */
  	MemSet(new_record, 0, sizeof(new_record));
  	MemSet(new_record_nulls, false, sizeof(new_record_nulls));
  	MemSet(new_record_repl, false, sizeof(new_record_repl));
  
  	if (dconnlimit)
  	{
! 		new_record[Anum_pg_database_datconnlimit - 1] = Int32GetDatum(connlimit);
  		new_record_repl[Anum_pg_database_datconnlimit - 1] = true;
  	}
  
--- 1456,1491 ----
  					   stmt->dbname);
  
  	/*
+ 	 * In order to avoid getting locked out and having to go through standalone
+ 	 * mode, we refuse to disallow connections on the database we're currently
+ 	 * connected to.  Lockout can still happen with concurrent sessions but the
+ 	 * likeliness of that is not high enough to worry about.
+ 	 */
+ 	if (!dballowconnections && dboid == MyDatabaseId)
+ 		ereport(ERROR,
+ 				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ 				 errmsg("cannot disallow connections for current database")));
+ 
+ 	/*
  	 * Build an updated tuple, perusing the information just obtained
  	 */
  	MemSet(new_record, 0, sizeof(new_record));
  	MemSet(new_record_nulls, false, sizeof(new_record_nulls));
  	MemSet(new_record_repl, false, sizeof(new_record_repl));
  
+ 	if (distemplate)
+ 	{
+ 		new_record[Anum_pg_database_datistemplate - 1] = BoolGetDatum(dbistemplate);
+ 		new_record_repl[Anum_pg_database_datistemplate - 1] = true;
+ 	}
+ 	if (dallowconnections)
+ 	{
+ 		new_record[Anum_pg_database_datallowconn - 1] = BoolGetDatum(dballowconnections);
+ 		new_record_repl[Anum_pg_database_datallowconn - 1] = true;
+ 	}
  	if (dconnlimit)
  	{
! 		new_record[Anum_pg_database_datconnlimit - 1] = Int32GetDatum(dbconnlimit);
  		new_record_repl[Anum_pg_database_datconnlimit - 1] = true;
  	}
  
*** a/src/bin/initdb/initdb.c
--- b/src/bin/initdb/initdb.c
***************
*** 2288,2298 **** make_template0(void)
  	PG_CMD_DECL;
  	const char **line;
  	static const char *template0_setup[] = {
! 		"CREATE DATABASE template0;\n",
! 		"UPDATE pg_database SET "
! 		"	datistemplate = 't', "
! 		"	datallowconn = 'f' "
! 		"    WHERE datname = 'template0';\n",
  
  		/*
  		 * We use the OID of template0 to determine lastsysoid
--- 2288,2294 ----
  	PG_CMD_DECL;
  	const char **line;
  	static const char *template0_setup[] = {
! 		"CREATE DATABASE template0 IS_TEMPLATE = true ALLOW_CONNECTIONS = false;\n",
  
  		/*
  		 * We use the OID of template0 to determine lastsysoid
*** a/src/bin/pg_dump/pg_dumpall.c
--- b/src/bin/pg_dump/pg_dumpall.c
***************
*** 1374,1392 **** dumpCreateDB(PGconn *conn)
  				appendPQExpBuffer(buf, " TABLESPACE = %s",
  								  fmtId(dbtablespace));
  
  			if (strcmp(dbconnlimit, "-1") != 0)
  				appendPQExpBuffer(buf, " CONNECTION_LIMIT = %s",
  								  dbconnlimit);
  
  			appendPQExpBufferStr(buf, ";\n");
  
- 			if (strcmp(dbistemplate, "t") == 0)
- 			{
- 				appendPQExpBufferStr(buf, "UPDATE pg_catalog.pg_database SET datistemplate = 't' WHERE datname = ");
- 				appendStringLiteralConn(buf, dbname, conn);
- 				appendPQExpBufferStr(buf, ";\n");
- 			}
- 
  			if (binary_upgrade)
  			{
  				appendPQExpBufferStr(buf, "-- For binary upgrade, set datfrozenxid.\n");
--- 1374,1388 ----
  				appendPQExpBuffer(buf, " TABLESPACE = %s",
  								  fmtId(dbtablespace));
  
+ 			if (strcmp(dbistemplate, "t") == 0)
+ 				appendPQExpBuffer(buf, " IS_TEMPLATE = true");
+ 
  			if (strcmp(dbconnlimit, "-1") != 0)
  				appendPQExpBuffer(buf, " CONNECTION_LIMIT = %s",
  								  dbconnlimit);
  
  			appendPQExpBufferStr(buf, ";\n");
  
  			if (binary_upgrade)
  			{
  				appendPQExpBufferStr(buf, "-- For binary upgrade, set datfrozenxid.\n");
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
***************
*** 1004,1010 **** psql_completion(const char *text, int start, int end)
  			 pg_strcasecmp(prev2_wd, "DATABASE") == 0)
  	{
  		static const char *const list_ALTERDATABASE[] =
! 		{"RESET", "SET", "OWNER TO", "RENAME TO", "CONNECTION_LIMIT", NULL};
  
  		COMPLETE_WITH_LIST(list_ALTERDATABASE);
  	}
--- 1004,1011 ----
  			 pg_strcasecmp(prev2_wd, "DATABASE") == 0)
  	{
  		static const char *const list_ALTERDATABASE[] =
! 		{"RESET", "SET", "OWNER TO", "RENAME TO", "IS_TEMPLATE",
! 		"ALLOW_CONNECTIONS", "CONNECTION_LIMIT", NULL};
  
  		COMPLETE_WITH_LIST(list_ALTERDATABASE);
  	}
***************
*** 2045,2052 **** psql_completion(const char *text, int start, int end)
  			 pg_strcasecmp(prev2_wd, "DATABASE") == 0)
  	{
  		static const char *const list_DATABASE[] =
! 		{"OWNER", "TEMPLATE", "ENCODING", "TABLESPACE", "CONNECTION_LIMIT",
! 		NULL};
  
  		COMPLETE_WITH_LIST(list_DATABASE);
  	}
--- 2046,2053 ----
  			 pg_strcasecmp(prev2_wd, "DATABASE") == 0)
  	{
  		static const char *const list_DATABASE[] =
! 		{"OWNER", "TEMPLATE", "ENCODING", "TABLESPACE", "IS_TEMPLATE",
! 		"ALLOW_CONNECTIONS", "CONNECTION_LIMIT", NULL};
  
  		COMPLETE_WITH_LIST(list_DATABASE);
  	}
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to