Re: multi-column range partition constraint

On Tue, May 2, 2017 at 2:51 AM, 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))
>
> 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.
>
> Adding to the open items list.

I think there are more problems here. With the patch:

rhaas=# create table p (a int, b int) partition by range (a, b);
CREATE TABLE
rhaas=# create table p1 partition of p for values from (unbounded,0)
to (unbounded,1);
CREATE TABLE
rhaas=# insert into p1 values (-2,-2);
ERROR: new row for relation "p1" violates partition constraint
DETAIL: Failing row contains (-2, -2).
rhaas=# insert into p1 values (2,2);
ERROR: new row for relation "p1" violates partition constraint
DETAIL: Failing row contains (2, 2).

Really, the whole CREATE TABLE .. PARTITION statement is meaningless
and should be disallowed, because it's not meaningful to have a
partition bound specification with a non-unbounded value following an
unbounded value.

BTW, I think we should also add a function that prints the partition
constraint, and have psql display that in the \d+ output, because
people might need that - e.g. if you want to attach a partition
without having to validate it, you need to be able to apply an
appropriate constraint to it in advance, so you'll want to see what
the existing partition constraints look like.

While I'm glad we have partitioning has a feature, I'm starting to get
a bit depressed by the number of bugs that are turning up here. This
was committed in early December, and ideally ought to have been stable
long before now.

Since Amit is back from vacation May 8th, I'll update no later than May 9th.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company