From: | Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> |
---|---|
To: | Michael Paquier <michael(at)paquier(dot)xyz> |
Cc: | David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: pgsql: Clarify use of temporary tables within partition trees |
Date: | 2018-07-03 08:49:11 |
Message-ID: | 528b14b6-3e08-c8c0-16bc-a4366dbfaf1e@lab.ntt.co.jp |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-committers pgsql-hackers |
On 2018/07/03 17:31, Amit Langote wrote:
> On 2018/07/03 16:05, Michael Paquier wrote:
>> On Tue, Jul 03, 2018 at 03:49:44PM +0900, Amit Langote wrote:
>>> I forgot that expand_partitioned_rtentry() will recursively call itself if
>>> a partition is itself a partitioned table, in which case the above
>>> code helps.
>>
>> Actually look at the coverage reports:
>> https://coverage.postgresql.org/src/backend/optimizer/prep/prepunion.c.gcov.html
>> 1742 : /*
>> 1743 : * If the partitioned table has no partitions or all the partitions are
>> 1744 : * temporary tables from other backends, treat this as non-inheritance
>> 1745 : * case.
>> 1746 : */
>> 1747 4920 : if (!has_child)
>> 1748 0 : parentrte->inh = false;
>> 1749 4920 : }
>>
>> expand_partitioned_rtentry() never disables this flag on recursive calls
>> with a multi-level tree. Could it be possible to get a test which
>> closes the gap?
>
> I guess that it will be hard as you mentioned before on the thread that
> led to this commit. We can't write regression tests which require using
> temporary partitions from other sessions.
>
> Anyway, I just wanted to say that I was wrong when I said that the block
> added by the patch is unreachable. It *is* reachable for multi-level
> partitioning. For example, it will execute in the following case:
>
> create table p (a int, b int) partition by list (a);
> create table pd partition of p default partition by list (b);
> select * from p;
>
> expand_partitioned_rtentry will get called twice and the newly added code
> would result in early return from the function in the 2nd invocation which
> is for 'pd'.
>
> But,
>
> 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...
>
> 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.
>
> Thanks,
> Amit
>
> [1] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=0a480502b09
>
> [2] https://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=d3cc37f1d80
For some reason, ML address got removed from the list of address when
sending the above message.
Thanks,
Amit
From | Date | Subject | |
---|---|---|---|
Next Message | David Rowley | 2018-07-03 09:05:41 | Re: pgsql: Clarify use of temporary tables within partition trees |
Previous Message | Michael Paquier | 2018-07-03 07:05:51 | Re: pgsql: Clarify use of temporary tables within partition trees |
From | Date | Subject | |
---|---|---|---|
Next Message | Craig Ringer | 2018-07-03 08:58:39 | Re: Copy function for logical replication slots |
Previous Message | Carter Thaxton | 2018-07-03 08:46:33 | Re: Add --include-table-data-where option to pg_dump, to export only a subset of table data |