Horton

Re: 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: Dilip Kumar <dilipbalaut@gmail.com>
Cc: Rajkumar Raghuwanshi <rajkumar.raghuwanshi@enterprisedb.com>, Beena Emerson <memissemerson@gmail.com>, David Rowley <david.rowley@2ndquadrant.com>, Pg Hackers <pgsql-hackers@postgresql.org>
Date: 2017-10-31T08:43:51Z
Lists: pgsql-hackers

Attachments

Thanks Dilip for reviewing.

On 2017/10/31 1:50, Dilip Kumar wrote:
> On Mon, Oct 30, 2017 at 12:20 PM, Amit Langote
> <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>> On 2017/10/30 14:55, Amit Langote wrote:
>>> Fixed in the attached updated patch, along with a new test in 0001 to
>>> cover this case.  Also, made a few tweaks to 0003 and 0005 (moved some
>>> code from the former to the latter) around the handling of ScalarArrayOpExprs.
>>
>> Sorry, I'd forgotten to include some changes.
>>
>> In the previous versions, RT index of the table needed to be passed to
>> partition.c, which I realized is no longer needed, so I removed that
>> requirement from the interface.  As a result, patches 0002 and 0003 have
>> changed in this version.
> 
> Some Minor comments:
> 
> + * get_rel_partitions
> + * Return the list of partitions of rel that pass the clauses mentioned
> + * rel->baserestrictinfo
> + *
> + * Returned list contains the AppendRelInfos of chosen partitions.
> + */
> +static List *
> +get_append_rel_partitions(PlannerInfo *root,
> 
> Function name in function header is not correct.

Fixed.

> + !DatumGetBool(((Const *) clause)->constvalue))
> + {
> + *constfalse = true;
> + continue;
> 
> DatumGetBool will return true if the first byte of constvalue is
> nonzero otherwise
> false.  IIUC, this is not the intention here. Or I am missing something?

This coding pattern is in use in quite a few places; see for example in
restriction_is_constant_false() and many others like
relation_excluded_by_constraints(), negate_clause(), etc.

If a RestrictInfo is marked pseudoconstant=true, then the clause therein
must be a Const with constvalue computing to 0 if the clause is false, so
that DatumGetBool(constvalue) returns boolean false and non-zero otherwise.

> + * clauses in this function ourselves, for example, having both a > 1 and
> + * a = 0 the list
> 
> a = 0 the list -> a = 0 in the list

Right, fixed.

> 
> +static bool
> +partkey_datum_from_expr(const Expr *expr, Datum *value)
> +{
> + /*
> + * Add more expression types here as needed to support higher-level
> + * code.
> + */
> + switch (nodeTag(expr))
> + {
> + case T_Const:
> + *value = ((Const *) expr)->constvalue;
> + return true;
> 
> I think for evaluating other expressions (e.g. T_Param) we will have
> to pass ExprContext to this function.

That's right.

> Or we can do something cleaner
> because if we want to access the ExprContext inside
> partkey_datum_from_expr then we may need to pass it to
> "get_partitions_from_clauses" which is a common function for optimizer
> and executor.

Yeah, I've thought about that a little.  Since nothing else but the
planner calls it now and the planner doesn't itself have its hands on the
ExprContext that would be necessary for computing something like Params, I
left it out of the interface for now.  That said, I *am* actually thinking
about some interface changes that would be necessary for some other
unrelated functionality/optimizations that callers like the run-time
pruning code would expect of get_partitions_from_clauses().  We can design
the interface extension such that the aforementioned ExprContext is passed
together.


Attached updated version of the patches addressing some of your comments
above and fixing a bug that Rajkumar reported [1].  As mentioned there,
I'm including here a patch (the 0005 of the attached) to tweak the default
range partition constraint to be explicit about null values that it might
contain.  So, there are 6 patches now and what used to be patch 0005 in
the previous set is patch 0006 in this version of the set.

Thanks,
Amit

[1]
https://www.postgresql.org/message-id/cd5a2d2e-0957-042c-40c2-06033fe0abf2@lab.ntt.co.jp

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.