Re: [HACKERS] path toward faster partition pruning

Amit Langote <langote_amit_f8@lab.ntt.co.jp>

From: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
To: Alvaro Herrera <alvherre@2ndquadrant.com>, Michael Paquier <michael@paquier.xyz>
Cc: Marina Polyakova <m.polyakova@postgrespro.ru>, Amit Langote <amitlangote09@gmail.com>, David Rowley <david.rowley@2ndquadrant.com>, Dilip Kumar <dilipbalaut@gmail.com>, Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>, Robert Haas <robertmhaas@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, pgsql-hackers@postgresql.org
Date: 2018-05-09T02:06:45Z
Lists: pgsql-hackers

Attachments

Hi.

On 2018/05/09 7:05, Alvaro Herrera wrote:
> So I found that this query also crashed (using your rig),
> 
> create table coercepart (a varchar) partition by list (a);
> create table coercepart_ab partition of coercepart for values in ('ab');
> create table coercepart_bc partition of coercepart for values in ('bc');
> create table coercepart_cd partition of coercepart for values in ('cd');
> explain (costs off) select * from coercepart where a ~ any ('{ab}');
> 
> The reason for this crash is that gen_partprune_steps_internal() is
> unable to generate any steps for the clause -- which is natural, since
> the operator is not in a btree opclass.  There are various callers
> of gen_partprune_steps_internal that are aware that it could return an
> empty set of steps, but this one in match_clause_to_partition_key for
> the ScalarArrayOpExpr case was being a bit too optimistic.

Yeah, good catch!  That fixes the crash, but looking around that code a
bit, it seems that we shouldn't even have gotten up to the point you're
proposing to fix.  If saop_op is not in the partitioning opfamily, it
should have bailed out much sooner, that is, here:

    /*
     * In case of NOT IN (..), we get a '<>', which we handle if list
     * partitioning is in use and we're able to confirm that it's negator
     * is a btree equality operator belonging to the partitioning operator
     * family.
     */
    if (!op_in_opfamily(saop_op, partopfamily))
    {
        <snip>

        negator = get_negator(saop_op);
        if (OidIsValid(negator) && op_in_opfamily(negator, partopfamily))
        {
            <snip>
        }
+       else
+           /* otherwise, unsupported! */
+           return PARTCLAUSE_UNSUPPORTED;

Let me propose that we also have this along with the rest of the changes
you made in that part of the function.  So, attached is an updated patch.

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.