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)

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

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

Thanks,
Amit

[1]
https://www.postgresql.org/message-id/64241fee-09af-fe4b-a0ab-7cd739965041%40lab.ntt.co.jp
From 4e3408541da4b67c37ee5dc733c807c0708e1ba7 Mon Sep 17 00:00:00 2001
From: amit <amitlangot...@gmail.com>
Date: Thu, 1 Feb 2018 11:32:23 +0900
Subject: [PATCH v1] Disregard nulls in SAOP rightarg array/list during CE

---
 src/backend/optimizer/util/predtest.c | 6 ++++++
 src/test/regress/expected/inherit.out | 6 +-----
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/src/backend/optimizer/util/predtest.c 
b/src/backend/optimizer/util/predtest.c
index 8106010329..0928306c62 100644
--- a/src/backend/optimizer/util/predtest.c
+++ b/src/backend/optimizer/util/predtest.c
@@ -968,6 +968,9 @@ arrayconst_next_fn(PredIterInfo info)
 
        if (state->next_elem >= state->num_elems)
                return NULL;
+       /* skip nulls */
+       while (state->elem_nulls[state->next_elem])
+               state->next_elem++;
        state->constexpr.constvalue = state->elem_values[state->next_elem];
        state->constexpr.constisnull = state->elem_nulls[state->next_elem];
        state->next_elem++;
@@ -1028,6 +1031,9 @@ arrayexpr_next_fn(PredIterInfo info)
 
        if (state->next == NULL)
                return NULL;
+       /* skip nulls */
+       while (IsA(state->next, Const) && ((Const *) state->next)->constisnull)
+               state->next = lnext(state->next);
        lsecond(state->opexpr.args) = lfirst(state->next);
        state->next = lnext(state->next);
        return (Node *) &(state->opexpr);
diff --git a/src/test/regress/expected/inherit.out 
b/src/test/regress/expected/inherit.out
index a79f891da7..d56e5d25d9 100644
--- a/src/test/regress/expected/inherit.out
+++ b/src/test/regress/expected/inherit.out
@@ -1715,11 +1715,7 @@ explain (costs off) select * from list_parted where a = 
'ab' or a in (null, 'cd'
  Append
    ->  Seq Scan on part_ab_cd
          Filter: (((a)::text = 'ab'::text) OR ((a)::text = ANY 
('{NULL,cd}'::text[])))
-   ->  Seq Scan on part_ef_gh
-         Filter: (((a)::text = 'ab'::text) OR ((a)::text = ANY 
('{NULL,cd}'::text[])))
-   ->  Seq Scan on part_null_xy
-         Filter: (((a)::text = 'ab'::text) OR ((a)::text = ANY 
('{NULL,cd}'::text[])))
-(7 rows)
+(3 rows)
 
 explain (costs off) select * from list_parted where a = 'ab';
                 QUERY PLAN                
-- 
2.11.0

Reply via email to