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?
On Tue, Nov 2, 2010 at 9:15 AM, Peter Eisentraut <peter_e@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; -- failsThe 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
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.
On Nov 2, 2010, at 11:17 AM, Peter Eisentraut <peter_e@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
Here is the patch that adds [RESTRICT|CASCADE] to ALTER TYPE ...
ADD/ALTER/DROP/RENAME ATTRIBUTE, so that recurses to typed tables.
Attachments:
alter-type-recursive.patchtext/x-patch; charset=UTF-8; name=alter-type-recursive.patchDownload
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="PARAMETER">name</replaceable> <replaceable class="PARAMETER">action</replaceable> [, ... ]
ALTER TYPE <replaceable class="PARAMETER">name</replaceable> OWNER TO <replaceable class="PARAMETER">new_owner</replaceable>
ALTER TYPE <replaceable class="PARAMETER">name</replaceable> RENAME ATTRIBUTE <replaceable class="PARAMETER">attribute_name</replaceable> TO <replaceable class="PARAMETER">new_attribute_name</replaceable>
-ALTER TYPE <replaceable class="PARAMETER">name</replaceable> RENAME TO <replaceable class="PARAMETER">new_name</replaceable>
+ALTER TYPE <replaceable class="PARAMETER">name</replaceable> RENAME TO <replaceable class="PARAMETER">new_name</replaceable> [ CASCADE | RESTRICT ]
ALTER TYPE <replaceable class="PARAMETER">name</replaceable> SET SCHEMA <replaceable class="PARAMETER">new_schema</replaceable>
ALTER TYPE <replaceable class="PARAMETER">name</replaceable> ADD <replaceable class="PARAMETER">new_enum_value</replaceable> [ { BEFORE | AFTER } <replaceable class="PARAMETER">existing_enum_value</replaceable> ]
<phrase>where <replaceable class="PARAMETER">action</replaceable> is one of:</phrase>
- ADD ATTRIBUTE <replaceable class="PARAMETER">attribute_name</replaceable> <replaceable class="PARAMETER">data_type</replaceable>
- DROP ATTRIBUTE [ IF EXISTS ] <replaceable class="PARAMETER">attribute_name</replaceable>
- ALTER ATTRIBUTE <replaceable class="PARAMETER">attribute_name</replaceable> [ SET DATA ] TYPE <replaceable class="PARAMETER">data_type</replaceable>
+ ADD ATTRIBUTE <replaceable class="PARAMETER">attribute_name</replaceable> <replaceable class="PARAMETER">data_type</replaceable> [ CASCADE | RESTRICT ]
+ DROP ATTRIBUTE [ IF EXISTS ] <replaceable class="PARAMETER">attribute_name</replaceable> [ CASCADE | RESTRICT ]
+ ALTER ATTRIBUTE <replaceable class="PARAMETER">attribute_name</replaceable> [ SET DATA ] TYPE <replaceable class="PARAMETER">data_type</replaceable> [ CASCADE | RESTRICT ]
</synopsis>
</refsynopsisdiv>
@@ -116,6 +116,26 @@ ALTER TYPE <replaceable class="PARAMETER">name</replaceable> ADD <replaceable cl
</para>
</listitem>
</varlistentry>
+
+ <varlistentry>
+ <term><literal>CASCADE</literal></term>
+ <listitem>
+ <para>
+ Automatically propagate the operation to typed tables of the
+ type being altered.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><literal>RESTRICT</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 lockmode);
static void ATExecDropColumn(List **wqueue, Relation rel, const char *colName,
DropBehavior behavior,
bool recurse, bool recursing,
@@ -1952,14 +1956,16 @@ setRelhassubclassInRelation(Oid relationId, bool relhassubclass)
/*
- * renameatt - changes the name of a attribute in a relation
+ * renameatt_internal - workhorse for renameatt
*/
-void
-renameatt(Oid myrelid,
- const char *oldattname,
- const char *newattname,
- bool recurse,
- int expected_parents)
+static void
+renameatt_internal(Oid myrelid,
+ const char *oldattname,
+ const char *newattname,
+ bool recurse,
+ bool recursing,
+ int expected_parents,
+ DropBehavior behavior)
{
Relation targetrelation;
Relation attrelation;
@@ -1974,15 +1980,11 @@ renameatt(Oid myrelid,
*/
targetrelation = relation_open(myrelid, AccessExclusiveLock);
- if (targetrelation->rd_rel->reloftype)
+ if (targetrelation->rd_rel->reloftype && !recursing)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("cannot rename column of typed table")));
- if (targetrelation->rd_rel->relkind == RELKIND_COMPOSITE_TYPE)
- find_typed_table_dependencies(targetrelation->rd_rel->reltype,
- RelationGetRelationName(targetrelation));
-
/*
* Renaming the columns of sequences or toast tables doesn't actually
* break anything from the system's point of view, since internal
@@ -2048,7 +2050,7 @@ renameatt(Oid myrelid,
if (childrelid == myrelid)
continue;
/* note we need not recurse again */
- renameatt(childrelid, oldattname, newattname, false, numparents);
+ renameatt_internal(childrelid, oldattname, newattname, false, true, numparents, behavior);
}
}
else
@@ -2067,6 +2069,20 @@ renameatt(Oid myrelid,
oldattname)));
}
+ /* rename attributes in typed tables of composite type */
+ if (targetrelation->rd_rel->relkind == RELKIND_COMPOSITE_TYPE)
+ {
+ List *child_oids;
+ ListCell *lo;
+
+ child_oids = find_typed_table_dependencies(targetrelation->rd_rel->reltype,
+ RelationGetRelationName(targetrelation),
+ behavior);
+
+ foreach(lo, child_oids)
+ renameatt_internal(lfirst_oid(lo), oldattname, newattname, true, true, 0, behavior);
+ }
+
attrelation = heap_open(AttributeRelationId, RowExclusiveLock);
atttup = SearchSysCacheCopyAttName(myrelid, oldattname);
@@ -2127,6 +2143,22 @@ renameatt(Oid myrelid,
/*
+ * renameatt - changes the name of a attribute in a relation
+ */
+void
+renameatt(Oid myrelid, RenameStmt *stmt)
+{
+ renameatt_internal(myrelid,
+ stmt->subname, /* old att name */
+ stmt->newname, /* new att name */
+ interpretInhOption(stmt->relation->inhOpt), /* recursive? */
+ false, /* recursing? */
+ 0, /* expected inhcount */
+ stmt->behavior);
+}
+
+
+/*
* Execute ALTER TABLE/INDEX/SEQUENCE/VIEW RENAME
*
* Caller has already done permissions checks.
@@ -2659,14 +2691,14 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
case AT_AddColumn: /* ADD COLUMN */
ATSimplePermissions(rel, false, true);
/* Performs own recursion */
- ATPrepAddColumn(wqueue, rel, recurse, cmd, lockmode);
+ ATPrepAddColumn(wqueue, rel, recurse, recursing, cmd, lockmode);
pass = AT_PASS_ADD_COL;
break;
case AT_AddColumnToView: /* add column via CREATE OR REPLACE
* VIEW */
ATSimplePermissions(rel, true, false);
/* Performs own recursion */
- ATPrepAddColumn(wqueue, rel, recurse, cmd, lockmode);
+ ATPrepAddColumn(wqueue, rel, recurse, recursing, cmd, lockmode);
pass = AT_PASS_ADD_COL;
break;
case AT_ColumnDefault: /* ALTER COLUMN DEFAULT */
@@ -2714,7 +2746,7 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
break;
case AT_DropColumn: /* DROP COLUMN */
ATSimplePermissions(rel, false, true);
- ATPrepDropColumn(rel, recurse, cmd);
+ ATPrepDropColumn(wqueue, rel, recurse, recursing, cmd, lockmode);
/* Recursion occurs during execution phase */
pass = AT_PASS_DROP;
break;
@@ -3681,6 +3713,37 @@ ATOneLevelRecursion(List **wqueue, Relation rel,
}
}
+/*
+ * ATTypedTableRecursion
+ *
+ * Propagate ALTER TYPE operations to the typed tables of that type.
+ * Also check the RESTRICT/CASCADE behavior.
+ */
+static void
+ATTypedTableRecursion(List **wqueue, Relation rel, AlterTableCmd *cmd,
+ LOCKMODE lockmode)
+{
+ ListCell *child;
+ List *children;
+
+ Assert(rel->rd_rel->relkind == RELKIND_COMPOSITE_TYPE);
+
+ children = find_typed_table_dependencies(rel->rd_rel->reltype,
+ RelationGetRelationName(rel),
+ cmd->behavior);
+
+ foreach(child, children)
+ {
+ Oid childrelid = lfirst_oid(child);
+ Relation childrel;
+
+ childrel = relation_open(childrelid, lockmode);
+ CheckTableNotInUse(childrel, "ALTER TABLE");
+ ATPrepCmd(wqueue, childrel, cmd, false, true, lockmode);
+ relation_close(childrel, NoLock);
+ }
+}
+
/*
* find_composite_type_dependencies
@@ -3788,17 +3851,17 @@ find_composite_type_dependencies(Oid typeOid,
* find_typed_table_dependencies
*
* Check to see if a composite type is being used as the type of a
- * typed table. Eventually, we'd like to propagate the alter
- * operation into such tables, but for now, just error out if we find
- * any.
+ * typed table. Abort if any are found and behavior is RESTRICT.
+ * Else return the list of tables.
*/
-static void
-find_typed_table_dependencies(Oid typeOid, const char *typeName)
+static List *
+find_typed_table_dependencies(Oid typeOid, const char *typeName, DropBehavior behavior)
{
Relation classRel;
ScanKeyData key[1];
HeapScanDesc scan;
HeapTuple tuple;
+ List *result = NIL;
classRel = heap_open(RelationRelationId, AccessShareLock);
@@ -3811,14 +3874,20 @@ find_typed_table_dependencies(Oid typeOid, const char *typeName)
if (HeapTupleIsValid(tuple = heap_getnext(scan, ForwardScanDirection)))
{
- ereport(ERROR,
- (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("cannot alter type \"%s\" because it is the type of a typed table",
- typeName)));
+ if (behavior == DROP_RESTRICT)
+ ereport(ERROR,
+ (errcode(ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST),
+ errmsg("cannot alter type \"%s\" because it is the type of a typed table",
+ typeName),
+ errhint("Use ALTER ... CASCADE to alter the typed tables too.")));
+ else
+ result = lappend_oid(result, HeapTupleGetOid(tuple));
}
heap_endscan(scan);
heap_close(classRel, AccessShareLock);
+
+ return result;
}
@@ -3831,10 +3900,10 @@ find_typed_table_dependencies(Oid typeOid, const char *typeName)
* AlterTableCmd's.
*/
static void
-ATPrepAddColumn(List **wqueue, Relation rel, bool recurse,
+ATPrepAddColumn(List **wqueue, Relation rel, bool recurse, bool recursing,
AlterTableCmd *cmd, LOCKMODE lockmode)
{
- if (rel->rd_rel->reloftype)
+ if (rel->rd_rel->reloftype && !recursing)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("cannot add column to typed table")));
@@ -3870,8 +3939,7 @@ ATPrepAddColumn(List **wqueue, Relation rel, bool recurse,
}
if (rel->rd_rel->relkind == RELKIND_COMPOSITE_TYPE)
- find_typed_table_dependencies(rel->rd_rel->reltype,
- RelationGetRelationName(rel));
+ ATTypedTableRecursion(wqueue, rel, cmd, lockmode);
}
static void
@@ -4172,7 +4240,7 @@ ATPrepAddOids(List **wqueue, Relation rel, bool recurse, AlterTableCmd *cmd, LOC
cdef->storage = 0;
cmd->def = (Node *) cdef;
}
- ATPrepAddColumn(wqueue, rel, recurse, cmd, lockmode);
+ ATPrepAddColumn(wqueue, rel, recurse, false, cmd, lockmode);
}
/*
@@ -4596,18 +4664,17 @@ ATExecSetStorage(Relation rel, const char *colName, Node *newValue, LOCKMODE loc
* correctly.)
*/
static void
-ATPrepDropColumn(Relation rel, bool recurse, AlterTableCmd *cmd)
+ATPrepDropColumn(List **wqueue, Relation rel, bool recurse, bool recursing,
+ AlterTableCmd *cmd, LOCKMODE lockmode)
{
- if (rel->rd_rel->reloftype)
+ if (rel->rd_rel->reloftype && !recursing)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("cannot drop column from typed table")));
if (rel->rd_rel->relkind == RELKIND_COMPOSITE_TYPE)
- find_typed_table_dependencies(rel->rd_rel->reltype,
- RelationGetRelationName(rel));
+ ATTypedTableRecursion(wqueue, rel, cmd, lockmode);
- /* No command-specific prep needed except saving recurse flag */
if (recurse)
cmd->subtype = AT_DropColumnRecurse;
}
@@ -6070,7 +6137,7 @@ ATPrepAlterColumnType(List **wqueue,
NewColumnValue *newval;
ParseState *pstate = make_parsestate(NULL);
- if (rel->rd_rel->reloftype)
+ if (rel->rd_rel->reloftype && !recursing)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("cannot alter column type of typed table")));
@@ -6188,9 +6255,6 @@ ATPrepAlterColumnType(List **wqueue,
find_composite_type_dependencies(rel->rd_rel->reltype,
NULL,
RelationGetRelationName(rel));
-
- find_typed_table_dependencies(rel->rd_rel->reltype,
- RelationGetRelationName(rel));
}
ReleaseSysCache(tuple);
@@ -6208,6 +6272,9 @@ ATPrepAlterColumnType(List **wqueue,
(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
errmsg("type of inherited column \"%s\" must be changed in child tables too",
colName)));
+
+ if (tab->relkind == RELKIND_COMPOSITE_TYPE)
+ ATTypedTableRecursion(wqueue, rel, cmd, lockmode);
}
static void
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index e91044b..bbfbab2 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -2803,6 +2803,7 @@ _copyRenameStmt(RenameStmt *from)
COPY_NODE_FIELD(objarg);
COPY_STRING_FIELD(subname);
COPY_STRING_FIELD(newname);
+ COPY_SCALAR_FIELD(behavior);
return newnode;
}
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 73b28f9..be4b835 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -1306,6 +1306,7 @@ _equalRenameStmt(RenameStmt *a, RenameStmt *b)
COMPARE_NODE_FIELD(objarg);
COMPARE_STRING_FIELD(subname);
COMPARE_STRING_FIELD(newname);
+ COMPARE_SCALAR_FIELD(behavior);
return true;
}
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 1394b21..aa2e389 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -2003,41 +2003,43 @@ alter_type_cmds:
;
alter_type_cmd:
- /* ALTER TYPE <name> ADD ATTRIBUTE <coldef> */
- ADD_P ATTRIBUTE TableFuncElement
+ /* ALTER TYPE <name> ADD ATTRIBUTE <coldef> [RESTRICT|CASCADE] */
+ ADD_P ATTRIBUTE TableFuncElement opt_drop_behavior
{
AlterTableCmd *n = makeNode(AlterTableCmd);
n->subtype = AT_AddColumn;
n->def = $3;
+ n->behavior = $4;
$$ = (Node *)n;
}
- /* ALTER TYPE <name> DROP ATTRIBUTE IF EXISTS <attname> */
- | DROP ATTRIBUTE IF_P EXISTS ColId
+ /* ALTER TYPE <name> DROP ATTRIBUTE IF EXISTS <attname> [RESTRICT|CASCADE] */
+ | DROP ATTRIBUTE IF_P EXISTS ColId opt_drop_behavior
{
AlterTableCmd *n = makeNode(AlterTableCmd);
n->subtype = AT_DropColumn;
n->name = $5;
- n->behavior = DROP_RESTRICT; /* currently no effect */
+ n->behavior = $6;
n->missing_ok = TRUE;
$$ = (Node *)n;
}
- /* ALTER TYPE <name> DROP ATTRIBUTE <attname> */
+ /* ALTER TYPE <name> DROP ATTRIBUTE <attname> [RESTRICT|CASCADE] */
| DROP ATTRIBUTE ColId opt_drop_behavior
{
AlterTableCmd *n = makeNode(AlterTableCmd);
n->subtype = AT_DropColumn;
n->name = $3;
- n->behavior = DROP_RESTRICT; /* currently no effect */
+ n->behavior = $4;
n->missing_ok = FALSE;
$$ = (Node *)n;
}
- /* ALTER TYPE <name> ALTER ATTRIBUTE <attname> [SET DATA] TYPE <typename> */
- | ALTER ATTRIBUTE ColId opt_set_data TYPE_P Typename
+ /* ALTER TYPE <name> ALTER ATTRIBUTE <attname> [SET DATA] TYPE <typename> [RESTRICT|CASCADE] */
+ | ALTER ATTRIBUTE ColId opt_set_data TYPE_P Typename opt_drop_behavior
{
AlterTableCmd *n = makeNode(AlterTableCmd);
n->subtype = AT_AlterColumnType;
n->name = $3;
n->def = (Node *) $6;
+ n->behavior = $7;
$$ = (Node *)n;
}
;
@@ -6005,13 +6007,14 @@ RenameStmt: ALTER AGGREGATE func_name aggr_args RENAME TO name
n->newname = $6;
$$ = (Node *)n;
}
- | ALTER TYPE_P any_name RENAME ATTRIBUTE name TO name
+ | ALTER TYPE_P any_name RENAME ATTRIBUTE name TO name opt_drop_behavior
{
RenameStmt *n = makeNode(RenameStmt);
n->renameType = OBJECT_ATTRIBUTE;
n->relation = makeRangeVarFromAnyName($3, @3, yyscanner);
n->subname = $6;
n->newname = $8;
+ n->behavior = $9;
$$ = (Node *)n;
}
;
diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h
index aed3960..ad932d3 100644
--- a/src/include/commands/tablecmds.h
+++ b/src/include/commands/tablecmds.h
@@ -42,11 +42,7 @@ extern void CheckTableNotInUse(Relation rel, const char *stmt);
extern void ExecuteTruncate(TruncateStmt *stmt);
-extern void renameatt(Oid myrelid,
- const char *oldattname,
- const char *newattname,
- bool recurse,
- int expected_parents);
+extern void renameatt(Oid myrelid, RenameStmt *stmt);
extern void RenameRelation(Oid myrelid,
const char *newrelname,
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index a320be4..d6cfafe 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2073,6 +2073,7 @@ typedef struct RenameStmt
char *subname; /* name of contained object (column, rule,
* trigger, etc) */
char *newname; /* the new name */
+ DropBehavior behavior; /* RESTRICT or CASCADE behavior */
} RenameStmt;
/* ----------------------
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index ab19a8e..6809ed6 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -1760,13 +1760,100 @@ ALTER TYPE test_type1 ALTER ATTRIBUTE b TYPE varchar; -- fails
ERROR: cannot alter type "test_type1" because column "test_tbl1"."y" uses it
CREATE TYPE test_type2 AS (a int, b text);
CREATE TABLE test_tbl2 OF test_type2;
+\d test_type2
+Composite type "public.test_type2"
+ Column | Type
+--------+---------
+ a | integer
+ b | text
+
+\d test_tbl2
+ Table "public.test_tbl2"
+ Column | Type | Modifiers
+--------+---------+-----------
+ a | integer |
+ b | text |
+Typed table of type: test_type2
+
ALTER TYPE test_type2 ADD ATTRIBUTE c text; -- fails
ERROR: cannot alter type "test_type2" because it is the type of a typed table
+HINT: Use ALTER ... CASCADE to alter the typed tables too.
+ALTER TYPE test_type2 ADD ATTRIBUTE c text CASCADE;
+\d test_type2
+Composite type "public.test_type2"
+ Column | Type
+--------+---------
+ a | integer
+ b | text
+ c | text
+
+\d test_tbl2
+ Table "public.test_tbl2"
+ Column | Type | Modifiers
+--------+---------+-----------
+ a | integer |
+ b | text |
+ c | text |
+Typed table of type: test_type2
+
ALTER TYPE test_type2 ALTER ATTRIBUTE b TYPE varchar; -- fails
ERROR: cannot alter type "test_type2" because it is the type of a typed table
+HINT: Use ALTER ... CASCADE to alter the typed tables too.
+ALTER TYPE test_type2 ALTER ATTRIBUTE b TYPE varchar CASCADE;
+\d test_type2
+Composite type "public.test_type2"
+ Column | Type
+--------+-------------------
+ a | integer
+ b | character varying
+ c | text
+
+\d test_tbl2
+ Table "public.test_tbl2"
+ Column | Type | Modifiers
+--------+-------------------+-----------
+ a | integer |
+ b | character varying |
+ c | text |
+Typed table of type: test_type2
+
ALTER TYPE test_type2 DROP ATTRIBUTE b; -- fails
ERROR: cannot alter type "test_type2" because it is the type of a typed table
-ALTER TYPE test_type2 RENAME ATTRIBUTE b TO bb; -- fails
+HINT: Use ALTER ... CASCADE to alter the typed tables too.
+ALTER TYPE test_type2 DROP ATTRIBUTE b CASCADE;
+\d test_type2
+Composite type "public.test_type2"
+ Column | Type
+--------+---------
+ a | integer
+ c | text
+
+\d test_tbl2
+ Table "public.test_tbl2"
+ Column | Type | Modifiers
+--------+---------+-----------
+ a | integer |
+ c | text |
+Typed table of type: test_type2
+
+ALTER TYPE test_type2 RENAME ATTRIBUTE a TO aa; -- fails
ERROR: cannot alter type "test_type2" because it is the type of a typed table
+HINT: Use ALTER ... CASCADE to alter the typed tables too.
+ALTER TYPE test_type2 RENAME ATTRIBUTE a TO aa CASCADE;
+\d test_type2
+Composite type "public.test_type2"
+ Column | Type
+--------+---------
+ aa | integer
+ c | text
+
+\d test_tbl2
+ Table "public.test_tbl2"
+ Column | Type | Modifiers
+--------+---------+-----------
+ aa | integer |
+ c | text |
+Typed table of type: test_type2
+
CREATE TYPE test_type_empty AS ();
DROP TYPE test_type_empty;
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 3e1646a..56b61b2 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -1272,10 +1272,28 @@ ALTER TYPE test_type1 ALTER ATTRIBUTE b TYPE varchar; -- fails
CREATE TYPE test_type2 AS (a int, b text);
CREATE TABLE test_tbl2 OF test_type2;
+\d test_type2
+\d test_tbl2
+
ALTER TYPE test_type2 ADD ATTRIBUTE c text; -- fails
+ALTER TYPE test_type2 ADD ATTRIBUTE c text CASCADE;
+\d test_type2
+\d test_tbl2
+
ALTER TYPE test_type2 ALTER ATTRIBUTE b TYPE varchar; -- fails
+ALTER TYPE test_type2 ALTER ATTRIBUTE b TYPE varchar CASCADE;
+\d test_type2
+\d test_tbl2
+
ALTER TYPE test_type2 DROP ATTRIBUTE b; -- fails
-ALTER TYPE test_type2 RENAME ATTRIBUTE b TO bb; -- fails
+ALTER TYPE test_type2 DROP ATTRIBUTE b CASCADE;
+\d test_type2
+\d test_tbl2
+
+ALTER TYPE test_type2 RENAME ATTRIBUTE a TO aa; -- fails
+ALTER TYPE test_type2 RENAME ATTRIBUTE a TO aa CASCADE;
+\d test_type2
+\d test_tbl2
CREATE TYPE test_type_empty AS ();
DROP TYPE test_type_empty;
Peter Eisentraut <peter_e@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
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.