Re: [HACKERS] transition table behavior with inheritance appearsbroken (was: Declarative partitioning - another take)

On 2017/05/19 15:16, Thomas Munro wrote:
> On Fri, May 19, 2017 at 5:51 PM, Amit Langote
> <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>> I saw in the latest patch that now ExecSetupTriggerTransitionState() looks
>> at mtstate->mt_partition_dispatch_info when setting up the transition
>> conversion map.  In the case where it's non-NULL, you may have realized
>> that mt_transition_tupconv_map will be an exact copy of
>> mt_partition_tupconv_maps that's already built.  Would it perhaps be a
>> good idea to either share the same or make a copy using memcpy() instead
>> of doing the convert_tuples_by_name() calls again?
> 
> Isn't it the opposite?  mt_partition_tupconv_maps holds maps that
> convert the parent format to the partition format.
> mt_transition_tupconv_maps holds maps that convert the partition
> format to the parent format (= transition tuplestore format).

You're right, never mind.

>>> On Thu, May 18, 2017 at 10:16 PM, Amit Langote
>>> <Langote_Amit_f8@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.
>>
>> Ah.  BTW, maybe it's not a problem, but the existing TupleTableSlot's
>> member names are prefixed with tts_, too. :)
> 
> Would TransitionCaptureState be a better name for this struct?

Yes.  Although, losing the Trigger prefix might make it sound a bit
ambiguous though.  Right above its definition, we have TriggerData.  So,
maybe TriggerTransitionCaptureState or TriggerTransitionCaptureData or
TriggerTransitionData may be worth considering.

Thanks,
Amit