From: | Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> |
---|---|
To: | Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, Kevin Grittner <kgrittn(at)gmail(dot)com> |
Cc: | Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, David Fetter <david(at)fetter(dot)org>, Rajkumar Raghuwanshi <rajkumar(dot)raghuwanshi(at)enterprisedb(dot)com>, Amit Langote <amitlangote09(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, pgsql-hackers-owner(at)postgresql(dot)org |
Subject: | Re: transition table behavior with inheritance appears broken (was: Declarative partitioning - another take) |
Date: | 2017-05-17 06:04:42 |
Message-ID: | 94b79287-a087-b5a9-19b1-532dae3cd6ea@lab.ntt.co.jp |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 2017/05/17 11:22, Thomas Munro wrote:
> On Wed, May 17, 2017 at 10:13 AM, Kevin Grittner <kgrittn(at)gmail(dot)com> wrote:
>> On Tue, May 16, 2017 at 4:50 PM, Thomas Munro
>> <thomas(dot)munro(at)enterprisedb(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Ashutosh Bapat | 2017-05-17 06:18:06 | Re: Hash Functions |
Previous Message | Ashutosh Bapat | 2017-05-17 05:55:17 | Re: statement_timeout is not working as expected with postgres_fdw |