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

Hi all.

Sorry about the long delay.

On Tue, Jul 10, 2018 at 10:17 AM Ashutosh Bapat <
ashutosh(dot)bapat(at)enterprisedb(dot)com> wrote:

> On Wed, Mar 7, 2018 at 11:49 PM, Matheus de Oliveira
> <matioli(dot)matheus(at)gmail(dot)com> wrote:
> >
> >
> > Em 3 de mar de 2018 19:32, "Peter Eisentraut"
> > <peter(dot)eisentraut(at)2ndquadrant(dot)com> escreveu:
> >
> > On 2/20/18 10:10, Matheus de Oliveira wrote:
> >> Besides that, there is a another change in this patch on current ALTER
> >> CONSTRAINT about deferrability options. Previously, if the user did
> >> ALTER CONSTRAINT without specifying an option on deferrable or
> >> initdeferred, it was implied the default options, so this:
> >>
> >> ALTER TABLE tbl
> >> ALTER CONSTRAINT con_name;
> >>
> >> Was equivalent to:
> >>
> >> ALTER TABLE tbl
> >> ALTER CONSTRAINT con_name NOT DEFERRABLE INITIALLY IMMEDIATE;
> >
> > Oh, that seems wrong. Probably, it shouldn't even accept that syntax
> > with an empty options list, let alone reset options that are not
> > mentioned.
> >
> >
> > Yeah, it felt really weird when I noticed it. And I just noticed while
> > reading the source.
> >
> > Can
> >
> > you prepare a separate patch for this issue?
> >
> >
> > I can do that, no problem. It'll take awhile though, I'm on a trip and
> will
> > be home around March 20th.
>
> Matheus,
> When do you think you can provide the patch for bug fix?
>
>
Attached the patch for the bug fix, all files with naming
`postgresql-fix-alter-constraint-${version}.patch`, with `${version}` since
9.4, where ALTER CONSTRAINT was introduced.

Not sure if you want to apply to master/12 as well (since the other patch
applies that as well), but I've attached it anyway, so you can decide.

> Also, the patch you originally posted doesn't apply cleanly. Can you
> please post a rebased version?
>
>
Attached the rebased version that applies to current master,
`postgresql-alter-constraint.v3.patch`.

> The patch contains 70 odd lines of test SQL and 3600 odd lines of
> output. The total patch is 4200 odd lines. I don't think that it will
> be acceptable to add that huge an output to the regression test. You
> will need to provide a patch with much smaller output addition and may
> be a smaller test as well.
>
>
You are correct. I have made a test that tries all combinations of ALTER
CONSTRAINT ON UPDATE/DELETE ACTION, but it caused a really huge output. I
have changed that to a simple DO block, and still trying all possibilities
but now I just verify if the ALTER matches the same definition on
pg_trigger of a constraint that was created with the target action already,
seems simpler and work for any change.

Please, let me know if you all think any change should be made on this
patch.

Best regards,
--
Matheus de Oliveira