Re: transition table behavior with inheritance appears broken (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(at)lab(dot)ntt(dot)co(dot)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(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.
>>
>> 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