Hi Tom, hi all,
On 05/06/2009 11:43 PM, Tom Lane wrote:
Andres Freund<and...@anarazel.de> writes:
As this is my first patch to PG I am happy with most sort of feedback.
Please add your patch to the commit-fest queue here:
http://wiki.postgresql.org/wiki/CommitFestInProgress
Will do so, after this email has arrived.
Since we are still busy with 8.4 beta, it's unlikely that anyone will
take a close look until the next commit fest begins. FWIW, I took a
very fast look through the patch and thought it was at least touching
the right places, except I think you missed equalfuncs.c. (It'd be a
good idea to grep for all uses of AlterTableCmd struct to see if you
missed anything else.) I don't have time now to look closer or do any
testing.
Ok, fixed that place (stupid me, I had seen that it is used there and
forgot about it) and found no other places.
Thanks,
Andres
>From adca6b0f0409c3ea95c5e93ab5e1d8f3f3edf802 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Thu, 7 May 2009 00:53:45 +0200
Subject: [PATCH] Feature: DROP (COLUMN|CONSTRAINT) IF EXISTS
---
doc/src/sgml/ref/alter_table.sgml | 14 ++++--
src/backend/commands/tablecmds.c | 74 ++++++++++++++++++++---------
src/backend/nodes/copyfuncs.c | 1 +
src/backend/nodes/equalfuncs.c | 1 +
src/backend/parser/gram.y | 22 +++++++++
src/include/nodes/parsenodes.h | 1 +
src/test/regress/expected/alter_table.out | 10 ++++
src/test/regress/sql/alter_table.sql | 9 ++++
8 files changed, 106 insertions(+), 26 deletions(-)
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index fe3f388..9678236 100644
*** a/doc/src/sgml/ref/alter_table.sgml
--- b/doc/src/sgml/ref/alter_table.sgml
*************** ALTER TABLE <replaceable class="PARAMETE
*** 33,39 ****
where <replaceable class="PARAMETER">action</replaceable> is one of:
ADD [ COLUMN ] <replaceable class="PARAMETER">column</replaceable> <replaceable class="PARAMETER">type</replaceable> [ <replaceable class="PARAMETER">column_constraint</replaceable> [ ... ] ]
! DROP [ COLUMN ] <replaceable class="PARAMETER">column</replaceable> [ RESTRICT | CASCADE ]
ALTER [ COLUMN ] <replaceable class="PARAMETER">column</replaceable> [ SET DATA ] TYPE <replaceable class="PARAMETER">type</replaceable> [ USING <replaceable class="PARAMETER">expression</replaceable> ]
ALTER [ COLUMN ] <replaceable class="PARAMETER">column</replaceable> SET DEFAULT <replaceable class="PARAMETER">expression</replaceable>
ALTER [ COLUMN ] <replaceable class="PARAMETER">column</replaceable> DROP DEFAULT
--- 33,39 ----
where <replaceable class="PARAMETER">action</replaceable> is one of:
ADD [ COLUMN ] <replaceable class="PARAMETER">column</replaceable> <replaceable class="PARAMETER">type</replaceable> [ <replaceable class="PARAMETER">column_constraint</replaceable> [ ... ] ]
! DROP [ COLUMN ] [ IF EXISTS ] <replaceable class="PARAMETER">column</replaceable> [ RESTRICT | CASCADE ]
ALTER [ COLUMN ] <replaceable class="PARAMETER">column</replaceable> [ SET DATA ] TYPE <replaceable class="PARAMETER">type</replaceable> [ USING <replaceable class="PARAMETER">expression</replaceable> ]
ALTER [ COLUMN ] <replaceable class="PARAMETER">column</replaceable> SET DEFAULT <replaceable class="PARAMETER">expression</replaceable>
ALTER [ COLUMN ] <replaceable class="PARAMETER">column</replaceable> DROP DEFAULT
*************** where <replaceable class="PARAMETER">act
*** 41,47 ****
ALTER [ COLUMN ] <replaceable class="PARAMETER">column</replaceable> SET STATISTICS <replaceable class="PARAMETER">integer</replaceable>
ALTER [ COLUMN ] <replaceable class="PARAMETER">column</replaceable> SET STORAGE { PLAIN | EXTERNAL | EXTENDED | MAIN }
ADD <replaceable class="PARAMETER">table_constraint</replaceable>
! DROP CONSTRAINT <replaceable class="PARAMETER">constraint_name</replaceable> [ RESTRICT | CASCADE ]
DISABLE TRIGGER [ <replaceable class="PARAMETER">trigger_name</replaceable> | ALL | USER ]
ENABLE TRIGGER [ <replaceable class="PARAMETER">trigger_name</replaceable> | ALL | USER ]
ENABLE REPLICA TRIGGER <replaceable class="PARAMETER">trigger_name</replaceable>
--- 41,47 ----
ALTER [ COLUMN ] <replaceable class="PARAMETER">column</replaceable> SET STATISTICS <replaceable class="PARAMETER">integer</replaceable>
ALTER [ COLUMN ] <replaceable class="PARAMETER">column</replaceable> SET STORAGE { PLAIN | EXTERNAL | EXTENDED | MAIN }
ADD <replaceable class="PARAMETER">table_constraint</replaceable>
! DROP CONSTRAINT [ IF EXISTS ] <replaceable class="PARAMETER">constraint_name</replaceable> [ RESTRICT | CASCADE ]
DISABLE TRIGGER [ <replaceable class="PARAMETER">trigger_name</replaceable> | ALL | USER ]
ENABLE TRIGGER [ <replaceable class="PARAMETER">trigger_name</replaceable> | ALL | USER ]
ENABLE REPLICA TRIGGER <replaceable class="PARAMETER">trigger_name</replaceable>
*************** where <replaceable class="PARAMETER">act
*** 82,88 ****
</varlistentry>
<varlistentry>
! <term><literal>DROP COLUMN</literal></term>
<listitem>
<para>
This form drops a column from a table. Indexes and
--- 82,88 ----
</varlistentry>
<varlistentry>
! <term><literal>DROP COLUMN [ IF EXISTS ]</literal></term>
<listitem>
<para>
This form drops a column from a table. Indexes and
*************** where <replaceable class="PARAMETER">act
*** 90,95 ****
--- 90,98 ----
dropped as well. You will need to say <literal>CASCADE</> if
anything outside the table depends on the column, for example,
foreign key references or views.
+ If <literal>IF EXISTS</literal> is specified, no error is thrown
+ if the specified column does not exist. A notice is issued in
+ this case.
</para>
</listitem>
</varlistentry>
*************** where <replaceable class="PARAMETER">act
*** 192,201 ****
</varlistentry>
<varlistentry>
! <term><literal>DROP CONSTRAINT</literal></term>
<listitem>
<para>
This form drops the specified constraint on a table.
</para>
</listitem>
</varlistentry>
--- 195,207 ----
</varlistentry>
<varlistentry>
! <term><literal>DROP CONSTRAINT [ IF EXISTS ]</literal></term>
<listitem>
<para>
This form drops the specified constraint on a table.
+ If <literal>IF EXISTS</literal> is specified, no error is thrown
+ if the specified constraint does not exist. A notice is issued in
+ this case.
</para>
</listitem>
</varlistentry>
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 0ba4c2c..2cd61eb 100644
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
*************** static void ATExecSetStorage(Relation re
*** 287,293 ****
Node *newValue);
static void ATExecDropColumn(List **wqueue, Relation rel, const char *colName,
DropBehavior behavior,
! bool recurse, bool recursing);
static void ATExecAddIndex(AlteredTableInfo *tab, Relation rel,
IndexStmt *stmt, bool is_rebuild);
static void ATExecAddConstraint(List **wqueue,
--- 287,294 ----
Node *newValue);
static void ATExecDropColumn(List **wqueue, Relation rel, const char *colName,
DropBehavior behavior,
! bool recurse, bool recursing,
! bool missing_ok);
static void ATExecAddIndex(AlteredTableInfo *tab, Relation rel,
IndexStmt *stmt, bool is_rebuild);
static void ATExecAddConstraint(List **wqueue,
*************** static void ATAddCheckConstraint(List **
*** 300,307 ****
static void ATAddForeignKeyConstraint(AlteredTableInfo *tab, Relation rel,
FkConstraint *fkconstraint);
static void ATExecDropConstraint(Relation rel, const char *constrName,
! DropBehavior behavior,
! bool recurse, bool recursing);
static void ATPrepAlterColumnType(List **wqueue,
AlteredTableInfo *tab, Relation rel,
bool recurse, bool recursing,
--- 301,309 ----
static void ATAddForeignKeyConstraint(AlteredTableInfo *tab, Relation rel,
FkConstraint *fkconstraint);
static void ATExecDropConstraint(Relation rel, const char *constrName,
! DropBehavior behavior,
! bool recurse, bool recursing,
! bool missing_ok);
static void ATPrepAlterColumnType(List **wqueue,
AlteredTableInfo *tab, Relation rel,
bool recurse, bool recursing,
*************** ATExecCmd(List **wqueue, AlteredTableInf
*** 2618,2628 ****
break;
case AT_DropColumn: /* DROP COLUMN */
ATExecDropColumn(wqueue, rel, cmd->name,
! cmd->behavior, false, false);
break;
case AT_DropColumnRecurse: /* DROP COLUMN with recursion */
ATExecDropColumn(wqueue, rel, cmd->name,
! cmd->behavior, true, false);
break;
case AT_AddIndex: /* ADD INDEX */
ATExecAddIndex(tab, rel, (IndexStmt *) cmd->def, false);
--- 2620,2630 ----
break;
case AT_DropColumn: /* DROP COLUMN */
ATExecDropColumn(wqueue, rel, cmd->name,
! cmd->behavior, false, false, cmd->missing_ok);
break;
case AT_DropColumnRecurse: /* DROP COLUMN with recursion */
ATExecDropColumn(wqueue, rel, cmd->name,
! cmd->behavior, true, false, cmd->missing_ok);
break;
case AT_AddIndex: /* ADD INDEX */
ATExecAddIndex(tab, rel, (IndexStmt *) cmd->def, false);
*************** ATExecCmd(List **wqueue, AlteredTableInf
*** 2637,2646 ****
ATExecAddConstraint(wqueue, tab, rel, cmd->def, true);
break;
case AT_DropConstraint: /* DROP CONSTRAINT */
! ATExecDropConstraint(rel, cmd->name, cmd->behavior, false, false);
break;
case AT_DropConstraintRecurse: /* DROP CONSTRAINT with recursion */
! ATExecDropConstraint(rel, cmd->name, cmd->behavior, true, false);
break;
case AT_AlterColumnType: /* ALTER COLUMN TYPE */
ATExecAlterColumnType(tab, rel, cmd->name, (TypeName *) cmd->def);
--- 2639,2652 ----
ATExecAddConstraint(wqueue, tab, rel, cmd->def, true);
break;
case AT_DropConstraint: /* DROP CONSTRAINT */
! ATExecDropConstraint(rel, cmd->name, cmd->behavior,
! false, false,
! cmd->missing_ok);
break;
case AT_DropConstraintRecurse: /* DROP CONSTRAINT with recursion */
! ATExecDropConstraint(rel, cmd->name, cmd->behavior,
! true, false,
! cmd->missing_ok);
break;
case AT_AlterColumnType: /* ALTER COLUMN TYPE */
ATExecAlterColumnType(tab, rel, cmd->name, (TypeName *) cmd->def);
*************** ATExecSetStorage(Relation rel, const cha
*** 4158,4164 ****
static void
ATExecDropColumn(List **wqueue, Relation rel, const char *colName,
DropBehavior behavior,
! bool recurse, bool recursing)
{
HeapTuple tuple;
Form_pg_attribute targetatt;
--- 4164,4171 ----
static void
ATExecDropColumn(List **wqueue, Relation rel, const char *colName,
DropBehavior behavior,
! bool recurse, bool recursing,
! bool missing_ok)
{
HeapTuple tuple;
Form_pg_attribute targetatt;
*************** ATExecDropColumn(List **wqueue, Relation
*** 4174,4184 ****
* get the number of the attribute
*/
tuple = SearchSysCacheAttName(RelationGetRelid(rel), colName);
! if (!HeapTupleIsValid(tuple))
! ereport(ERROR,
! (errcode(ERRCODE_UNDEFINED_COLUMN),
! errmsg("column \"%s\" of relation \"%s\" does not exist",
! colName, RelationGetRelationName(rel))));
targetatt = (Form_pg_attribute) GETSTRUCT(tuple);
attnum = targetatt->attnum;
--- 4181,4201 ----
* get the number of the attribute
*/
tuple = SearchSysCacheAttName(RelationGetRelid(rel), colName);
! if (!HeapTupleIsValid(tuple)){
! if (!missing_ok){
! ereport(ERROR,
! (errcode(ERRCODE_UNDEFINED_COLUMN),
! errmsg("column \"%s\" of relation \"%s\" does not exist",
! colName, RelationGetRelationName(rel))));
! }
! else
! {
! ereport(NOTICE,
! (errmsg("column \"%s\" of relation \"%s\" does not exist, skipping",
! colName, RelationGetRelationName(rel))));
! return;
! }
! }
targetatt = (Form_pg_attribute) GETSTRUCT(tuple);
attnum = targetatt->attnum;
*************** ATExecDropColumn(List **wqueue, Relation
*** 4242,4248 ****
{
/* Time to delete this child column, too */
ATExecDropColumn(wqueue, childrel, colName,
! behavior, true, true);
}
else
{
--- 4259,4266 ----
{
/* Time to delete this child column, too */
ATExecDropColumn(wqueue, childrel, colName,
! behavior, true, true,
! false);
}
else
{
*************** createForeignKeyTriggers(Relation rel, F
*** 5357,5363 ****
static void
ATExecDropConstraint(Relation rel, const char *constrName,
DropBehavior behavior,
! bool recurse, bool recursing)
{
List *children;
ListCell *child;
--- 5375,5382 ----
static void
ATExecDropConstraint(Relation rel, const char *constrName,
DropBehavior behavior,
! bool recurse, bool recursing,
! bool missing_ok)
{
List *children;
ListCell *child;
*************** ATExecDropConstraint(Relation rel, const
*** 5419,5430 ****
systable_endscan(scan);
! if (!found)
! ereport(ERROR,
! (errcode(ERRCODE_UNDEFINED_OBJECT),
! errmsg("constraint \"%s\" of relation \"%s\" does not exist",
! constrName, RelationGetRelationName(rel))));
!
/*
* Propagate to children as appropriate. Unlike most other ALTER
* routines, we have to do this one level of recursion at a time; we can't
--- 5438,5459 ----
systable_endscan(scan);
! if (!found){
! if (!missing_ok){
! ereport(ERROR,
! (errcode(ERRCODE_UNDEFINED_OBJECT),
! errmsg("constraint \"%s\" of relation \"%s\" does not exist",
! constrName, RelationGetRelationName(rel))));
! }
! else
! {
! ereport(NOTICE,
! (errmsg("constraint \"%s\" of relation \"%s\" does not exist, skipping",
! constrName, RelationGetRelationName(rel))));
! heap_close(conrel, RowExclusiveLock);
! return;
! }
! }
/*
* Propagate to children as appropriate. Unlike most other ALTER
* routines, we have to do this one level of recursion at a time; we can't
*************** ATExecDropConstraint(Relation rel, const
*** 5485,5491 ****
{
/* Time to delete this child constraint, too */
ATExecDropConstraint(childrel, constrName, behavior,
! true, true);
}
else
{
--- 5514,5521 ----
{
/* Time to delete this child constraint, too */
ATExecDropConstraint(childrel, constrName, behavior,
! true, true,
! false);
}
else
{
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 831f7e4..20795f5 100644
*** a/src/backend/nodes/copyfuncs.c
--- b/src/backend/nodes/copyfuncs.c
*************** _copyAlterTableCmd(AlterTableCmd *from)
*** 2272,2277 ****
--- 2272,2278 ----
COPY_NODE_FIELD(def);
COPY_NODE_FIELD(transform);
COPY_SCALAR_FIELD(behavior);
+ COPY_SCALAR_FIELD(missing_ok);
return newnode;
}
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index e4965d6..e9a8d65 100644
*** a/src/backend/nodes/equalfuncs.c
--- b/src/backend/nodes/equalfuncs.c
*************** _equalAlterTableCmd(AlterTableCmd *a, Al
*** 958,963 ****
--- 958,964 ----
COMPARE_NODE_FIELD(def);
COMPARE_NODE_FIELD(transform);
COMPARE_SCALAR_FIELD(behavior);
+ COMPARE_SCALAR_FIELD(missing_ok);
return true;
}
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index b5ec7cb..ab25fa8 100644
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
*************** alter_table_cmd:
*** 1593,1598 ****
--- 1593,1608 ----
n->def = (Node *) makeString($6);
$$ = (Node *)n;
}
+ /* ALTER TABLE <name> DROP [COLUMN] IF EXISTS <colname> [RESTRICT|CASCADE] */
+ | DROP opt_column IF_P EXISTS ColId opt_drop_behavior
+ {
+ AlterTableCmd *n = makeNode(AlterTableCmd);
+ n->subtype = AT_DropColumn;
+ n->name = $5;
+ n->behavior = $6;
+ n->missing_ok = TRUE;
+ $$ = (Node *)n;
+ }
/* ALTER TABLE <name> DROP [COLUMN] <colname> [RESTRICT|CASCADE] */
| DROP opt_column ColId opt_drop_behavior
{
*************** alter_table_cmd:
*** 1600,1605 ****
--- 1610,1616 ----
n->subtype = AT_DropColumn;
n->name = $3;
n->behavior = $4;
+ n->missing_ok = FALSE;
$$ = (Node *)n;
}
/*
*************** alter_table_cmd:
*** 1623,1628 ****
--- 1634,1649 ----
n->def = $2;
$$ = (Node *)n;
}
+ /* ALTER TABLE <name> DROP CONSTRAINT IF EXISTS <name> [RESTRICT|CASCADE] */
+ | DROP CONSTRAINT IF_P EXISTS name opt_drop_behavior
+ {
+ AlterTableCmd *n = makeNode(AlterTableCmd);
+ n->subtype = AT_DropConstraint;
+ n->name = $5;
+ n->behavior = $6;
+ n->missing_ok = TRUE;
+ $$ = (Node *)n;
+ }
/* ALTER TABLE <name> DROP CONSTRAINT <name> [RESTRICT|CASCADE] */
| DROP CONSTRAINT name opt_drop_behavior
{
*************** alter_table_cmd:
*** 1630,1635 ****
--- 1651,1657 ----
n->subtype = AT_DropConstraint;
n->name = $3;
n->behavior = $4;
+ n->missing_ok = FALSE;
$$ = (Node *)n;
}
/* ALTER TABLE <name> SET WITH OIDS */
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 6d3265f..ad22cd8 100644
*** a/src/include/nodes/parsenodes.h
--- b/src/include/nodes/parsenodes.h
*************** typedef struct AlterTableCmd /* one subc
*** 1144,1149 ****
--- 1144,1150 ----
* index, constraint, or parent table */
Node *transform; /* transformation expr for ALTER TYPE */
DropBehavior behavior; /* RESTRICT or CASCADE for DROP cases */
+ bool missing_ok; /* skip error if missing? */
} AlterTableCmd;
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index b14d61f..031e59c 100644
*** a/src/test/regress/expected/alter_table.out
--- b/src/test/regress/expected/alter_table.out
*************** alter table gc1 drop column name;
*** 1150,1155 ****
--- 1150,1161 ----
ERROR: column "name" of relation "gc1" does not exist
-- should work and drop the attribute in all tables
alter table p2 drop column height;
+ -- IF EXISTS test
+ create table dropColumnExists ();
+ alter table dropColumnExists drop column non_existing; --fail
+ ERROR: column "non_existing" of relation "dropcolumnexists" does not exist
+ alter table dropColumnExists drop column if exists non_existing; --succeed
+ NOTICE: column "non_existing" of relation "dropcolumnexists" does not exist, skipping
select relname, attname, attinhcount, attislocal
from pg_class join pg_attribute on (pg_class.oid = pg_attribute.attrelid)
where relname in ('p1','p2','c1','gc1') and attnum > 0 and not attisdropped
*************** alter table anothertab alter column atco
*** 1421,1426 ****
--- 1427,1436 ----
ERROR: operator does not exist: boolean <= integer
HINT: No operator matches the given name and argument type(s). You might need to add explicit type casts.
alter table anothertab drop constraint anothertab_chk;
+ alter table anothertab drop constraint anothertab_chk; -- fails
+ ERROR: constraint "anothertab_chk" of relation "anothertab" does not exist
+ alter table anothertab drop constraint IF EXISTS anothertab_chk; -- succeeds
+ NOTICE: constraint "anothertab_chk" of relation "anothertab" does not exist, skipping
alter table anothertab alter column atcol1 type boolean
using case when atcol1 % 2 = 0 then true else false end;
select * from anothertab;
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index e041cec..82c2e4e 100644
*** a/src/test/regress/sql/alter_table.sql
--- b/src/test/regress/sql/alter_table.sql
*************** create table dropColumnAnother (d int) i
*** 816,821 ****
--- 816,823 ----
alter table dropColumnchild drop column a;
alter table only dropColumnChild drop column b;
+
+
-- these three should work
alter table only dropColumn drop column e;
alter table dropColumnChild drop column c;
*************** alter table gc1 drop column name;
*** 913,918 ****
--- 915,925 ----
-- should work and drop the attribute in all tables
alter table p2 drop column height;
+ -- IF EXISTS test
+ create table dropColumnExists ();
+ alter table dropColumnExists drop column non_existing; --fail
+ alter table dropColumnExists drop column if exists non_existing; --succeed
+
select relname, attname, attinhcount, attislocal
from pg_class join pg_attribute on (pg_class.oid = pg_attribute.attrelid)
where relname in ('p1','p2','c1','gc1') and attnum > 0 and not attisdropped
*************** alter table anothertab alter column atco
*** 1057,1062 ****
--- 1064,1071 ----
alter table anothertab alter column atcol1 type boolean
using case when atcol1 % 2 = 0 then true else false end; -- fails
alter table anothertab drop constraint anothertab_chk;
+ alter table anothertab drop constraint anothertab_chk; -- fails
+ alter table anothertab drop constraint IF EXISTS anothertab_chk; -- succeeds
alter table anothertab alter column atcol1 type boolean
using case when atcol1 % 2 = 0 then true else false end;
--
1.6.3.rc4.30.g7b9ea
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
