Re: transition table behavior with inheritance appears broken (was: Declarative partitioning - another take)

On Fri, May 19, 2017 at 1:38 AM, Kevin Grittner <kgrittn(at)gmail(dot)com> wrote:
> On Thu, May 18, 2017 at 5:16 AM, Amit Langote
> <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)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_f8(at)lab(dot)ntt(dot)co(dot)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