On 2017/05/17 11:22, Thomas Munro wrote:
> On Wed, May 17, 2017 at 10:13 AM, Kevin Grittner <kgri...@gmail.com> wrote:
>> On Tue, May 16, 2017 at 4:50 PM, Thomas Munro
>> <thomas.mu...@enterprisedb.com> wrote:
>>>
>>> I'm about to post a much simpler patch that collects uniform tuples
>>> from children, addressing the reported bug, and simply rejects
>>> transition tables on row-triggers on tables that are in either kind of
>>> inheritance hierarchy.  More soon...
>>
>> I agree that we can safely go that far, but not farther.  Thanks!
> 
> Here is that patch.  Thoughts?

I looked at the patch and noticed that there might be some confusion about
what's in the EState.es_root_result_relations array.

If you see the following block of code in ExecInitModifyTable():

    /* If modifying a partitioned table, initialize the root table info */
    if (node->rootResultRelIndex >= 0)
        mtstate->rootResultRelInfo = estate->es_root_result_relations +
                                                node->rootResultRelIndex;

You might be able to see that node->rootResultRelIndex is used as offset
into es_root_result_relations, which means the partitioned table root
being modified by this node.  The entries in es_root_result_relations
correspond to the partitioned table roots referenced as targets in
different parts of the query (for example, a WITH query might have its own
target partitioned tables).

So, in ExecSetupTriggerTransitionState() added by the patch, the following
code needs to be changed:

    /*
     * Find the ResultRelInfo corresponding to the relation that was
     * explicitly named in the statement.
     */
    if (estate->es_num_root_result_relations > 0)
    {
        /* Partitioned table.  The named relation is the first root. */
        targetRelInfo = &estate->es_root_result_relations[0];
    }

targetRelInfo should instead be set to mtstate->rootResultRelInfo that was
set in ExecInitModifyTable() as described above, IOW, as follows:

    /* Partitioned table. */
    if (mtstate->rootResultRelInfo != NULL)
        targetRelInfo = mtstate->rootResultRelInfo;

I guess that's what you intend to do here.

Sorry if the comments that I wrote about es_root_result_relations in what
got committed as e180c8aa8c were not clear enough.

Thanks,
Amit



-- 
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