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

Поиск
Список
Период
Сортировка
От Matheus de Oliveira
Тема Re: [PATCH] Add support for ON UPDATE/DELETE actions on ALTER CONSTRAINT
Дата
Msg-id CAJghg4J=c0jWzX0j2AhtzWjc7xt2OEjSY01bRNTwtu649KE3Aw@mail.gmail.com
обсуждение исходный текст
Ответ на Re: [PATCH] Add support for ON UPDATE/DELETE actions on ALTER CONSTRAINT  (Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>)
Ответы Re: [PATCH] Add support for ON UPDATE/DELETE actions on ALTERCONSTRAINT  (Michael Paquier <michael@paquier.xyz>)
Список 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


Вложения

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Thomas Munro
Дата:
Сообщение: Re: [HACKERS] proposal - Default namespaces for XPath expressions(PostgreSQL 11)
Следующее
От: Tom Lane
Дата:
Сообщение: Re: XMLNAMESPACES (was Re: Clarification of nodeToString() use cases)