Re: constraint exclusion and nulls in IN (..) clause - Mailing list pgsql-hackers
From | Amit Langote |
---|---|
Subject | Re: constraint exclusion and nulls in IN (..) clause |
Date | |
Msg-id | 0cdf4876-13e5-5a59-cde6-11295f2f12aa@lab.ntt.co.jp Whole thread Raw |
In response to | Re: constraint exclusion and nulls in IN (..) clause (Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>) |
Responses |
Re: constraint exclusion and nulls in IN (..) clause
Re: constraint exclusion and nulls in IN (..) clause |
List | 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@lab.ntt.co.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
pgsql-hackers by date: