On Fri, May 19, 2017 at 1:38 AM, Kevin Grittner <kgri...@gmail.com> wrote:
> On Thu, May 18, 2017 at 5:16 AM, Amit Langote
> <langote_amit...@lab.ntt.co.jp> wrote:
>
>> Do we need to update documentation?  Perhaps, some clarification on the
>> inheritance/partitioning behavior somewhere.
>
> Yeah, I think so.

Here is an attempt at documenting the situation in the CREATE TRIGGER
notes section.

>> -    Assert((enrmd->reliddesc == InvalidOid) != (enrmd->tupdesc == NULL));
>> +    Assert((enrmd->reliddesc == InvalidOid) !=
>> +           (enrmd->tupdesc == NULL));
>>
>> Perhaps, unintentional change?
>
> Agreed; line is not long enough to need to wrap.

Fixed.

>> I'm not sure if it's significant for transition tables, but what if a
>> partition's BR trigger modified the tuple?  Would we want to include the
>> modified version of the tuple in the transition table or the original as
>> the patch does?  Same for the code in CopyFrom().
>
> Good spot!  If the BR trigger on the child table modifies or
> suppresses the action, I strongly feel that must be reflected in the
> transition table.  This needs to be fixed.

Gah.  Right.  In the attached version, there is a still an 'original
tuple' optimisation for insertions (avoiding parent -> child -> parent
conversion), but it's disabled if there are any BEFORE INSERT or
INSTEAD OF INSERT row-level triggers.

That's demonstrated by this part of the regression test, which
modifies the value inserted into the 'CCC' partition (and similar case
for COPY):

insert into parent values ('AAA', 42), ('BBB', 42), ('CCC', 66);
NOTICE:  trigger = parent_stmt_trig, old table = <NULL>, new table =
(AAA,42), (BBB,42), (CCC,1066)

On Thu, May 18, 2017 at 10:16 PM, Amit Langote
<langote_amit...@lab.ntt.co.jp> wrote:
> +typedef struct TriggerTransitionState
> +{
> ...
> +    bool        ttf_delete_old_table;
>
> Just curious: why ttf_?  TriggerTransition field?

Oops.  Changed to "tts_".  I had renamed this struct but not the members.

-- 
Thomas Munro
http://www.enterprisedb.com

Attachment: transition-tuples-from-child-tables-v7.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to