Hello, this is the second part of the review.
At Fri, 10 Nov 2017 12:30:00 +0900 (Tokyo Standard Time), Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp> wrote in
<20171110.123000.151902771.horiguchi.kyotaro@lab.ntt.co.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
