Re: default range partition and constraint exclusion

Horiguchi-san, thanks for the clarifying comment.

On 2017/11/27 18:04, Kyotaro HORIGUCHI wrote:
> At Fri, 24 Nov 2017 10:49:07 -0500, Robert Haas wrote
>> 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?

Yes.

>> 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.

Yes, too.

>> 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.

Right.

> 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.

Yes, a non-default range partition does not accept nulls in any of the
partition keys.

> | 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)

That is, the default partition's constraint, as currently generated, is wrong.

> 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.

Thanks for testing.

Thanks,
Amit