Re: [PATCH] Add support for ON UPDATE/DELETE actions on ALTER CONSTRAINT

From: Wolfgang Walther <walther(at)technowledgy(dot)de>
To: Matheus de Oliveira <matioli(dot)matheus(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, José Arthur Benetasso Villanova <jose(dot)arthur(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Michael Paquier <michael(at)paquier(dot)xyz>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>
Subject: Re: [PATCH] Add support for ON UPDATE/DELETE actions on ALTER CONSTRAINT
Date: 2020-08-03 16:31:19
Message-ID: 1cb598ca-7cf1-b422-3f17-68005e7b0c5b@technowledgy.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Tom Lane:
> We don't generally act that way in other ALTER commands and I don't see
> a strong argument to start doing so here. [...]
>
> In short, I'm inclined to argue that this variant of ALTER TABLE
> should replace *all* the fields of the constraint with the same
> properties it'd have if you'd created it fresh using the same syntax.
> This is by analogy to CREATE OR REPLACE commands, which don't
> preserve any of the old properties of the replaced object. Given
> the interactions between these fields, I think you're going to end up
> with a surprising mess of ad-hoc choices if you do differently.
> Indeed, you already have, but I think it'll get worse if anyone
> tries to extend the feature set further.

I don't think the analogy to CREATE OR REPLACE holds. Semantically
REPLACE and ALTER are very different. Using ALTER the expectation is to
change something, keeping everything else unchanged. Looking at all the
other ALTER TABLE actions, especially ALTER COLUMN, it looks like every
command does exactly one thing and not more. I don't think deferrability
and ON UPDATE / ON CASCADE should be changed together at all, neither
implicitly nor explicitly.

There seems to be a fundamental difference between deferrability and the
ON UPDATE/ON DELETE clauses as well - the latter only apply to FOREIGN
KEYs, while the former apply to multiple types of constraints.

Matheus de Oliveira:
> I'm still not sure if the chosen path is the best way. But I'd be glad
> to follow any directions we all see fit.
>
> For now, this patch applies two methods:
> 1. Changes full constraint definition (which keeps compatibility with
> current ALTER CONSTRAINT):
>     ALTER CONSTRAINT [<on_update>] [<on_delete>] [<deferrability>]
> 2. Changes only the subset explicit seem in the command (a new way, I've
> chosen to just add SET in the middle, similar to `ALTER COLUMN ... SET
> {DEFAULT | NOT NULL}` ):
>     ALTER CONSTRAINT SET [<on_update>] [<on_delete>] [<deferrability>]
>
> I'm OK with changing the approach, we just need to chose the color :D

The `ALTER CONSTRAINT SET [<on_update>] [<on_delete>] [<deferrability>]`
has the same problem about implied changes: What happens if you only do
e.g. ALTER CONSTRAINT SET ON UPDATE xy - will the ON DELETE part be kept
as-is or set to the default?

Also, since the ON UPDATE/ON DELETE just applies to FOREIGN KEYs and no
other constraints, there's one level of "nesting" missing in your SET
variant, I think.

I suggest to:

- keep `ALTER CONSTRAINT constraint_name [ DEFERRABLE | NOT DEFERRABLE ]
[ INITIALLY DEFERRED | INITIALLY IMMEDIATE ]` exactly as-is

- add both:
+ `ALTER CONSTRAINT constraint_name [ALTER] FOREIGN KEY ON UPDATE
referential_action`
+ `ALTER CONSTRAINT constraint_name [ALTER] FOREIGN KEY ON DELETE
referential_action`

This does not imply any changes, that are not in the command - very much
analog to the ALTER COLUMN variants.

This could also be extended in the future with stuff like `ALTER
CONSTRAINT constraint_name [ALTER] FOREIGN KEY MATCH [ FULL | PARTIAL |
SIMPLE ]`.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2020-08-03 16:35:15 Re: Confusing behavior of create table like
Previous Message Ashutosh Sharma 2020-08-03 16:13:04 Re: recovering from "found xmin ... from before relfrozenxid ..."