Re: [Sender Address Forgery]Re: [Sender Address Forgery]Re: [HACKERS] path toward faster partition pruning

Amit Langote <amitlangote09@gmail.com>

From: Amit Langote <amitlangote09@gmail.com>
To: David Rowley <david.rowley@2ndquadrant.com>
Cc: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>, 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-17T10:48:21Z
Lists: pgsql-hackers
Hi David.

On Wed, Jan 17, 2018 at 6:19 PM, David Rowley
<david.rowley@2ndquadrant.com> wrote:
> On 17 January 2018 at 17:05, David Rowley <david.rowley@2ndquadrant.com> wrote:
>> 6. Which brings me to; why do we need match_clauses_to_partkey at all?
>> classify_partition_bounding_keys seems to do all the work
>> match_clauses_to_partkey does, plus more. Item #3 above is caused by
>> an inconsistency between these functions. What benefit does
>> match_clauses_to_partkey give? I might understand if you were creating
>> list of clauses matching each partition key, but you're just dumping
>> everything in one big list which causes
>> classify_partition_bounding_keys() to have to match each clause to a
>> partition key again, and classify_partition_bounding_keys is even
>> coded to ignore clauses that don't' match any key, so it makes me
>> wonder what is match_clauses_to_partkey actually for?
>
> I started to look at this and ended up shuffling the patch around a
> bit to completely remove the match_clauses_to_partkey function.
>
> I also cleaned up some of the comments and shuffled some fields around
> in some of the structs to shrink them down a bit.
>
> All up, this has saved 268 lines of code in the patch.
>
> src/backend/catalog/partition.c       | 296 ++++++++++++++++-----------
> src/backend/optimizer/path/allpaths.c | 368 ++--------------------------------
> 2 files changed, 198 insertions(+), 466 deletions(-)
>
> It's had very minimal testing. Really I've only tested that the
> regression tests pass.
>
> I also fixed up the bad assumption that IN lists will contain Consts
> only which hopefully fixes the crash I reported earlier.
>
> I saw you'd added a check to look for contradicting IS NOT NULL
> clauses when processing an IS NULL clause, but didn't do anything for
> the opposite case. I added code for this so it behaves the same
> regardless of the clause order.
>
> Can you look at my changes and see if I've completely broken anything?

Thanks for the patch.  I applied the patch and see that it didn't
break any tests, although haven't closely reviewed the code yet.

I'm concerned that after your patch to remove
match_clauses_to_partkey(), we'd be doing more work than necessary in
some cases.  For example, consider the case of using run-time pruning
for nested loop where the inner relation is a partitioned table.  With
the old approach, get_partitions_from_clauses() would only be handed
the clauses that are known to match the partition keys (which most
likely is fewer than all of the query's clauses), so
get_partitions_from_clauses() doesn't have to do the work of filtering
non-partition clauses every time (that is, for every outer row).
That's why I had decided to keep that part in the planner.

Thanks,
Amit


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.