Re: [HACKERS] path toward faster partition pruning - Mailing list pgsql-hackers

From David Rowley
Subject Re: [HACKERS] path toward faster partition pruning
Date
Msg-id CAKJS1f_iquLwAFry6jVL3rFD5+cuqkq-DHgq_OsmECs+=EG5HA@mail.gmail.com
Whole thread Raw
In response to Re: [HACKERS] path toward faster partition pruning  (David Rowley <david.rowley@2ndquadrant.com>)
Responses Re: [Sender Address Forgery]Re: [HACKERS] path toward fasterpartition pruning
List pgsql-hackers
On 10 January 2018 at 17:18, David Rowley <david.rowley@2ndquadrant.com> wrote:
> Basically, the changes to add_paths_to_append_rel() are causing
> duplication in partition_rels.
>
> A test case is:
>
> create table part (a int, b int) partition by list(a);
> create table part1 partition of part for values in(1) partition by list (b);
> create table part2 partition of part1 for values in(1);
>
> select * from part;
>
> partition_rels ends up with 3 items in the list, but there's only 2
> partitions here. The reason for this is that, since planning here is
> recursively calling add_paths_to_append_rel, the list for part ends up
> with itself and part1 in it, then since part1's list already contains
> itself, per set_append_rel_size's "rel->live_partitioned_rels =
> list_make1_int(rti);", then part1 ends up in the list twice.
>
> It would be nicer if you could use a RelIds for this, but you'd also
> need some way to store the target partition relation since
> nodeModifyTable.c does:
>
> /* The root table RT index is at the head of the partitioned_rels list */
> if (node->partitioned_rels)
> {
>     Index root_rti;
>     Oid root_oid;
>
>     root_rti = linitial_int(node->partitioned_rels);
>     root_oid = getrelid(root_rti, estate->es_range_table);
>     rel = heap_open(root_oid, NoLock); /* locked by InitPlan */
> }
>
> You could also fix it by instead of doing:
>
> /*
> * Accumulate the live partitioned children of this child, if it's
> * itself partitioned rel.
> */
> if (childrel->part_scheme)
>     partitioned_rels = list_concat(partitioned_rels,
>        childrel->live_partitioned_rels);
>
> do something along the lines of:
>
> if (childrel->part_scheme)
> {
>     ListCell *lc;
>     ListCell *start = lnext(list_head(childrel->live_partitioned_rels));
>
>     for_each_cell(lc, start)
>        partitioned_rels = lappend_int(partitioned_rels,
>        lfirst_int(lc));
> }
>
> Although it seems pretty fragile. It would probably be better to find
> a nicer way of handling all this.

Hi Amit,

I also noticed earlier that this is still broken in v19.

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


pgsql-hackers by date:

Previous
From: Ashutosh Bapat
Date:
Subject: Re: CREATE ROUTINE MAPPING
Next
From: Michael Paquier
Date:
Subject: Re: Enhance pg_stat_wal_receiver view to display connected host