On 22/11/11 17:24, Mark Kirkwood wrote:
On 22/11/11 16:41, Tom Lane wrote:
Mark Kirkwood<mark.kirkw...@catalyst.net.nz> writes:
I've been helping out several customers recently who all seem to be
wrestling with the same issue: wanting to update/refresh non-production
databases from the latest corresponding prod version. Typically they
have (fairly complex) scripts that at some point attempt to restore a
dump into new database and then rename the to-be-retired db out of the
way and rename the newly restored one to take over.
In many cases such scripts would be simplified if a database could be
renamed without requiring its connections terminated. I've been asked
several times if this could be added... so I've caved in a done a patch
that allows this.
The default behavior is unchanged - it is required to specify an
additional trailing FORCE keyword to elicit the more brutal behavior.
Note that existing connections to the renamed database are unaffected,
but obviously SELECT current_database() returns the new name (in the
next transaction).
This patch seems to me to be pretty thoroughly misguided. Either
renaming a database with open connections is safe, or it isn't. If it
is safe, we should just allow it. If it isn't, making people write an
extra FORCE keyword does not make it safe. It's particularly silly
to allow someone to rename the database out from under other sessions
(which won't know what happened) but not rename it out from under his
own session (which would or at least could know it).
What you need to be doing is investigating whether the comments about
this in RenameDatabase() are really valid concerns or not.
The reason I added FORCE was to preserve backwards compatibility - for
any people out there that like the way it behaves right now. I am
certainly willing to be convinced that such a concern is unneeded.
You are quite right about the patch being inconsistent with respect to
the renaming the current database, it should allow that too (will
change if this overall approach makes sense).
With respect to the concerns in RenameDatabase(), that seems to boil
down to applications stashing the current dbname somewhere and caring
about it. This was not viewed as a issue by any of the folks who I
talked to about this (they are all application developers/architects
etc so they understand that issue). However there may well be
application frameworks out there that do care... which seemed to me to
be another reason for making the forced rename require an extra keyword.
I have not been able to find any other problems caused by this...
renaming a db (many times) with hundreds of pgbench connections does
not give rise to any issues.
Minor change to be allow current database to be renamed as well if FORCE
is used, which makes more sense.
Cheers
Mark
diff --git a/doc/src/sgml/ref/alter_database.sgml
b/doc/src/sgml/ref/alter_database.sgml
new file mode 100644
index 360732f..98ea473
*** a/doc/src/sgml/ref/alter_database.sgml
--- b/doc/src/sgml/ref/alter_database.sgml
*************** ALTER DATABASE <replaceable class="PARAM
*** 27,33 ****
CONNECTION LIMIT <replaceable class="PARAMETER">connlimit</replaceable>
! ALTER DATABASE <replaceable class="PARAMETER">name</replaceable> RENAME TO
<replaceable>new_name</replaceable>
ALTER DATABASE <replaceable class="PARAMETER">name</replaceable> OWNER TO
<replaceable>new_owner</replaceable>
--- 27,33 ----
CONNECTION LIMIT <replaceable class="PARAMETER">connlimit</replaceable>
! ALTER DATABASE <replaceable class="PARAMETER">name</replaceable> RENAME TO
<replaceable>new_name</replaceable> [ FORCE ]
ALTER DATABASE <replaceable class="PARAMETER">name</replaceable> OWNER TO
<replaceable>new_owner</replaceable>
*************** ALTER DATABASE <replaceable class="PARAM
*** 57,65 ****
The second form changes the name of the database. Only the database
owner or a superuser can rename a database; non-superuser owners must
also have the
! <literal>CREATEDB</literal> privilege. The current database cannot
! be renamed. (Connect to a different database if you need to do
! that.)
</para>
<para>
--- 57,69 ----
The second form changes the name of the database. Only the database
owner or a superuser can rename a database; non-superuser owners must
also have the
! <literal>CREATEDB</literal> privilege. If <literal>FORCE</literal>
! is not specified the current database cannot be renamed (Connect to
! a different database if you need to do that).
! Similarly any database that has connections cannot be renamed.
! If <literal>FORCE</literal> is specified the previous two restrictions
! do not apply and the rename will be performed (existing connections
! will not be disconnected or canceled).
</para>
<para>
diff --git a/src/backend/commands/alter.c b/src/backend/commands/alter.c
new file mode 100644
index c321224..bdc5f12
*** a/src/backend/commands/alter.c
--- b/src/backend/commands/alter.c
*************** ExecRenameStmt(RenameStmt *stmt)
*** 62,68 ****
break;
case OBJECT_DATABASE:
! RenameDatabase(stmt->subname, stmt->newname);
break;
case OBJECT_FUNCTION:
--- 62,68 ----
break;
case OBJECT_DATABASE:
! RenameDatabase(stmt->subname, stmt->newname,
stmt->force);
break;
case OBJECT_FUNCTION:
diff --git a/src/backend/commands/dbcommands.c
b/src/backend/commands/dbcommands.c
new file mode 100644
index 4551db7..217f7ca
*** a/src/backend/commands/dbcommands.c
--- b/src/backend/commands/dbcommands.c
*************** dropdb(const char *dbname, bool missing_
*** 886,892 ****
* Rename database
*/
void
! RenameDatabase(const char *oldname, const char *newname)
{
Oid db_id;
HeapTuple newtup;
--- 886,892 ----
* Rename database
*/
void
! RenameDatabase(const char *oldname, const char *newname, bool force)
{
Oid db_id;
HeapTuple newtup;
*************** RenameDatabase(const char *oldname, cons
*** 927,954 ****
errmsg("database \"%s\" already exists",
newname)));
/*
* XXX Client applications probably store the current database
somewhere,
* so renaming it could cause confusion. On the other hand, there may
not
* be an actual problem besides a little confusion, so think about this
* and decide.
*/
! if (db_id == MyDatabaseId)
! ereport(ERROR,
! (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
! errmsg("current database cannot be renamed")));
/*
! * Make sure the database does not have active sessions. This is the
same
! * concern as above, but applied to other sessions.
*
* As in CREATE DATABASE, check this after other error conditions.
*/
! if (CountOtherDBBackends(db_id, ¬herbackends, &npreparedxacts))
! ereport(ERROR,
! (errcode(ERRCODE_OBJECT_IN_USE),
! errmsg("database \"%s\" is being accessed by
other users",
! oldname),
! errdetail_busy_db(notherbackends,
npreparedxacts)));
/* rename */
newtup = SearchSysCacheCopy1(DATABASEOID, ObjectIdGetDatum(db_id));
--- 927,958 ----
errmsg("database \"%s\" already exists",
newname)));
/*
+ * Don't allow current database to be renamed unless force was
specified.
* XXX Client applications probably store the current database
somewhere,
* so renaming it could cause confusion. On the other hand, there may
not
* be an actual problem besides a little confusion, so think about this
* and decide.
*/
! if (!force)
! if (db_id == MyDatabaseId)
! ereport(ERROR,
! (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
! errmsg("current database cannot be
renamed")));
/*
! * Make sure the database does not have active sessions unless force was
! * specified. This is the same concern as above, but applied to other
! * sessions.
*
* As in CREATE DATABASE, check this after other error conditions.
*/
! if (!force)
! if (CountOtherDBBackends(db_id, ¬herbackends,
&npreparedxacts))
! ereport(ERROR,
! (errcode(ERRCODE_OBJECT_IN_USE),
! errmsg("database \"%s\" is being
accessed by other users",
! oldname),
! errdetail_busy_db(notherbackends,
npreparedxacts)));
/* rename */
newtup = SearchSysCacheCopy1(DATABASEOID, ObjectIdGetDatum(db_id));
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
new file mode 100644
index c135465..d3ad439
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
*************** reindex_type:
*** 6404,6412 ****
| TABLE
{ $$ = OBJECT_TABLE; }
;
- opt_force: FORCE
{ $$ = TRUE; }
- | /* EMPTY */
{ $$ = FALSE; }
- ;
/*****************************************************************************
--- 6404,6409 ----
*************** RenameStmt: ALTER AGGREGATE func_name ag
*** 6440,6451 ****
n->newname = $6;
$$ = (Node *)n;
}
! | ALTER DATABASE database_name RENAME TO database_name
{
RenameStmt *n = makeNode(RenameStmt);
n->renameType = OBJECT_DATABASE;
n->subname = $3;
n->newname = $6;
$$ = (Node *)n;
}
| ALTER FUNCTION function_with_argtypes RENAME TO name
--- 6437,6449 ----
n->newname = $6;
$$ = (Node *)n;
}
! | ALTER DATABASE database_name RENAME TO database_name
opt_force
{
RenameStmt *n = makeNode(RenameStmt);
n->renameType = OBJECT_DATABASE;
n->subname = $3;
n->newname = $6;
+ n->force = $7;
$$ = (Node *)n;
}
| ALTER FUNCTION function_with_argtypes RENAME TO name
*************** opt_column: COLUMN
{ $$ = COLUMN
*** 6675,6680 ****
--- 6673,6681 ----
opt_set_data: SET DATA_P
{ $$ = 1; }
| /*EMPTY*/
{ $$ = 0; }
;
+ opt_force: FORCE
{ $$ = TRUE; }
+ | /* EMPTY */
{ $$ = FALSE; }
+ ;
/*****************************************************************************
*
diff --git a/src/include/commands/dbcommands.h
b/src/include/commands/dbcommands.h
new file mode 100644
index 21dacff..68141b8
*** a/src/include/commands/dbcommands.h
--- b/src/include/commands/dbcommands.h
*************** typedef struct xl_dbase_drop_rec
*** 54,60 ****
extern void createdb(const CreatedbStmt *stmt);
extern void dropdb(const char *dbname, bool missing_ok);
! extern void RenameDatabase(const char *oldname, const char *newname);
extern void AlterDatabase(AlterDatabaseStmt *stmt, bool isTopLevel);
extern void AlterDatabaseSet(AlterDatabaseSetStmt *stmt);
extern void AlterDatabaseOwner(const char *dbname, Oid newOwnerId);
--- 54,60 ----
extern void createdb(const CreatedbStmt *stmt);
extern void dropdb(const char *dbname, bool missing_ok);
! extern void RenameDatabase(const char *oldname, const char *newname, bool
force);
extern void AlterDatabase(AlterDatabaseStmt *stmt, bool isTopLevel);
extern void AlterDatabaseSet(AlterDatabaseSetStmt *stmt);
extern void AlterDatabaseOwner(const char *dbname, Oid newOwnerId);
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
new file mode 100644
index af6565e..8d30af9
*** a/src/include/nodes/parsenodes.h
--- b/src/include/nodes/parsenodes.h
*************** typedef struct RenameStmt
*** 2193,2198 ****
--- 2193,2199 ----
* trigger,
etc) */
char *newname; /* the new name */
DropBehavior behavior; /* RESTRICT or CASCADE behavior */
+ bool force; /* with prejudice */
} RenameStmt;
/* ----------------------
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
