Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1os9KO-0003Qi-Ao for pgsql-bugs@arkaria.postgresql.org; Mon, 07 Nov 2022 21:06:44 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1os9KN-0001QO-4l for pgsql-bugs@arkaria.postgresql.org; Mon, 07 Nov 2022 21:06:43 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1os9KM-0001QF-Sm for pgsql-bugs@lists.postgresql.org; Mon, 07 Nov 2022 21:06:42 +0000 Received: from sss.pgh.pa.us ([66.207.139.130]) by magus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1os9KH-00009J-IG for pgsql-bugs@lists.postgresql.org; Mon, 07 Nov 2022 21:06:42 +0000 Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1]) by sss.pgh.pa.us (8.15.2/8.15.2) with ESMTP id 2A7L6XDK3495355; Mon, 7 Nov 2022 16:06:33 -0500 From: Tom Lane To: Richard Guo cc: Jeff Janes , sindysenorita@gmail.com, pgsql-bugs@lists.postgresql.org Subject: Re: BUG #17618: unnecessary filter column <> text even after adding index In-reply-to: References: <17618-7a2240bfaa7e84ae@postgresql.org> <349136.1663601052@sss.pgh.pa.us> <739953.1663701274@sss.pgh.pa.us> <946195.1663718159@sss.pgh.pa.us> <2100290.1663942209@sss.pgh.pa.us> <2174559.1663977891@sss.pgh.pa.us> <2616649.1667668064@sss.pgh.pa.us> Comments: In-reply-to Richard Guo message dated "Mon, 07 Nov 2022 17:01:19 +0800" MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0" Content-ID: <3495233.1667855126.0@sss.pgh.pa.us> Date: Mon, 07 Nov 2022 16:06:33 -0500 Message-ID: <3495354.1667855193@sss.pgh.pa.us> List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk ------- =_aaaaaaaaaa0 Content-Type: text/plain; charset="us-ascii" Content-ID: <3495233.1667855126.1@sss.pgh.pa.us> Richard Guo writes: > I've updated the patch according to the suggestions as in v4. Thanks > for reviewing this patch! I was about ready to commit this when I re-read your initial comment and realized that there's a second way to fix it. We can improve predtest.c so that it understands that "x = true" implies "x" and so on, whereupon the existing logic in create_bitmap_scan_plan handles the case correctly. This is pretty nearly the same code as in your v4, except that it's in a considerably less hot code path, plus there's at least some chance that it could be useful for other purposes. So I think I like this way better. Thoughts? regards, tom lane ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name*0="v5-0001-constant-folding-for-indexquals-in-bitmap-scan.patc"; name*1="h"; charset="us-ascii" Content-ID: <3495233.1667855126.2@sss.pgh.pa.us> Content-Description: v5-0001-constant-folding-for-indexquals-in-bitmap-scan.patch Content-Transfer-Encoding: quoted-printable diff --git a/contrib/btree_gin/expected/bool.out b/contrib/btree_gin/expec= ted/bool.out index 207a3f2328..b379622baf 100644 --- a/contrib/btree_gin/expected/bool.out +++ b/contrib/btree_gin/expected/bool.out @@ -92,7 +92,7 @@ EXPLAIN (COSTS OFF) SELECT * FROM test_bool WHERE i=3Dtr= ue ORDER BY i; Sort Sort Key: i -> Bitmap Heap Scan on test_bool - Filter: i + Recheck Cond: i -> Bitmap Index Scan on idx_bool Index Cond: (i =3D true) (6 rows) @@ -119,3 +119,15 @@ EXPLAIN (COSTS OFF) SELECT * FROM test_bool WHERE i>t= rue ORDER BY i; Index Cond: (i > true) (6 rows) = +-- probably sufficient to check just this one: +EXPLAIN (COSTS OFF) SELECT * FROM test_bool WHERE i=3Dfalse ORDER BY i; + QUERY PLAN = +------------------------------------------- + Sort + Sort Key: i + -> Bitmap Heap Scan on test_bool + Recheck Cond: (NOT i) + -> Bitmap Index Scan on idx_bool + Index Cond: (i =3D false) +(6 rows) + diff --git a/contrib/btree_gin/sql/bool.sql b/contrib/btree_gin/sql/bool.s= ql index dad2ff32b8..08f2986e8c 100644 --- a/contrib/btree_gin/sql/bool.sql +++ b/contrib/btree_gin/sql/bool.sql @@ -25,3 +25,5 @@ EXPLAIN (COSTS OFF) SELECT * FROM test_bool WHERE i<=3Dt= rue ORDER BY i; EXPLAIN (COSTS OFF) SELECT * FROM test_bool WHERE i=3Dtrue ORDER BY i; EXPLAIN (COSTS OFF) SELECT * FROM test_bool WHERE i>=3Dtrue ORDER BY i; EXPLAIN (COSTS OFF) SELECT * FROM test_bool WHERE i>true ORDER BY i; +-- probably sufficient to check just this one: +EXPLAIN (COSTS OFF) SELECT * FROM test_bool WHERE i=3Dfalse ORDER BY i; diff --git a/src/backend/optimizer/util/predtest.c b/src/backend/optimizer= /util/predtest.c index 182d5b1523..90701dab08 100644 --- a/src/backend/optimizer/util/predtest.c +++ b/src/backend/optimizer/util/predtest.c @@ -15,6 +15,7 @@ */ #include "postgres.h" = +#include "catalog/pg_operator.h" #include "catalog/pg_proc.h" #include "catalog/pg_type.h" #include "executor/executor.h" @@ -1093,7 +1094,7 @@ arrayexpr_cleanup_fn(PredIterInfo info) * * We return true if able to prove the implication, false if not. * - * We have three strategies for determining whether one simple clause + * We have several strategies for determining whether one simple clause * implies another: * * A simple and general way is to see if they are equal(); this works for= any @@ -1101,6 +1102,12 @@ arrayexpr_cleanup_fn(PredIterInfo info) * there is an implied assumption that the functions in the expression ar= e * immutable --- but this was checked for the predicate by the caller.) * + * Another way that always works is that "x =3D TRUE" implies "x" (and vi= ce + * versa), likewise for "x =3D FALSE" and "NOT x". These can be worth + * checking because, while we preferentially simplify boolean comparisons + * down to "x" and "NOT x", the other form has to be dealt with anyway + * in the context of index conditions. + * * If the predicate is of the form "foo IS NOT NULL", and we are consider= ing * strong implication, we can conclude that the predicate is implied if t= he * clause is strict for "foo", i.e., it must yield false or NULL when "fo= o" @@ -1124,6 +1131,43 @@ predicate_implied_by_simple_clause(Expr *predicate,= Node *clause, if (equal((Node *) predicate, clause)) return true; = + /* Next see if clause is boolean equality to a constant */ + if (is_opclause(clause) && + ((OpExpr *) clause)->opno =3D=3D BooleanEqualOperator) + { + OpExpr *op =3D (OpExpr *) clause; + Node *rightop; + + Assert(list_length(op->args) =3D=3D 2); + rightop =3D lsecond(op->args); + /* We might never see a null Const here, but better check anyway */ + if (rightop && IsA(rightop, Const) && + !((Const *) rightop)->constisnull) + { + Node *leftop =3D linitial(op->args); + + if (DatumGetBool(((Const *) rightop)->constvalue)) + { + /* X =3D true implies X */ + if (equal(predicate, leftop)) + return true; + } + else + { + /* X =3D false implies NOT X */ + if (is_notclause(predicate) && + equal(get_notclausearg(predicate), leftop)) + return true; + } + } + } + + /* + * We could likewise check whether the predicate is boolean equality to = a + * constant; but there are no known use-cases for that at the moment, + * assuming that the predicate has been through const-folding. + */ + /* Next try the IS NOT NULL case */ if (!weak && predicate && IsA(predicate, NullTest)) ------- =_aaaaaaaaaa0--