On 06/23/2014 06:45 PM, Pavel Stehule wrote:
>
>
>
> 2014-06-23 18:39 GMT+02:00 Vik Fearing <[email protected]
> <mailto:[email protected]>>:
>
> On 06/23/2014 06:21 PM, Robert Haas wrote:
> > On Sun, Jun 22, 2014 at 2:59 PM, Pavel Stehule
> <[email protected] <mailto:[email protected]>> wrote:
> >> I found only one problem - first patch introduce a new property
> >> CONNECTION_LIMIT and replace previously used "CONNECTION LIMIT" in
> >> documentation. But "CONNECTION LIMIT" is still supported, but it
> is not
> >> documented. So for some readers it can look like breaking
> compatibility, but
> >> it is false. This should be documented better.
> >
> > Yeah, I think the old syntax should be documented also.
>
> Why do we want to document syntax that should eventually be deprecated?
>
>
> It is fair to our users. It can be deprecated, ok, we can write in doc -
> this feature will be deprecated in next three years. Don't use it, but
> this should be documentated.
Okay, here is version two of the refactoring patch that documents that
the with-space version is deprecated but still accepted.
The feature patch is not affected by this and so I am not attaching a
new version of 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,117 **** 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
to this database. -1 means no limit.
</para>
</listitem>
</varlistentry>
--- 107,119 ----
</varlistentry>
<varlistentry>
! <term><replaceable class="parameter">connection_limit</replaceable></term>
<listitem>
<para>
How many concurrent connections can be made
to this database. -1 means no limit.
+ The deprecated spelling <command>CONNECTION LIMIT</command> is
+ still accepted.
</para>
</listitem>
</varlistentry>
*** 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,158 **** 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
to this database. -1 (the default) means no limit.
</para>
</listitem>
</varlistentry>
--- 148,160 ----
</varlistentry>
<varlistentry>
! <term><replaceable class="parameter">connection_limit</replaceable></term>
<listitem>
<para>
How many concurrent connections can be made
to this database. -1 (the default) means no limit.
+ The deprecated spelling <command>CONNECTION LIMIT</command> is
+ still accepted.
</para>
</listitem>
</varlistentry>
***************
*** 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.
--- 233,239 ----
</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
***************
*** 264,273 **** 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
--- 264,273 ----
%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
***************
*** 462,467 **** static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
--- 462,468 ----
%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,570 **** 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
--- 565,571 ----
KEY
! LABEL LANGUAGE LARGE_P LAST_P LATERAL_P
LEADING LEAKPROOF LEAST LEFT LEVEL LIKE LIMIT LISTEN LOAD LOCAL
LOCALTIME LOCALTIMESTAMP LOCATION LOCK_P
***************
*** 8320,8397 **** 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.
*/
--- 8321,8372 ----
;
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.
*/
***************
*** 8407,8419 **** 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);
--- 8382,8401 ----
*****************************************************************************/
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);
***************
*** 8435,8453 **** 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 ]
--- 8417,8422 ----
***************
*** 12958,12965 **** unreserved_keyword:
| LANGUAGE
| LARGE_P
| LAST_P
- | LC_COLLATE_P
- | LC_CTYPE_P
| LEAKPROOF
| LEVEL
| LISTEN
--- 12927,12932 ----
*** 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
***************
*** 1021,1027 **** 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);
}
--- 1021,1027 ----
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);
}
***************
*** 2111,2117 **** 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);
--- 2111,2117 ----
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 ----
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
