From: | Dmitry Dolgov <9erthalion6(at)gmail(dot)com> |
---|---|
To: | Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> |
Cc: | Michael Paquier <michael(at)paquier(dot)xyz>, Marco Slot <marco(at)citusdata(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Onder Kalaci <onder(at)citusdata(dot)com> |
Subject: | Re: Segfault when creating partition with a primary key and sql_drop trigger exists |
Date: | 2018-10-04 16:08:41 |
Message-ID: | CA+q6zcWDHJB9K+8-RsiG1wXW=L38YNHx+RtdzDe9LEr-+qjQXA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
> On Wed, 3 Oct 2018 at 09:53, Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Fri, Sep 28, 2018 at 12:17:00PM +0900, Michael Paquier wrote:
> > I think that Alvaro should definitely look at this patch to be sure, or
> > I could do it, but I would need to spend way more time on this and check
> > event trigger interactions.
> >
> > Anyway, I was struggling a bit regarding the location where adding a
> > regression test. event_trigger.sql makes the most sense but in tests
> > for drops the objects are created before the event trigger is defined,
> > so that would need to move around so as the original problem is
> > reproducible. Perhaps you have an idea for that?
>
> Okay. I have spent more time on this issue, and I have been able to
> integrate a test in the existing event_trigger.sql which is able to
> reproduce the reported failure. Attached is what I am finishing with.
>
> I still want to do more testing on it, and the day is ending here.
>
> Thoughts?
Sorry, couldn't answer your previous message, since was away. So far I don't
see any problems with your proposed patch.
> On Thu, 4 Oct 2018 at 17:22, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> wrote:
>
> I admit I'm surprised that your patch fixes the bug. sql_drop was added
> before the command-stashing was added for pg_event_trigger_ddl_commands
> was added, and sql_drop only processes objects from the list passed to
> performMultipleDeletions, so adding the EventTriggerAlterTableStart() /
> End() calls should not affect it ... evidently I must be missing
> something here.
>
> Still looking.
I also find strange another part of this problem, namely why we ended up doing
AlterTableInternal at all, since I assumed that after MergeAttributes all
attnotnull should be merged with OR. But for example these would work:
CREATE TABLE collections_list_1
PARTITION OF collections_list
FOR VALUES IN (1);
CREATE TABLE collections_list_1
PARTITION OF collections_list (key, ts, collection_id not null, value)
FOR VALUES IN (1);
Looks like in MergeAttributes at the branch:
if (is_partition && list_length(saved_schema) > 0)
we override not null property of ColumnDef:
if (coldef->is_from_parent)
{
coldef->is_not_null = restdef->is_not_null;
It looks a bit confusing, so I wonder if it's how it should be?
From | Date | Subject | |
---|---|---|---|
Next Message | Alvaro Herrera | 2018-10-04 16:30:27 | Re: Segfault when creating partition with a primary key and sql_drop trigger exists |
Previous Message | Konstantin Knizhnik | 2018-10-04 15:45:20 | Re: [HACKERS] Secondary index access optimizations |