pgsql: Add ALTER TABLESPACE ... MOVE command

Lists: pgsql-committerspgsql-hackers
From: Stephen Frost <sfrost(at)snowman(dot)net>
To: pgsql-committers(at)postgresql(dot)org
Subject: pgsql: Add ALTER TABLESPACE ... MOVE command
Date: 2014-01-19 00:11:20
Message-ID: [email protected]
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers

Add ALTER TABLESPACE ... MOVE command

This adds a 'MOVE' sub-command to ALTER TABLESPACE which allows moving sets of
objects from one tablespace to another. This can be extremely handy and avoids
a lot of error-prone scripting. ALTER TABLESPACE ... MOVE will only move
objects the user owns, will notify the user if no objects were found, and can
be used to move ALL objects or specific types of objects (TABLES, INDEXES, or
MATERIALIZED VIEWS).

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/76e91b38ba64e1da70ea21744b342cb105ea3400

Modified Files
--------------
doc/src/sgml/ref/alter_tablespace.sgml | 59 +++++++++-
src/backend/commands/tablespace.c | 171 +++++++++++++++++++++++++++++
src/backend/nodes/copyfuncs.c | 15 +++
src/backend/nodes/equalfuncs.c | 14 +++
src/backend/parser/gram.y | 46 +++++++-
src/backend/tcop/utility.c | 14 +++
src/include/commands/tablespace.h | 1 +
src/include/nodes/nodes.h | 1 +
src/include/nodes/parsenodes.h | 10 ++
src/include/parser/kwlist.h | 1 +
src/test/regress/input/tablespace.source | 7 +-
src/test/regress/output/tablespace.source | 8 +-
src/tools/pgindent/typedefs.list | 1 +
13 files changed, 340 insertions(+), 8 deletions(-)


From: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
To: Stephen Frost <sfrost(at)snowman(dot)net>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: [COMMITTERS] pgsql: Add ALTER TABLESPACE ... MOVE command
Date: 2014-04-03 19:14:32
Message-ID: [email protected]
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers

Stephen Frost wrote:
> Add ALTER TABLESPACE ... MOVE command
>
> This adds a 'MOVE' sub-command to ALTER TABLESPACE which allows moving sets of
> objects from one tablespace to another. This can be extremely handy and avoids
> a lot of error-prone scripting. ALTER TABLESPACE ... MOVE will only move
> objects the user owns, will notify the user if no objects were found, and can
> be used to move ALL objects or specific types of objects (TABLES, INDEXES, or
> MATERIALIZED VIEWS).

I just noticed that this commit added the new commands under the
"ALTER THING name RENAME TO" production (which were originally for
RenameStmt); since commit d86d51a95 had already added some
AlterTableSpaceOptionsStmt nodes to the possible results, maybe this
wasn't so bad in itself; but still it seems quite unlike the way we
organize our parse productions.

If we don't want to add new productions for these new node types, I
think at least this comment update is warranted:

diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 2867fa2..359bb8c 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -7009,6 +7009,8 @@ opt_force: FORCE { $$ = TRUE; }
/*****************************************************************************
*
* ALTER THING name RENAME TO newname
+ * ALTER TABLESPACE name MOVE blah
+ * ALTER TABLESPACE name SET/RESET blah
*
*****************************************************************************/

Other thoughts?

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Cc: Stephen Frost <sfrost(at)snowman(dot)net>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: [COMMITTERS] pgsql: Add ALTER TABLESPACE ... MOVE command
Date: 2014-04-03 21:10:46
Message-ID: [email protected]
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers

Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> writes:
> Stephen Frost wrote:
>> Add ALTER TABLESPACE ... MOVE command

> I just noticed that this commit added the new commands under the
> "ALTER THING name RENAME TO" production (which were originally for
> RenameStmt); since commit d86d51a95 had already added some
> AlterTableSpaceOptionsStmt nodes to the possible results, maybe this
> wasn't so bad in itself; but still it seems quite unlike the way we
> organize our parse productions.

Ick. That's just plain sloppy. Please create a separate production,
*and* a separate comment header.

Commit d86d51a95 was pretty damn awful in this regard as well, but
let's clean them both up, not make it worse.

Existing precedent would suggest inventing two new productions named the
same as the parse node types they produce, viz AlterTableSpaceMoveStmt
and AlterTableSpaceOptionsStmt.

regards, tom lane


From: Stephen Frost <sfrost(at)snowman(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: [COMMITTERS] pgsql: Add ALTER TABLESPACE ... MOVE command
Date: 2014-04-04 20:39:03
Message-ID: [email protected]
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers

* Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
> Commit d86d51a95 was pretty damn awful in this regard as well, but
> let's clean them both up, not make it worse.

Fair enough. I recall being a bit surprised at it also but didn't spend
much time thinking about it. I'll clean it up.

Thanks,

Stephen


From: Stephen Frost <sfrost(at)snowman(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: [COMMITTERS] pgsql: Add ALTER TABLESPACE ... MOVE command
Date: 2014-04-13 05:10:15
Message-ID: [email protected]
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers

* Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
> Ick. That's just plain sloppy. Please create a separate production,
> *and* a separate comment header.

Done.

> Commit d86d51a95 was pretty damn awful in this regard as well, but
> let's clean them both up, not make it worse.

Yeah, I think I noticed this in passing but probably did the same as
Robert and figured "oh, I'm just playing with this, I'll go back and
clean it up later.." and then promptly forgot about it 'cause it worked
just fine.

> Existing precedent would suggest inventing two new productions named the
> same as the parse node types they produce, viz AlterTableSpaceMoveStmt
> and AlterTableSpaceOptionsStmt.

This felt a bit like overkill to me for these few, so I just did them
under a single 'AlterTblSpcStmt'. I'm not against going back and
changing it if others feel differently.

Thanks,

Stephen