Thread: conditional dropping of columns/constraints
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
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
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
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!"
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.
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
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
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
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
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
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
Attachment
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
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
Attachment
--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
Attachment
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