Re: pgsql: Clarify use of temporary tables within partition trees

(re-adding committers list)

On 3 July 2018 at 20:31, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp> wrote:
> 1. I still insist that it's better for the newly added code to be near the
> top of the function body than in the middle, which brings me to...

That will cause the AppendRelInfo not to be built for a partitioned
table with no partitions. I'm not personally certain that doing that
comes without consequence.  Are you certain of that?  If not 100%,
then I think that's more of a task for PG12. I'm trying to propose
removing dead code for PG11 and on.

My current thoughts are that moving this test up a few lines is
unlikely to improve performance in a real-world situation, so it does
not seem worth the risk for a backpatch to me.

> 2. While we're at fixing the code around here, I think we should think
> about trying to get rid of the *non-dead* code that produces a structure
> that isn't used anywhere, which I was under the impression, 0a480502b09
> [1] already did (cc'ing Ashutosh).  To clarify, we still unnecessarily
> create a "duplicate" RTE for partitioned tables in a partition tree
> (non-leaf tables) in its role as a child.  So, for the above query, there
> end up being created 4 entries in the query's range table (2 for 'p' and 2
> for 'pd').  That makes sense for plain inheritance, because even the root
> parent table in a plain inheritance tree is a regular table containing
> data.  That's not true for partition inheritance trees, where non-leaf
> tables contain no data, so we don't create a plan to scan them (see
> d3cc37f1d80 [2]), which in turn means we don't need to create the
> redundant "duplicate" child RTEs for them either.
>
> See attached my delta patch to address both 1 and 2.

I recently saw this too and wondered about it. I then wrote a patch to
just have it create a single RangeTblEntry for each partitioned table.
The tests all passed.

I'd categorise this one the same as I have #1 above, i.e. not
backpatch material. It seems like something useful to look into for
v12 though. I assumed this was done for a reason and that I just
didn't understand what that reason was. I don't recall any comments to
explain the reason why we build two RangeTblEntrys for each
partitioned table.

In light of what Amit has highlighted, I'm still standing by the v3
patch assuming the typo is fixed.

-- 
 David Rowley                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services