Re: multi-column range partition constraint

Hi Beena,

On 2017/05/02 17:47, Beena Emerson wrote:
> Hello Amit,
>
> On Tue, May 2, 2017 at 12:21 PM, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp
>> wrote:
>
>> Per an off-list report from Olaf Gawenda (thanks Olaf), it seems that the
>> range partition's constraint is sometimes incorrect, at least in the case
>> of multi-column range partitioning. See below:
>>
>> create table p (a int, b int) partition by range (a, b);
>> create table p1 partition of p for values from (1, 1) to (10 ,10);
>> create table p2 partition of p for values from (11, 1) to (20, 10);
>>
>> Perhaps unusual, but it's still a valid definition. Tuple-routing puts
>> rows where they belong correctly.
>>
>> -- ok
>> insert into p values (10, 9);
>> select tableoid::regclass, * from p;
>> tableoid | a | b
>> ----------+----+---
>> p1 | 10 | 9
>> (1 row)
>>
>> -- but see this
>> select tableoid::regclass, * from p where a = 10;
>> tableoid | a | b
>> ----------+---+---
>> (0 rows)
>>
>> explain select tableoid::regclass, * from p where a = 10;
>> QUERY PLAN
>> -------------------------------------------
>> Result (cost=0.00..0.00 rows=0 width=12)
>> One-Time Filter: false
>> (2 rows)
>>
>> -- or this
>> insert into p1 values (10, 9);
>> ERROR: new row for relation "p1" violates partition constraint
>> DETAIL: Failing row contains (10, 9).
>>
>> This is because of the constraint being generated is not correct in this
>> case. p1's constraint is currently:
>>
>> a >= 1 and a < 10
>>
>> where it should really be the following:
>>
>> (a > 1 OR (a = 1 AND b >= 1))
>> AND
>> (a < 10 OR (a = 10 AND b < 10))
>>
>
>
> IIUC, when we say range 1 to 10 we allow values from 1 to 9. Here we are
> allowing a=10 be stored in p1 Is it okay?

Yes it is. Consider that the multi-column range partitioning uses
tuple-comparison logic to determine if a row's partition key is >=
lower_bound and < upper_bound tuple.

In this case, p1's lower bound is a "tuple" (1, 1) and (10, 10) its upper
bound. Consider an input row with (2, -1) as its partition key.
Tuple-comparison logic puts this row into p1, because:

select (1, 1) <= (2, -1) and (2, -1) < (10, 10);
?column?
----------
t
(1 row)

When performing tuple-comparison with the lower bound, since 2 > 1, the
tuple comparison is done and the whole tuple is concluded to be > (1, 1);
no attention is paid to the second column (or to the fact that -1 not >= 1).

Now consider an input row with (10, 9) as its partition key, which too
fits in p1, because:

select (1, 1) <= (10, 9) and (10, 9) < (10, 10);
?column?
----------
t
(1 row)

In this case, since 10 = 10, tuple-comparison considers the next column to
determine the result. In this case, since the second column 9 < 10, the
whole tuple is concluded to be < (10, 10).

However, neither of (1, 0), (10, 10), or (10, 11) is admissible into p1 by
the above comparison logic:

select (1, 1) <= (1, 0) and (1, 0) < (10, 10);
?column?
----------
f
(1 row)

select (1, 1) <= (10, 10) and (10, 10) < (10, 10);
?column?
----------
f
(1 row)

select (1, 1) <= (10, 11) and (10, 11) < (10, 10);
?column?
----------
f
(1 row)

> I havent been following these partition mails much. Sorry if I am missing
> something obvious.

No problem.

>> Attached patch rewrites get_qual_for_range() for the same, along with some
>> code rearrangement for reuse. I also added some new tests to insert.sql
>> and inherit.sql, but wondered (maybe, too late now) whether there should
>> really be a declarative_partition.sql for these, moving in some of the old
>> tests too.
>>
> I got the following warning on compiling:
> partition.c: In function ‘make_partition_op_expr’:
> partition.c:1267:2: warning: ‘result’ may be used uninitialized in this
> function [-Wmaybe-uninitialized]
> return result;

Oops, fixed. Updated patch attached.

Thanks,
Amit