Horton

Re: [HACKERS] path toward faster partition pruning

David Rowley <david.rowley@2ndquadrant.com>

View thread View on postgr.es
From: David Rowley <david.rowley@2ndquadrant.com>
To: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>, Rajkumar Raghuwanshi <rajkumar.raghuwanshi@enterprisedb.com>, Robert Haas <robertmhaas@gmail.com>, Dilip Kumar <dilipbalaut@gmail.com>, Beena Emerson <memissemerson@gmail.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2018-01-12T09:09:32Z
Lists: 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

Commits

  1. Fix assorted partition pruning bugs

  2. Make gen_partprune_steps static

  3. Remove useless 'default' clause

  4. Reorganize partitioning code

  5. Use custom hash opclass for hash partition pruning

  6. Blindly attempt to fix sepgsql tests broken due to 9fdb675fc5.

  7. Attempt to fix endianess issues in new hash partition test.

  8. Faster partition pruning

  9. For partitionwise join, match on partcollation, not parttypcoll.

  10. Revise API for partition bound search functions.

  11. Revise API for partition_rbound_cmp/partition_rbound_datum_cmp.

  12. Fix possible crash in partition-wise join.

  13. Refactor code for partition bound searching

  14. New C function: bms_add_range

  15. Add extensive tests for partition pruning.

  16. Add null test to partition constraint for default range partitions.

  17. Remove BufFile's isTemp flag.

  18. Make OWNER TO subcommand mention consistent

  19. Fix index matching for operators with mixed collatable/noncollatable inputs.