On ons, 2010-07-21 at 15:48 -0400, Alvaro Herrera wrote:
> Excerpts from Peter Eisentraut's message of mié jul 21 15:18:58 -0400 2010:
> > After some investigation I figured that I need to add two more checks
> > into the ALTER TABLE code to prevent certain types of direct changes to
> > typed tables (see attached patch).
> >
> > But it's not clear to me whether such checks should go into the "Prep"
> > or the "Exec" phases. Prep seems more plausible to me, but some
> > commands such as DropColumn don't have a Prep handler. A clarification
> > would be helpful.
>
> I think if there's no Prep phase, you should add it. I don't think it
> makes sense to have this kind of check in Exec.
OK, here is my patch for this. (should go into 9.0 and 9.1)
Index: src/backend/commands/tablecmds.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/tablecmds.c,v
retrieving revision 1.332
diff -u -3 -p -r1.332 tablecmds.c
--- src/backend/commands/tablecmds.c 6 Jul 2010 19:18:56 -0000 1.332
+++ src/backend/commands/tablecmds.c 22 Jul 2010 14:53:24 -0000
@@ -288,6 +288,7 @@ static void ATExecSetOptions(Relation re
Node *options, bool isReset);
static void ATExecSetStorage(Relation rel, const char *colName,
Node *newValue);
+static void ATPrepDropColumn(Relation rel, bool recurse, AlterTableCmd *cmd);
static void ATExecDropColumn(List **wqueue, Relation rel, const char *colName,
DropBehavior behavior,
bool recurse, bool recursing,
@@ -327,7 +328,8 @@ static void ATExecEnableDisableTrigger(R
char fires_when, bool skip_system);
static void ATExecEnableDisableRule(Relation rel, char *rulename,
char fires_when);
-static void ATExecAddInherit(Relation rel, RangeVar *parent);
+static void ATPrepAddInherit(Relation child_rel);
+static void ATExecAddInherit(Relation child_rel, RangeVar *parent);
static void ATExecDropInherit(Relation rel, RangeVar *parent);
static void copy_relation_data(SMgrRelation rel, SMgrRelation dst,
ForkNumber forkNum, bool istemp);
@@ -2499,10 +2501,8 @@ ATPrepCmd(List **wqueue, Relation rel, A
break;
case AT_DropColumn: /* DROP COLUMN */
ATSimplePermissions(rel, false);
+ ATPrepDropColumn(rel, recurse, cmd);
/* Recursion occurs during execution phase */
- /* No command-specific prep needed except saving recurse flag */
- if (recurse)
- cmd->subtype = AT_DropColumnRecurse;
pass = AT_PASS_DROP;
break;
case AT_AddIndex: /* ADD INDEX */
@@ -2579,6 +2579,12 @@ ATPrepCmd(List **wqueue, Relation rel, A
/* No command-specific prep needed */
pass = AT_PASS_MISC;
break;
+ case AT_AddInherit: /* INHERIT */
+ ATSimplePermissions(rel, false);
+ /* This command never recurses */
+ ATPrepAddInherit(rel);
+ pass = AT_PASS_MISC;
+ break;
case AT_EnableTrig: /* ENABLE TRIGGER variants */
case AT_EnableAlwaysTrig:
case AT_EnableReplicaTrig:
@@ -2591,8 +2597,7 @@ ATPrepCmd(List **wqueue, Relation rel, A
case AT_EnableAlwaysRule:
case AT_EnableReplicaRule:
case AT_DisableRule:
- case AT_AddInherit: /* INHERIT / NO INHERIT */
- case AT_DropInherit:
+ case AT_DropInherit: /* NO INHERIT */
ATSimplePermissions(rel, false);
/* These commands never recurse */
/* No command-specific prep needed */
@@ -3568,6 +3573,11 @@ static void
ATPrepAddColumn(List **wqueue, Relation rel, bool recurse,
AlterTableCmd *cmd)
{
+ if (rel->rd_rel->reloftype)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot add column to typed table")));
+
/*
* Recurse to add the column to child classes, if requested.
*
@@ -3616,11 +3626,6 @@ ATExecAddColumn(AlteredTableInfo *tab, R
Form_pg_type tform;
Expr *defval;
- if (rel->rd_rel->reloftype)
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("cannot add column to typed table")));
-
attrdesc = heap_open(AttributeRelationId, RowExclusiveLock);
/*
@@ -4326,6 +4331,19 @@ ATExecSetStorage(Relation rel, const cha
* correctly.)
*/
static void
+ATPrepDropColumn(Relation rel, bool recurse, AlterTableCmd *cmd)
+{
+ if (rel->rd_rel->reloftype)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot drop column from typed table")));
+
+ /* No command-specific prep needed except saving recurse flag */
+ if (recurse)
+ cmd->subtype = AT_DropColumnRecurse;
+}
+
+static void
ATExecDropColumn(List **wqueue, Relation rel, const char *colName,
DropBehavior behavior,
bool recurse, bool recursing,
@@ -4337,11 +4355,6 @@ ATExecDropColumn(List **wqueue, Relation
List *children;
ObjectAddress object;
- if (rel->rd_rel->reloftype)
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("cannot drop column from typed table")));
-
/* At top level, permission check was done in ATPrepCmd, else do it */
if (recursing)
ATSimplePermissions(rel, false);
@@ -5788,6 +5801,11 @@ ATPrepAlterColumnType(List **wqueue,
NewColumnValue *newval;
ParseState *pstate = make_parsestate(NULL);
+ if (rel->rd_rel->reloftype)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot alter column type of typed table")));
+
/* lookup the attribute so we can check inheritance status */
tuple = SearchSysCacheAttName(RelationGetRelid(rel), colName);
if (!HeapTupleIsValid(tuple))
@@ -7116,6 +7134,15 @@ ATExecEnableDisableRule(Relation rel, ch
* same data types and expressions.
*/
static void
+ATPrepAddInherit(Relation child_rel)
+{
+ if (child_rel->rd_rel->reloftype)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot change inheritance of typed table")));
+}
+
+static void
ATExecAddInherit(Relation child_rel, RangeVar *parent)
{
Relation parent_rel,
Index: src/test/regress/expected/typed_table.out
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/expected/typed_table.out,v
retrieving revision 1.1
diff -u -3 -p -r1.1 typed_table.out
--- src/test/regress/expected/typed_table.out 28 Jan 2010 23:21:13 -0000 1.1
+++ src/test/regress/expected/typed_table.out 22 Jul 2010 14:53:26 -0000
@@ -25,12 +25,18 @@ SELECT * FROM get_all_persons();
----+------
(0 rows)
+-- certain ALTER TABLE operations on typed tables are not allowed
ALTER TABLE persons ADD COLUMN comment text;
ERROR: cannot add column to typed table
ALTER TABLE persons DROP COLUMN name;
ERROR: cannot drop column from typed table
ALTER TABLE persons RENAME COLUMN id TO num;
ERROR: cannot rename column of typed table
+ALTER TABLE persons ALTER COLUMN name TYPE varchar;
+ERROR: cannot alter column type of typed table
+CREATE TABLE stuff (id int);
+ALTER TABLE persons INHERIT stuff;
+ERROR: cannot change inheritance of typed table
CREATE TABLE personsx OF person_type (myname WITH OPTIONS NOT NULL); -- error
ERROR: column "myname" does not exist
CREATE TABLE persons2 OF person_type (
@@ -83,3 +89,4 @@ DETAIL: drop cascades to table persons
drop cascades to function get_all_persons()
drop cascades to table persons2
drop cascades to table persons3
+DROP TABLE stuff;
Index: src/test/regress/sql/typed_table.sql
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/sql/typed_table.sql,v
retrieving revision 1.1
diff -u -3 -p -r1.1 typed_table.sql
--- src/test/regress/sql/typed_table.sql 28 Jan 2010 23:21:13 -0000 1.1
+++ src/test/regress/sql/typed_table.sql 22 Jul 2010 14:53:26 -0000
@@ -13,9 +13,13 @@ $$;
SELECT * FROM get_all_persons();
+-- certain ALTER TABLE operations on typed tables are not allowed
ALTER TABLE persons ADD COLUMN comment text;
ALTER TABLE persons DROP COLUMN name;
ALTER TABLE persons RENAME COLUMN id TO num;
+ALTER TABLE persons ALTER COLUMN name TYPE varchar;
+CREATE TABLE stuff (id int);
+ALTER TABLE persons INHERIT stuff;
CREATE TABLE personsx OF person_type (myname WITH OPTIONS NOT NULL); -- error
@@ -40,3 +44,5 @@ CREATE TABLE persons4 OF person_type (
DROP TYPE person_type RESTRICT;
DROP TYPE person_type CASCADE;
+
+DROP TABLE stuff;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
