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

On Mon, May 8, 2017 at 7:09 PM, Thomas Munro
<thomas(dot)munro(at)enterprisedb(dot)com> wrote:
> On Fri, May 5, 2017 at 8:29 AM, Thomas Munro
> <thomas(dot)munro(at)enterprisedb(dot)com> wrote:
>> On Fri, May 5, 2017 at 2:40 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>>> On Thu, May 4, 2017 at 4:46 AM, Thomas Munro
>>> <thomas(dot)munro(at)enterprisedb(dot)com> wrote:
>>>> On Thu, May 4, 2017 at 4:02 AM, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> wrote:
>>>>> Robert Haas wrote:
>>>>>> I suspect that most users would find it more useful to capture all of
>>>>>> the rows that the statement actually touched, regardless of whether
>>>>>> they hit the named table or an inheritance child.
>>>>>
>>>>> Yes, agreed. For the plain inheritance cases each row would need to
>>>>> have an indicator of which relation it comes from (tableoid); I'm not
>>>>> sure if such a thing would be useful in the partitioning case.
>>>>
>>>> On Thu, May 4, 2017 at 4:26 AM, David Fetter <david(at)fetter(dot)org> wrote:
>>>>> +1 on the not-duct-tape view of partitioned tables.
>>>>
>>>> Hmm. Ok. Are we talking about PG10 or PG11 here? Does this approach
>>>> makes sense?
>>>
>>> I was thinking PG10 if it can be done straightforwardly.
>>
>> Ok, I will draft a patch to do it the way I described and see what people think.
>
> FYI I am still working on this and will post a draft patch to do this
> (that is: make transition tables capture changes from children with
> appropriate tuple conversion) in the next 24 hours.

Ok, here is a first swing at it, for discussion.

In master, the decision to populate transition tables happens in
AfterTriggerSaveEvent (called by the ExecAR* functions) in trigger.c.
It does that if it sees that there are triggers on the
relation-being-modified that have transition tables.

With this patch, nodeModifyTuple.c makes a 'TriggerTransitionFilter'
object to override that behaviour, if there are child tables of either
kind. That is passed to the ExecAR* functions and thence
AfterTriggerSaveEvent, overriding its usual behaviour, so that it can
know that it needs collect tuples from child nodes and how to convert
them to the layout needed for the tuplestores if necessary.

Thoughts? I'm not yet sure about the locking situation. Generally
needs some more testing.

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