Re: [HACKERS] path toward faster partition pruning

Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>

From: Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>
To: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Cc: Robert Haas <robertmhaas@gmail.com>, David Rowley <david.rowley@2ndquadrant.com>, Rajkumar Raghuwanshi <rajkumar.raghuwanshi@enterprisedb.com>, Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>, Jesper Pedersen <jesper.pedersen@redhat.com>, Amit Langote <amitlangote09@gmail.com>, Dilip Kumar <dilipbalaut@gmail.com>, Beena Emerson <memissemerson@gmail.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2018-02-28T11:25:47Z
Lists: pgsql-hackers
On Tue, Feb 27, 2018 at 3:03 PM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:
> Attached an updated version in which I incorporated some of the revisions
> that David Rowley suggested to OR clauses handling (in partprune.c) that
> he posted as a separate patch on the run-time pruning thread [1].

Some comments on 0001.

             partnatts != part_scheme->partnatts)
             continue;

-        /* Match the partition key types. */
+        /*
+         * Match the partition key types and partitioning-specific collations.
+         */

We are comparing opfamily and opclass input type as well, but this comment
doesn't explicitly mention those like it mentions collation. Instead, I think
we should just say, "Match partition key type properties"

You had commented on "advanced partition matching code" about asserting that
parsupfuncs also match. Robert too has expressed similar opinion upthread. May
be we should at least try to assert that the function OIDs match.

-    Oid           *parttypcoll;    /* OIDs of collations of partition keys. */
+
+    /*
+     * We store both the collation implied by the partition key's type and the
+     * one specified for partitioning.  Values in the former are used as
+     * varcollid in the Vars corresponding to simple column partition keys so
+     * as to make them match corresponding Vars appearing elsewhere in the
+     * query tree.  Whereas, the latter is used when actually comparing values
+     * against partition bounds datums, such as, when doing partition pruning.
+     */
+    Oid           *parttypcoll;
+    Oid           *partcollation;

As you have already mentioned upthread only partcollation is needed, not
parttypcoll.

     /* Cached information about partition key data types. */
     int16       *parttyplen;
     bool       *parttypbyval;
+
+    /*
+     * Cached array of partitioning comparison functions' fmgr structs.  We
+     * don't compare these when trying to match two partition schemes.
+     */

I think this comment should go away. The second sentence doesn't explain why
and if it did so it should do that in find_partition_scheme() not here.
partsupfunc is another property of partition keys that is cached like
parttyplen, parttypbyval. Why does it deserve a separate comment when others
don't?



-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company


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.