Re: [PATCH] Add support for ON UPDATE/DELETE actions on ALTER CONSTRAINT - Mailing list pgsql-hackers

From Matheus de Oliveira
Subject Re: [PATCH] Add support for ON UPDATE/DELETE actions on ALTER CONSTRAINT
Date
Msg-id CAJghg4J=c0jWzX0j2AhtzWjc7xt2OEjSY01bRNTwtu649KE3Aw@mail.gmail.com
Whole thread Raw
In response to Re: [PATCH] Add support for ON UPDATE/DELETE actions on ALTER CONSTRAINT  (Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>)
Responses Re: [PATCH] Add support for ON UPDATE/DELETE actions on ALTERCONSTRAINT  (Michael Paquier <michael@paquier.xyz>)
List pgsql-hackers
Hi all.

Sorry about the long delay.

On Tue, Jul 10, 2018 at 10:17 AM Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> wrote:
On Wed, Mar 7, 2018 at 11:49 PM, Matheus de Oliveira
<matioli.matheus@gmail.com> wrote:
>
>
> Em 3 de mar de 2018 19:32, "Peter Eisentraut"
> <peter.eisentraut@2ndquadrant.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


Attachment

pgsql-hackers by date:

Previous
From: Thomas Munro
Date:
Subject: Re: [HACKERS] proposal - Default namespaces for XPath expressions(PostgreSQL 11)
Next
From: Tom Lane
Date:
Subject: Re: XMLNAMESPACES (was Re: Clarification of nodeToString() use cases)