Re: BUG #15361: Add column if not exists create duplicate constraint

Stephen Frost <sfrost(at)snowman(dot)net> writes:
> * Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
>> The thing I was half-remembering was probably this bug report about
>> the unique-constraint aspect of the same problem:
>> https://www.postgresql.org/message-id/flat/152509815280.19803.16118194452213577808%40wrigleys.postgresql.org
>> wherein it was argued that if the IF NOT EXISTS fires, it should prevent
>> all side-effects of the ADD COLUMN clause it's attached to, whether or not
>> any of the subsidiary objects exist (in some form) already. If you buy
>> that theory then there's a sketch for a fix there. Nobody's done anything
>> about it AFAIK.

> For my 2c, at least, I continue to be of the opinion (as it seems the OP
> is also..) that IF NOT EXISTS means "ONLY DO THIS IF THE OBJECT
> REFERENCED DOESN'T EXIST".

The problem is precisely that it's very fuzzy exactly what THIS is.
The case at hand, with an FK constraint, is maybe even a better
demonstration of that than the cases we considered previously.
Currently, if the column exists but lacks an FK constraint, the code
will make sure one gets added; with what you propose, it won't.
You can argue all day about which semantics are more useful, but I don't
see that there's a bright line dividing right from wrong here ---
especially since, AFAICS, there's not currently any way for a user to
get the other behavior if she doesn't like the one we choose.

The latter point could be addressed if there were also an IF NOT EXISTS
syntax for constraints. Then the useful aspect of the current behavior
could be duplicated with

ALTER TABLE t
ADD COLUMN IF NOT EXISTS c1 int,
ADD CONSTRAINT IF NOT EXISTS c1_fk FOREIGN KEY (c1) REFERENCES othert(c2);

I'd imagine that we'd address the "what is it that doesn't exist" issue
by saying that you have to attach IF NOT EXISTS to named-constraint
syntax, and what's checked is just the existence of a constraint by that
name, not whether its properties are the same. So this isn't an exact
replacement for what happens now, but it's probably close enough to
cover the real use-cases.

> I still consider the current behavior to be
> a bug, but I could see an argument for not back-patching it out of
> concern about breaking scripts in a back-patch and because the code
> looks like it'd need to be whacked around some.

It's definitely not going to be a back-patchable change. But I could
get behind a change that addresses the confusion without taking away
arguably-useful functionality.

regards, tom lane