conditional dropping of columns/constraints

Started by Andres Freundover 16 years ago15 messages
#1Andres Freund
andres@anarazel.de

Hi,

Would a patch adding 'IF EXISTS' support to:
- ALTER TABLE ... DROP COLUMN
- ALTER TABLE ... DROP CONSTRAINT
possibly be accepted?

Having it makes the annoying task of writing/testing of schema-upgrade
scripts a bit easier.

Andres

#2Andres Freund
andres@anarazel.de
In reply to: Andres Freund (#1)
Re: conditional dropping of columns/constraints

On 05/04/2009 04:10 PM, Andres Freund wrote:

Would a patch adding 'IF EXISTS' support to:
- ALTER TABLE ... DROP COLUMN
- ALTER TABLE ... DROP CONSTRAINT
possibly be accepted?

Having it makes the annoying task of writing/testing of schema-upgrade
scripts a bit easier.

Oh, and to any other sensible place given there are more?

Andres

#3Robert Haas
robertmhaas@gmail.com
In reply to: Andres Freund (#1)
Re: conditional dropping of columns/constraints

On Mon, May 4, 2009 at 10:10 AM, Andres Freund <andres@anarazel.de> wrote:

Would a patch adding 'IF EXISTS' support to:
- ALTER TABLE ... DROP COLUMN
- ALTER TABLE ... DROP CONSTRAINT
possibly be accepted?

Having it makes the annoying task of writing/testing of schema-upgrade
scripts a bit easier.

Can't speak for the committers, but I've wished for this a time or two myself.

...Robert

#4Chris Browne
cbbrowne@acm.org
In reply to: Andres Freund (#1)
Re: conditional dropping of columns/constraints

robertmhaas@gmail.com (Robert Haas) writes:

On Mon, May 4, 2009 at 10:10 AM, Andres Freund <andres@anarazel.de> wrote:

Would a patch adding 'IF EXISTS' support to:
- ALTER TABLE ... DROP COLUMN
- ALTER TABLE ... DROP CONSTRAINT
possibly be accepted?

Having it makes the annoying task of writing/testing of schema-upgrade
scripts a bit easier.

Can't speak for the committers, but I've wished for this a time or two myself.

For constraints, it's easy enough to treat that as idempotent; it's no
big deal to drop and re-add a constraint.

For columns, I'd *much* more frequently be interested in
ALTER TABLE ... ADD COLUMN IF NOT EXISTS ...

Note that this is distinctly NOT the same as:
ALTER TABLE ... DROP COLUMN IF EXISTS ...
ALTER TABLE ... ADD COLUMN ...
--
(format nil "~S@~S" "cbbrowne" "linuxdatabases.info")
http://linuxdatabases.info/info/lisp.html
Signs of a Klingon Programmer - 10. "A TRUE Klingon Warrior does not
comment his code!"

#5Peter Eisentraut
peter_e@gmx.net
In reply to: Chris Browne (#4)
Re: conditional dropping of columns/constraints

On Monday 04 May 2009 22:21:10 Chris Browne wrote:

For constraints, it's easy enough to treat that as idempotent; it's no
big deal to drop and re-add a constraint.

Not if the constraint is a primary key, for example.

#6Andrew Dunstan
andrew@dunslane.net
In reply to: Chris Browne (#4)
Re: conditional dropping of columns/constraints

Chris Browne wrote:

For columns, I'd *much* more frequently be interested in
ALTER TABLE ... ADD COLUMN IF NOT EXISTS ...

We have debated CREATE ... IF NOT EXISTS in the past, and there is no
consensus on what it should do, so we don't have it for any command.
That is quite a different case from what's being asked for, and the two
should not be conflated.

cheers

andrew

#7Andres Freund
andres@anarazel.de
In reply to: Chris Browne (#4)
Re: conditional dropping of columns/constraints

Hi Chris,

On 05/04/2009 09:21 PM, Chris Browne wrote:

robertmhaas@gmail.com (Robert Haas) writes:

On Mon, May 4, 2009 at 10:10 AM, Andres Freund<andres@anarazel.de> wrote:

Would a patch adding 'IF EXISTS' support to:
- ALTER TABLE ... DROP COLUMN
- ALTER TABLE ... DROP CONSTRAINT
possibly be accepted?

Can't speak for the committers, but I've wished for this a time or two myself.

For constraints, it's easy enough to treat that as idempotent; it's no
big deal to drop and re-add a constraint.

For columns, I'd *much* more frequently be interested in
ALTER TABLE ... ADD COLUMN IF NOT EXISTS ...

Note that this is distinctly NOT the same as:
ALTER TABLE ... DROP COLUMN IF EXISTS ...
ALTER TABLE ... ADD COLUMN ...

Yes, I would like to have that myself - but this seems to open a way
much bigger can of worms.
Also the problem solved by both suggestions are not completely congruent
- so I thought better tackle the easier one first before starting a long
and arduous discussion...

Andres

#8Robert Haas
robertmhaas@gmail.com
In reply to: Andrew Dunstan (#6)
Re: conditional dropping of columns/constraints

On Tue, May 5, 2009 at 8:56 AM, Andrew Dunstan <andrew@dunslane.net> wrote:

We have debated CREATE ... IF NOT EXISTS in the past, and there is no
consensus on what it should do, so we don't have it for any command. That is
quite a different case from what's being asked for, and the two should not
be conflated.

I must be missing something, because the semantics of CREATE ... IF
NOT EXISTS seem pretty well-defined to me, at least for any object
that has a name. Check whether that name is in use; if not, create
the object per the specified definition. Now for something like ALTER
TABLE ... ADD FOREIGN KEY I can see that there could be a problem.
That having been said, it's certain that CREATE IF NOT EXISTS is a
bigger foot-gun than DROP IF EXISTS, because after a succesful DROP IF
EXISTS the state of the object is known, whereas after CREATE IF NOT
EXISTS, it isn't (yes, it exists, but the definitions might not
match). Still, that seems no reason not to implement it. Right now,
I have a complex set of scripts which track the state of the database
to determine which DDL statements have already been applied.
Something like this would potentially simplify those scripts quite a
bit, so I'm much in favor.

On the other hand, I also agree with the point already made that these
are two different features, and we may as well focus on one at a time.

...Robert

#9Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#8)
Re: conditional dropping of columns/constraints

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, May 5, 2009 at 8:56 AM, Andrew Dunstan <andrew@dunslane.net> wrote:

We have debated CREATE ... IF NOT EXISTS in the past, and there is no
consensus on what it should do, so we don't have it for any command. That is
quite a different case from what's being asked for, and the two should not
be conflated.

I must be missing something, because the semantics of CREATE ... IF
NOT EXISTS seem pretty well-defined to me,

Please go read the prior threads (I think searching for "CINE" might
help, because we pretty shortly started abbreviating it like that).

regards, tom lane

#10Andrew Dunstan
andrew@dunslane.net
In reply to: Robert Haas (#8)
Re: conditional dropping of columns/constraints

Robert Haas wrote:

On Tue, May 5, 2009 at 8:56 AM, Andrew Dunstan <andrew@dunslane.net> wrote:

We have debated CREATE ... IF NOT EXISTS in the past, and there is no
consensus on what it should do, so we don't have it for any command. That is
quite a different case from what's being asked for, and the two should not
be conflated.

I must be missing something, because the semantics of CREATE ... IF
NOT EXISTS seem pretty well-defined to me, at least for any object
that has a name. Check whether that name is in use; if not, create
the object per the specified definition.

And if it does exist but the definitions don't match? That's the issue
on which there has not been consensus. You apparently thing the command
should silently do nothing, but that's not what everyone thinks. (I have
no very strong feelings on the subject - I'm just explaining the issue.)

cheers

andrew

#11Andres Freund
andres@anarazel.de
In reply to: Andres Freund (#1)
1 attachment(s)
Re: conditional dropping of columns/constraints

Hi,

On 05/04/2009 04:10 PM, Andres Freund wrote:

Would a patch adding 'IF EXISTS' support to:
- ALTER TABLE ... DROP COLUMN
- ALTER TABLE ... DROP CONSTRAINT
possibly be accepted?

A first version of a patch is attached:
- allows [ IF EXISTS ] for both, conditional dropping of columns and
constraints
- adds two tiny additions to the alter_table regression suite
- adds minimal documentation (my wording might be completely off)

As this is my first patch to PG I am happy with most sort of feedback.

Andres

Attachments:

drop-col-if-exists-v1.context.patchtext/x-diff; name=drop-col-if-exists-v1.context.patchDownload
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/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;
#12Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andres Freund (#11)
Re: conditional dropping of columns/constraints

Andres Freund <andres@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

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.

regards, tom lane

#13Andres Freund
andres@anarazel.de
In reply to: Tom Lane (#12)
1 attachment(s)
Re: conditional dropping of columns/constraints

Hi Tom, hi all,

On 05/06/2009 11:43 PM, Tom Lane wrote:

Andres Freund<andres@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

Attachments:

0001-Feature-DROP-COLUMN-CONSTRAINT-IF-EXISTS.patchtext/x-diff; name=0001-Feature-DROP-COLUMN-CONSTRAINT-IF-EXISTS.patchDownload
#14Bernd Helmle
mailings@oopsware.de
In reply to: Andres Freund (#13)
1 attachment(s)
Re: conditional dropping of columns/constraints

--On Donnerstag, Mai 07, 2009 12:57:56 +0200 Andres Freund
<andres@anarazel.de> wrote:

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

Here is a slightly updated version of the patch. I did some (very minor)
editing on the wording in the docs and cleaned a merge conflict in
tablecmds.c. I saw no code issues so far, make check passes without errors.
Maybe someone with more bison skills can comment on the duplicated parser
stuff, but it seems consistent with the rest of the code.

--
Thanks

Bernd

Attachments:

drop_column_constraint_if_exists_reviewed.patchtext/x-diff; charset=utf-8; name=drop_column_constraint_if_exists_reviewed.patchDownload
diff --git a/doc/src/sgml/ref/alter_table.sgml b/doc/src/sgml/ref/alter_table.sgml
index fe3f388..f065e78 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 and the column 
+       does not exist, no error is thrown. In this case a notice 
+       is issued instead.
       </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,206 ----
     </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 and the constraint
+       does not exist, no error is thrown. In this case a notice is issued instead.
       </para>
      </listitem>
     </varlistentry>
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 7afe6e7..52ae408 100644
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
*************** static void ATExecSetStorage(Relation re
*** 286,292 ****
  				 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,
--- 286,293 ----
  				 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 **
*** 299,306 ****
  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,
--- 300,308 ----
  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
*** 2621,2631 ****
  			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);
--- 2623,2633 ----
  			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
*** 2640,2649 ****
  			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);
--- 2642,2655 ----
  			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
*** 4161,4167 ****
  static void
  ATExecDropColumn(List **wqueue, Relation rel, const char *colName,
  				 DropBehavior behavior,
! 				 bool recurse, bool recursing)
  {
  	HeapTuple	tuple;
  	Form_pg_attribute targetatt;
--- 4167,4174 ----
  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
*** 4177,4187 ****
  	 * 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;
--- 4184,4204 ----
  	 * 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
*** 4247,4253 ****
  				{
  					/* Time to delete this child column, too */
  					ATExecDropColumn(wqueue, childrel, colName,
! 									 behavior, true, true);
  				}
  				else
  				{
--- 4264,4271 ----
  				{
  					/* Time to delete this child column, too */
  					ATExecDropColumn(wqueue, childrel, colName,
! 									 behavior, true, true,
! 									 false);
  				}
  				else
  				{
*************** createForeignKeyTriggers(Relation rel, F
*** 5361,5367 ****
  static void
  ATExecDropConstraint(Relation rel, const char *constrName,
  					 DropBehavior behavior,
! 					 bool recurse, bool recursing)
  {
  	List	   *children;
  	ListCell   *child;
--- 5379,5386 ----
  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
*** 5423,5434 ****
  
  	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
--- 5442,5463 ----
  
  	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
*** 5491,5497 ****
  				{
  					/* Time to delete this child constraint, too */
  					ATExecDropConstraint(childrel, constrName, behavior,
! 										 true, true);
  				}
  				else
  				{
--- 5520,5527 ----
  				{
  					/* 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 2c7d481..d0cab57 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 c61de97..cf28313 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 20ab0ba..e7ee4a2 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 a108b80..bd8dec9 100644
*** a/src/include/nodes/parsenodes.h
--- b/src/include/nodes/parsenodes.h
*************** typedef struct AlterTableCmd	/* one subc
*** 1145,1150 ****
--- 1145,1151 ----
  								 * 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;
#15Andrew Dunstan
andrew@dunslane.net
In reply to: Bernd Helmle (#14)
Re: conditional dropping of columns/constraints

Bernd Helmle wrote:

Here is a slightly updated version of the patch. I did some (very
minor) editing on the wording in the docs and cleaned a merge conflict
in tablecmds.c. I saw no code issues so far, make check passes without
errors. Maybe someone with more bison skills can comment on the
duplicated parser stuff, but it seems consistent with the rest of the
code.

The duplicated stuff in the grammar is pretty much what had to be done
for other DROP IF EXISTS items to avoid shift/reduce conflicts, so it's
quite kosher.

patch committed.

Thanks.

andrew