Re: path toward faster partition pruning

From: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
To: Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp
Cc: sulamul(at)gmail(dot)com, david(dot)rowley(at)2ndquadrant(dot)com, robertmhaas(at)gmail(dot)com, dilipbalaut(at)gmail(dot)com, rajkumar(dot)raghuwanshi(at)enterprisedb(dot)com, memissemerson(at)gmail(dot)com, pgsql-hackers(at)postgresql(dot)org
Subject: Re: path toward faster partition pruning
Date: 2017-11-10 05:38:11
Message-ID: 20171110.143811.97616847.horiguchi.kyotaro@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hello, this is the second part of the review.

At Fri, 10 Nov 2017 12:30:00 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> wrote in <20171110(dot)123000(dot)151902771(dot)horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
> In 0002, bms_add_range has a bit naive-looking loop
> In 0003,

In 0004,

The name get_partitions_from_clauses_guts(), it seems to me that
we usually use _internal for recursive part of some function. (I
have the same comment as David about the comment for
get_partition_from_clause())

About the same function:

Couldn't we get out in the fast path when clauses == NIL?

+ /* No constraints on the keys, so, return *all* partitions. */
+ result = bms_add_range(result, 0, partdesc->nparts - 1);

This allows us to return immediately here. And just above this,

+ if (nkeys > 0 && !constfalse)
+ result = get_partitions_for_keys(relation, &keys);
+ else if (!constfalse)

Those two conditions are not orthogonal. Maybe something like
following seems more understantable.

> if (!constfalse)
> {
> /* No constraints on the keys, so, return *all* partitions. */
> if (nkeys == 0)
> return bms_add_range(result, 0, partdesc->nparts - 1);
>
> result = get_partitions_for_keys(relation, &keys);
> }

I'm not sure what is meant to be (formally) done here, but it
seems to me that OrExpr is assumed to be only at the top level of
the caluses. So the following (just an example, but meaningful
expression in this shpape must exists.) expression is perhaps
wrongly processed here.

CREATE TABLE p (a int) PARITION BY (a);
CREATE TABLE c1 PARTITION OF p FOR VALUES FROM (0) TO (10);
CREATE TABLE c2 PARTITION OF p FOR VALUES FROM (10) TO (20);

SELECT * FROM p WHERE a = 15 AND (a = 15 OR a = 5);

get_partitions_for_keys() returns both c1 and c2 and still
or_clauses here holds (a = 15 OR a = 5) and the function tries to
*add* partitions for a = 15 and a = 5 separetely.

I'd like to pause here.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kyotaro HORIGUCHI 2017-11-10 05:44:55 Re: path toward faster partition pruning
Previous Message amul sul 2017-11-10 05:38:05 Re: [POC] hash partitioning