Re: default range partition and constraint exclusion

At Fri, 24 Nov 2017 10:49:07 -0500, Robert Haas <robertmhaas(at)gmail(dot)com> wrote in <CA+TgmoarK4aCcSjYheH7QDchb7uJRpiKkGpPo7O9kMBNf13N3w(at)mail(dot)gmail(dot)com>
> On Wed, Nov 22, 2017 at 4:21 AM, Amit Langote
> <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> >>> If all predicate_refuted_by() receives is the expression tree (AND/OR)
> >>> with individual nodes being strict clauses involving partition keys (and
> >>> nothing about the nullness of the keys), the downstream code is just
> >>> playing by the rules as explained in the header comment of
> >>> predicate_refuted_by_recurse() in concluding that query's restriction
> >>> clause a = 2 refutes it.
> >>
> >> Oh, wait a minute. Actually, I think predicate_refuted_by() is doing
> >> the right thing here. Isn't the problem that mc2p2 shouldn't be
> >> accepting a (2, null) tuple at all?
> >
> > It doesn't. But, for a query, it does contain (2, <unknown>) tuples,
> > where <unknown> would always be non-null. So, it should be scanned in the
> > plan for the query that has only a = 2 as restriction and no restriction
> > on b. That seems to work.
>
> OK, so I am still confused about whether the constraint is wrong or
> the constraint exclusion logic is wrong. One of them, at least, has
> to be wrong, and we have to fix whichever one is wrong. Fixing broken
> constraint exclusion logic by hacking up the constraint, or conversely
> fixing a broken constraint by hacking up the constraint exclusion
> logic, wouldn't be right.
>
> I think my last email was confused: I thought that the (2, null) tuple
> was ending up in mc2p2, but it's really ending up in mc2p_default,
> whose constraint currently looks like this:
>
> NOT (
> ((a < 1) OR ((a = 1) AND (b < 1)))
> OR
> ((a > 1) OR ((a = 1) AND (b >= 1)))
> )
>
> Now where exactly is constraint exclusion going wrong here? a = 2
> refutes a < 1 and a = 1, which means that (a < 1) OR ((a = 1) AND (b <
> 1)) must be false and that (a = 1) AND (b >= 1) must also be false.
> But (a > 1) could be either true or null, which means (a > 1) OR ((a =

a > 1 is true when a = 2, so the second term is true?

> 1) AND (b >= 1)) can be true or null, which means the whole thing can
> be false or null, which means that it is not refuted by a = 2. It

Then the whole thing is false.

> should be possible to dig down in there step by step and figure out
> where the wheels are coming off -- have you tried to do that?

| select NOT (
| ((a < 1) OR ((a = 1) AND (b < 1)))
| OR
| ((a > 1) OR ((a = 1) AND (b >= 1)))
| )
| from (values (2::int, null::int)) as t(a, b);
| ?column?
| ----------
| f

The problem here I think is that get_qual_for_range() for default
partition returns an inconsistent qual with what partition
get_partition_for_tuple chooses for keys containing nulls. It
chooses default partition if any of the key values is null,
without referring the constraint expression.

The current behavior is apparently odd.

| select pg_get_partition_constraintdef('mc2p2'::regclass);
| pg_get_partition_constraintdef
| ----------------------------------------------------------------------------
| ((a IS NOT NULL) AND (b IS NOT NULL) AND ((a > 1) OR ((a = 1) AND (b >= 1))))

| select pg_get_partition_constraintdef('mc2p_default'::regclass);
| pg_get_partition_constraintdef
|
| ---------------------------------------------------------------------------
| (NOT (((a < 1) OR ((a = 1) AND (b < 1))) OR ((a > 1) OR ((a = 1) AND (b >= 1)))))

| insert into mc2p2 values (2);
| ERROR: new row for relation "mc2p2" violates partition constraint
| DETAIL: Failing row contains (2, null).

This is the correct behavior.

| insert into mc2p_default values (2);
| ERROR: new row for relation "mc2p_default" violates partition constraint
| DETAIL: Failing row contains (2, null).

This is the correct behavior in terms of constraint, but
incorrect in terms of partition routing.

But interestingly, the following *works*, in a way contradicting
to the constraint.

| insert into mc2p values (2);
| INSERT 0 1
|
| select * from mc2p_default;
| a | b
| ---+---
| 2 |
| (1 row)

After applying the patch upthread, get_qual_for_range() returns
the consistent qual and "insert into mc2p_default values (2)" is
accepted correctly and everything become consistent.

This is the story in my understanding.

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center