Re: [HACKERS] ALTER TYPE recursion to typed tables
On ons, 2010-11-17 at 21:05 +0100, Dimitri Fontaine wrote: Code wise, though, I wonder about the name of the recursing parameter of the renameatt_internal function is src/backend/commands/tablecmds.c, which seems to only get used to detect erroneous attempt at renaming the table column directly. Maybe it's only me not used enough to PostgreSQL code yet, but here it distract the code reader. Having another parameter called recurse is not helping, too, but I don't see this one needs to be changed. This parameter has only minimal use in the renameatt case, but the same terminology is used throughout the ALTER TABLE code, so I think it's wise to keep it. -- 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] ALTER TYPE recursion to typed tables
Peter Eisentraut pete...@gmx.net writes: Here is the patch that adds [RESTRICT|CASCADE] to ALTER TYPE ... ADD/ALTER/DROP/RENAME ATTRIBUTE, so that recurses to typed tables. And here's my commitfest review of it: - patch applies cleanly - adds regression tests - passes them - is useful and needed, and something we don't already have - don't generate warnings (or I missed them) :) Code wise, though, I wonder about the name of the recursing parameter of the renameatt_internal function is src/backend/commands/tablecmds.c, which seems to only get used to detect erroneous attempt at renaming the table column directly. Maybe it's only me not used enough to PostgreSQL code yet, but here it distract the code reader. Having another parameter called recurse is not helping, too, but I don't see this one needs to be changed. I'm not sure what a good name would be here, alter_type_cascade is an example that comes to mind, on the verbose side. As I think the issue is to be decided by a commiter, I will go and mark this patch as ready for commiter! Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] ALTER TYPE recursion to typed tables
Here is the patch that adds [RESTRICT|CASCADE] to ALTER TYPE ...
ADD/ALTER/DROP/RENAME ATTRIBUTE, so that recurses to typed tables.
diff --git a/doc/src/sgml/ref/alter_type.sgml b/doc/src/sgml/ref/alter_type.sgml
index 90de2e8..04395c9 100644
--- a/doc/src/sgml/ref/alter_type.sgml
+++ b/doc/src/sgml/ref/alter_type.sgml
@@ -26,15 +26,15 @@ PostgreSQL documentation
ALTER TYPE replaceable class=PARAMETERname/replaceable replaceable class=PARAMETERaction/replaceable [, ... ]
ALTER TYPE replaceable class=PARAMETERname/replaceable OWNER TO replaceable class=PARAMETERnew_owner/replaceable
ALTER TYPE replaceable class=PARAMETERname/replaceable RENAME ATTRIBUTE replaceable class=PARAMETERattribute_name/replaceable TO replaceable class=PARAMETERnew_attribute_name/replaceable
-ALTER TYPE replaceable class=PARAMETERname/replaceable RENAME TO replaceable class=PARAMETERnew_name/replaceable
+ALTER TYPE replaceable class=PARAMETERname/replaceable RENAME TO replaceable class=PARAMETERnew_name/replaceable [ CASCADE | RESTRICT ]
ALTER TYPE replaceable class=PARAMETERname/replaceable SET SCHEMA replaceable class=PARAMETERnew_schema/replaceable
ALTER TYPE replaceable class=PARAMETERname/replaceable ADD replaceable class=PARAMETERnew_enum_value/replaceable [ { BEFORE | AFTER } replaceable class=PARAMETERexisting_enum_value/replaceable ]
phrasewhere replaceable class=PARAMETERaction/replaceable is one of:/phrase
-ADD ATTRIBUTE replaceable class=PARAMETERattribute_name/replaceable replaceable class=PARAMETERdata_type/replaceable
-DROP ATTRIBUTE [ IF EXISTS ] replaceable class=PARAMETERattribute_name/replaceable
-ALTER ATTRIBUTE replaceable class=PARAMETERattribute_name/replaceable [ SET DATA ] TYPE replaceable class=PARAMETERdata_type/replaceable
+ADD ATTRIBUTE replaceable class=PARAMETERattribute_name/replaceable replaceable class=PARAMETERdata_type/replaceable [ CASCADE | RESTRICT ]
+DROP ATTRIBUTE [ IF EXISTS ] replaceable class=PARAMETERattribute_name/replaceable [ CASCADE | RESTRICT ]
+ALTER ATTRIBUTE replaceable class=PARAMETERattribute_name/replaceable [ SET DATA ] TYPE replaceable class=PARAMETERdata_type/replaceable [ CASCADE | RESTRICT ]
/synopsis
/refsynopsisdiv
@@ -116,6 +116,26 @@ ALTER TYPE replaceable class=PARAMETERname/replaceable ADD replaceable cl
/para
/listitem
/varlistentry
+
+ varlistentry
+termliteralCASCADE/literal/term
+listitem
+ para
+ Automatically propagate the operation to typed tables of the
+ type being altered.
+ /para
+/listitem
+ /varlistentry
+
+ varlistentry
+termliteralRESTRICT/literal/term
+listitem
+ para
+ Refuse the operation if the type being altered is the type of a
+ typed table. This is the default.
+ /para
+/listitem
+ /varlistentry
/variablelist
/para
diff --git a/src/backend/commands/alter.c b/src/backend/commands/alter.c
index 794d355..0d0227d 100644
--- a/src/backend/commands/alter.c
+++ b/src/backend/commands/alter.c
@@ -125,11 +125,7 @@ ExecRenameStmt(RenameStmt *stmt)
}
case OBJECT_COLUMN:
case OBJECT_ATTRIBUTE:
- renameatt(relid,
- stmt-subname, /* old att name */
- stmt-newname, /* new att name */
- interpretInhOption(stmt-relation-inhOpt), /* recursive? */
- 0); /* expected inhcount */
+ renameatt(relid, stmt);
break;
case OBJECT_TRIGGER:
renametrig(relid,
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 6ec8a85..2b35943 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -269,8 +269,11 @@ static void ATSimpleRecursion(List **wqueue, Relation rel,
AlterTableCmd *cmd, bool recurse, LOCKMODE lockmode);
static void ATOneLevelRecursion(List **wqueue, Relation rel,
AlterTableCmd *cmd, LOCKMODE lockmode);
-static void find_typed_table_dependencies(Oid typeOid, const char *typeName);
-static void ATPrepAddColumn(List **wqueue, Relation rel, bool recurse,
+static void ATTypedTableRecursion(List **wqueue, Relation rel, AlterTableCmd *cmd,
+ LOCKMODE lockmode);
+static List *find_typed_table_dependencies(Oid typeOid, const char *typeName,
+ DropBehavior behavior);
+static void ATPrepAddColumn(List **wqueue, Relation rel, bool recurse, bool recursing,
AlterTableCmd *cmd, LOCKMODE lockmode);
static void ATExecAddColumn(AlteredTableInfo *tab, Relation rel,
ColumnDef *colDef, bool isOid, LOCKMODE lockmode);
@@ -290,7 +293,8 @@ static void ATExecSetOptions(Relation rel, const char *colName,
Node *options, bool isReset, LOCKMODE lockmode);
static void ATExecSetStorage(Relation rel, const char *colName,
Node *newValue, LOCKMODE lockmode);
-static void ATPrepDropColumn(Relation rel, bool recurse, AlterTableCmd *cmd);
+static void ATPrepDropColumn(List **wqueue, Relation rel, bool recurse, bool recursing,
+ AlterTableCmd *cmd, LOCKMODE
[HACKERS] ALTER TYPE recursion to typed tables
I'm working on propagating ALTER TYPE commands to typed tables. This is currently prohibited. For example, take these regression test cases: CREATE TYPE test_type2 AS (a int, b text); CREATE TABLE test_tbl2 OF test_type2; ALTER TYPE test_type2 ADD ATTRIBUTE c text; -- fails ALTER TYPE test_type2 ALTER ATTRIBUTE b TYPE varchar; -- fails ALTER TYPE test_type2 DROP ATTRIBUTE b; -- fails ALTER TYPE test_type2 RENAME ATTRIBUTE b TO bb; -- fails The actual implementation isn't very difficult, because the ALTER TABLE code already knows everything about recursion. Now I'm wondering what kind of syntax should be used to control this. I think you don't want to automatically propagate such innocent looking operations to tables in a potentially data-destroying manner. The natural idea would be RESTRICT/CASCADE. This is currently only associated with DROP operations, but I suppose ADD/ALTER/RENAME ATTRIBUTE x ... CASCADE doesn't sound too odd. Comments, other ideas? -- 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] ALTER TYPE recursion to typed tables
On Tue, Nov 2, 2010 at 9:15 AM, Peter Eisentraut pete...@gmx.net wrote: I'm working on propagating ALTER TYPE commands to typed tables. This is currently prohibited. For example, take these regression test cases: CREATE TYPE test_type2 AS (a int, b text); CREATE TABLE test_tbl2 OF test_type2; ALTER TYPE test_type2 ADD ATTRIBUTE c text; -- fails ALTER TYPE test_type2 ALTER ATTRIBUTE b TYPE varchar; -- fails ALTER TYPE test_type2 DROP ATTRIBUTE b; -- fails ALTER TYPE test_type2 RENAME ATTRIBUTE b TO bb; -- fails The actual implementation isn't very difficult, because the ALTER TABLE code already knows everything about recursion. Now I'm wondering what kind of syntax should be used to control this. I think you don't want to automatically propagate such innocent looking operations to tables in a potentially data-destroying manner. The natural idea would be RESTRICT/CASCADE. This is currently only associated with DROP operations, but I suppose ADD/ALTER/RENAME ATTRIBUTE x ... CASCADE doesn't sound too odd. Comments, other ideas? That seems reasonable. What do you plan to do about this case? CREATE TYPE test_type AS (a int, b text); CREATE TABLE test_tbl (x test_type); ALTER TYPE test_type ADD ATTRIBUTE c text; -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- 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] ALTER TYPE recursion to typed tables
On tis, 2010-11-02 at 10:54 -0700, Robert Haas wrote: What do you plan to do about this case? CREATE TYPE test_type AS (a int, b text); CREATE TABLE test_tbl (x test_type); ALTER TYPE test_type ADD ATTRIBUTE c text; This is currently prohibited, and I'm not planning to do anything about that. -- 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] ALTER TYPE recursion to typed tables
On Nov 2, 2010, at 11:17 AM, Peter Eisentraut pete...@gmx.net wrote: On tis, 2010-11-02 at 10:54 -0700, Robert Haas wrote: What do you plan to do about this case? CREATE TYPE test_type AS (a int, b text); CREATE TABLE test_tbl (x test_type); ALTER TYPE test_type ADD ATTRIBUTE c text; This is currently prohibited, and I'm not planning to do anything about that. OK. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
