From: | Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> |
---|---|
To: | Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com> |
Cc: | Pg Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: constraint exclusion and nulls in IN (..) clause |
Date: | 2018-02-01 08:53:31 |
Message-ID: | 0cdf4876-13e5-5a59-cde6-11295f2f12aa@lab.ntt.co.jp |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Thanks for the comments.
On 2018/02/01 16:40, Ashutosh Bapat wrote:
> On Thu, Feb 1, 2018 at 12:26 PM, Amit Langote
> <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>> Hi.
>>
>> When addressing a review comment on the fast partition pruning thread [1],
>> I noticed that specifying null in the IN-list will cause constraint
>> exclusion to wrongly fail to refute a table's check predicate.
>>
>> create table foo (a int check (a = 1));
>> insert into foo values (null), (1);
>>
>> -- ExecEvalScalarArrayOp() won't return true for any record in foo
>> select * from foo where a in (null, 2);
>> a
>> ---
>> (0 rows)
>
> AFAIU, that's true only when = operator is strict. For a non-strict
> operator which treats two NULL values as equivalent it would return
> row with NULL value.
That's true.
>> -- The null in the IN-list prevents constraint exclusion to exclude foo
>> -- from being scanned in the first place
>> explain (costs off) select * from foo where a in (null, 2);
>> QUERY PLAN
>> ---------------------------------------------
>> Seq Scan on foo
>> Filter: (a = ANY ('{NULL,2}'::integer[]))
>> (2 rows)
>>
>> I propose a patch that teaches predtest.c to disregard any null values in
>> a SAOP (i.e., the IN (..) expression) when performing constraint exclusion
>> using that SAOP, because they cause predicate_refuted_by_recurse()'s logic
>> to fail to conclude the refutation. There is a rule that all items of an
>> OR clause (SAOP is treated as one) must refute the predicate. But the
>> OpExpr constructed with null as its constant argument won't refute
>> anything and thus will cause the whole OR clause to fail to refute the
>> predicate.
>
> A cursory look through constraint exclusion code esp.
> operator_predicate_proof() doesn't show any special handling for
> strict/non-strict operators. Probably that's why that function refuses
> to refute/imply anything when it encounters NULLs.
> 1593 * We have two identical subexpressions, and two other
> subexpressions that
> 1594 * are not identical but are both Consts; and we have commuted the
> 1595 * operators if necessary so that the Consts are on the
> right. We'll need
> 1596 * to compare the Consts' values. If either is NULL, fail.
> 1597 */
> 1598 if (pred_const->constisnull)
> 1599 return false;
> 1600 if (clause_const->constisnull)
> 1601 return false;
There does actually exist strictness check in
predicate_refuted_by_simple_clause(), but comes into play only if the
predicate is a IS NULL clause. In our case, both predicate and clause
expressions would be operator clauses for which the strictness doesn't
seem to be considered.
>> -- With the patch
>> explain (costs off) select * from foo where a in (null, 2);
>> QUERY PLAN
>> --------------------------
>> Result
>> One-Time Filter: false
>> (2 rows)
>>
>> explain (costs off) select * from foo where a in (null, 2, 1);
>> QUERY PLAN
>> -----------------------------------------------
>> Seq Scan on foo
>> Filter: (a = ANY ('{NULL,2,1}'::integer[]))
>> (2 rows)
>>
>> Thoughts?
>
> AFAIU, this doesn't look to be the right way to fix the problem; it
> assumes that the operators are strict. Sorry, if I have misunderstood
> the patch and your thoughts behind it. May be constraint exclusion
> code should be taught to treat strict/non-strict operators separately.
> I am not sure.
Yeah, the patch in its current form is wrong, because it will give wrong
answers if the operator being used in a SAOP is non-strict. I modified
the patch to consider operator strictness before doing anything with nulls.
Thanks,
Amit
Attachment | Content-Type | Size |
---|---|---|
v2-0001-Disregard-nulls-in-SAOP-rightarg-array-list-durin.patch | text/plain | 3.6 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Simon Riggs | 2018-02-01 09:32:02 | Re: [HACKERS] Surjective functional indexes |
Previous Message | Konstantin Knizhnik | 2018-02-01 08:49:24 | Re: [HACKERS] Surjective functional indexes |