Horton

Re: [HACKERS] path toward faster partition pruning

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

View thread View on postgr.es
From: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
To: Robert Haas <robertmhaas@gmail.com>
Cc: 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-03-02T06:22:03Z
Lists: pgsql-hackers 
On 2018/03/01 21:56, Robert Haas wrote:
> On Tue, Feb 27, 2018 at 4:33 AM, 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].
> 
> I'm very skeptical about this patch's desire to remove the static
> qualifier from evaluate_expr().  Why does this patch need that and
> constraint exclusion not need it?  Why should this patch not instead
> by using eval_const_expressions?  partkey_datum_from_expr() is
> prepared to give up if evaluate_expr() doesn't return a Const, but
> there's nothing in evaluate_expr() to make it give up if, for example,
> the input is -- or contains -- a volatile function, e.g. random().

Thinking on this a bit, I have removed the evaluate_expr() business from
partkey_datum_from_expr() and thus switched evaluate_expr() back to static.

Let me explain why I'd added there in the first place -- if the constant
expression received in partkey_datum_from_expr() was not of the same type
as that of the partition key, it'd try to coerce_to_target_type() the
input expression to the partition key type which may result in a non-Const
expression.  We'd turn it back into a Const by calling evaluate_expr().  I
thought the coercion was needed because we'd be comparing the resulting
datum with the partition bound datums using a partition comparison
function that would require its arguments to be of given types.

But I realized we don't need the coercion.  Earlier steps would have
determined that the clause from which the expression originated contains
an operator that is compatible with the partitioning operator family.  If
so, the type of the expression in question, even though different from the
partition key type, would be binary coercible with it.  So, it'd be okay
to pass the datum extracted from such expression to the partition
comparison function to compare it with datums in PartitionBoundInfo,
without performing any coercion.

> +       if (OidIsValid(get_default_oid_from_partdesc(partdesc)))
> +               rel->has_default_part = true;
> +       else
> +               rel->has_default_part = false;
>
> This can be written a lot more compactly as rel->has_default_part =
> OidIsValid(get_default_oid_from_partdesc(partdesc));

Indeed, will fix.

> PartitionPruneContext has no comment explaining its general purpose; I
> think it should.

Will fix.

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.