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

From: Ibrar Ahmed <ibrar(dot)ahmad(at)gmail(dot)com>
To: Wolfgang Walther <walther(at)technowledgy(dot)de>
Cc: Matheus de Oliveira <matioli(dot)matheus(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, 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: 2021-03-04 11:28:03
Message-ID: CALtqXTfH05JBo8YgHj6CA4fp5uLnkmb7jbYd3K=FLVRU34n+PA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Aug 3, 2020 at 9:31 PM Wolfgang Walther <walther(at)technowledgy(dot)de>
wrote:

> 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 ]`.
>
>
>

This patch set no longer applies
http://cfbot.cputube.org/patch_32_1533.log

Can we get a rebase?

I am marking the patch "Waiting on Author"

--
Ibrar Ahmed

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Matthias van de Meent 2021-03-04 11:32:38 Re: Improvements and additions to COPY progress reporting
Previous Message Ibrar Ahmed 2021-03-04 11:25:06 Re: About to add WAL write/fsync statistics to pg_stat_wal view