Re: [HACKERS] [COMMITTERS] pgsql: Add ALTER TABLESPACE ... MOVE command
* Tom Lane (t...@sss.pgh.pa.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 signature.asc Description: Digital signature
Re: [HACKERS] [COMMITTERS] pgsql: Add ALTER TABLESPACE ... MOVE command
* Tom Lane (t...@sss.pgh.pa.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 signature.asc Description: Digital signature
Re: [HACKERS] [COMMITTERS] pgsql: Add ALTER TABLESPACE ... MOVE command
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 Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [COMMITTERS] pgsql: Add ALTER TABLESPACE ... MOVE command
Alvaro Herrera alvhe...@2ndquadrant.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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
