On Thu, Mar 16, 2017 at 6:03 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
> I think we'll need to store *somewhere* the mapping of which inh=false
> partitioned table RTE is the child of which inh=true (IOW, parent)
> partitioned table RTE.
I mean, for the children you're going to scan, that seems to be
necessary so that you can do things like translate targetlists to use
the correct varno. But for the children you're not going to scan,
well, you need to know which ones they are so you can lock them, but
do you really need the parent-child mappings? Or just a list of which
ones there are?
> I've come to think that AppendRelInfos, although
> contain extraneous information that won't be used, are better than
> inventing something new altogether for time being. AppendRelInfos are
> referred to a few times by query_planner() steps before we eventually get
> to either set_append_rel_pathlist() or inheritance_planner(), so not
> changing that approach seems less worrisome for now. So now if we both
> create child RTEs and AppendRelInfos for the partitioned tables, we don't
> need to change expand_inherited_rtentry() at all with this patch.
> Finally, set_append_rel_size/pathlist() and inheritance_planner() skip the
> child partitioned table RTEs, because no path/plan need to be created. We
> can do away with having to create RelOptInfos for child partitioned table
> RTEs, which I found to be not that invasive.
Yes, but on the flip side, you're having to add code in a lot of
places -- I think I counted 7 -- where you turn around and ignore
those AppendRelInfos. That's a lot; how do we know we've got them
all? I'm not sure what the patch would look like the other way, but
I'm hoping that you could just keep the list of partitioned table RTIs
someplace that mostly gets ignored, and then all of that special
handling could be ripped out.
> Append node elision does not occur in the one-child case. With the patch:
Oh, OK. Somehow the commit message you included led me to the
contrary conclusion.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company