From: Richard Guo <guofenglinux@gmail.com>
To: David Rowley <dgrowleyml@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: william.duclot@gmail.com, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Subject: Re: BUG #17540: Prepared statement: PG switches to a generic query plan which is consistently much slower
Date: Mon, 10 Jul 2023 14:39:27 +0800
Message-ID: <CAMbWs4-cWwe0whACTuKTLT+4jNcSJ+RLEcSp4us1vBAt0k7GAA@mail.gmail.com> (raw)
In-Reply-To: <CAApHDvr+70jEZpraMZ8sj=UH77dBzCaeUc9EcJHB3YaxX9syNg@mail.gmail.com>
References: <17540-7aa1855ad5ec18b4@postgresql.org>
<CAApHDvqRR81Dvur_zcrY+RsKxv_e8uDpJ2xxAPMmnfgDY7VNew@mail.gmail.com>
<2732256.1657163219@sss.pgh.pa.us>
<CAApHDvqhr1eU9v42w5gt_gL=G15pB9RBpAH_Jmg7eDU6iZ_2zg@mail.gmail.com>
<2740652.1657165848@sss.pgh.pa.us>
<CAApHDvqHCNKJi9CrQZG-reQDXTfRWnT5rhzNtDQhnrBzAAusfA@mail.gmail.com>
<CAMbWs48fiio5RcsBHztjNXXYvU4QKTekbszECs0ydpY2faqo9A@mail.gmail.com>
<CAMbWs49E2Ps7xdPNTAQOuDg6B9RAidcgQ2S4_nEdveSg_GqiDg@mail.gmail.com>
<CAApHDvr+70jEZpraMZ8sj=UH77dBzCaeUc9EcJHB3YaxX9syNg@mail.gmail.com>
On Mon, Jul 10, 2023 at 10:14 AM David Rowley <dgrowleyml@gmail.com> wrote:
> On Fri, 7 Jul 2023 at 19:03, Richard Guo <guofenglinux@gmail.com> wrote:
> > Attached is what I have in mind. The patch extends the logic from two
> > points.
> >
> > * it also checks OR clauses to see if it is always true.
> >
> > * it also checks for join clauses by additionally testing if the nulling
> > bitmap is empty.
>
> Do you mind writing some regression tests for this?
>
> I don't really see an existing test file that would suit, maybe it's
> worth adding something like predicate.sql
Here is v3 patch with regression tests. I add the new test into the
group where stats test is in, but I'm not sure if this is the right
place.
Thanks
Richard
Attachments:v3-0001-ignore-is-not-null-quals-on-not-null-columns.patch (application/octet-stream, 16.6K) download
From a45cdbe1d440ab2642c8cc312dd609b4b9939ca8 Mon Sep 17 00:00:00 2001
From: Richard Guo <guofenglinux@gmail.com>
Date: Mon, 10 Jul 2023 14:24:42 +0800
Subject: [PATCH v3] ignore is not null quals on not null columns
---
src/backend/optimizer/plan/initsplan.c | 179 +++++++++++++++++-------
src/backend/optimizer/util/joininfo.c | 5 +
src/backend/optimizer/util/plancat.c | 9 ++
src/backend/optimizer/util/relnode.c | 2 +
src/include/nodes/pathnodes.h | 2 +
src/include/optimizer/planmain.h | 2 +
src/test/regress/expected/predicate.out | 107 ++++++++++++++
src/test/regress/parallel_schedule | 2 +-
src/test/regress/sql/predicate.sql | 48 +++++++
9 files changed, 308 insertions(+), 48 deletions(-)
create mode 100644 src/test/regress/expected/predicate.out
create mode 100644 src/test/regress/sql/predicate.sql
diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.cindex b31d892121..980429d469 100644--- a/src/backend/optimizer/plan/initsplan.c+++ b/src/backend/optimizer/plan/initsplan.c@@ -2618,6 +2618,110 @@ check_redundant_nullability_qual(PlannerInfo *root, Node *clause)
return false;
}
+/*+ * add_baserestrictinfo_to_rel+ * Add 'restrictinfo' as a baserestrictinfo to the base relation denoted by+ * 'relid' with some prechecks to try to determine if the qual is always+ * true, in which case we ignore it rather than add it.+ */+static void+add_baserestrictinfo_to_rel(PlannerInfo *root, Index relid,+ RestrictInfo *restrictinfo)+{+ RelOptInfo *rel = find_base_rel(root, relid);++ Assert(bms_membership(restrictinfo->required_relids) == BMS_SINGLETON);++ /* Don't add the clause if it is always true */+ if (restriction_is_always_true(root, restrictinfo))+ return;++ /* Add clause to rel's restriction list */+ rel->baserestrictinfo = lappend(rel->baserestrictinfo, restrictinfo);++ /* Update security level info */+ rel->baserestrict_min_security = Min(rel->baserestrict_min_security,+ restrictinfo->security_level);+}++/*+ * nulltest_is_always_true+ * Check to see if the NullTest qual is always true.+ *+ * If the NullTest is an IS_NOT_NULL qual on a NOT NULL column, and meanwhile+ * its Var is not nulled by any outer joins, then we can know that it would be+ * always true.+ */+static bool+nulltest_is_always_true(PlannerInfo *root, NullTest *nulltest)+{+ RelOptInfo *rel;+ Var *var;++ Assert(IsA(nulltest, NullTest));++ /* is this NullTest an IS_NOT_NULL qual? */+ if (nulltest->nulltesttype != IS_NOT_NULL)+ return false;++ if (!IsA(nulltest->arg, Var))+ return false;++ var = (Var *) nulltest->arg;++ /* could the Var be nulled by any outer joins? */+ if (!bms_is_empty(var->varnullingrels))+ return false;++ /* system columns cannot be NULL */+ if (var->varattno < 0)+ return true;++ /* is the column defined NOT NULL? */+ rel = find_base_rel(root, var->varno);+ if (var->varattno > 0 &&+ bms_is_member(var->varattno, rel->notnullattnums))+ return true;++ return false;+}++/*+ * restriction_is_always_true+ * Check to see if the RestrictInfo is always true.+ *+ * Currently we only check for NullTest quals and OR clauses that include+ * NullTest quals. We may extend it in the future.+ */+bool+restriction_is_always_true(PlannerInfo *root,+ RestrictInfo *restrictinfo)+{+ /* Check for NullTest qual */+ if (IsA(restrictinfo->clause, NullTest))+ return nulltest_is_always_true(root, (NullTest *) restrictinfo->clause);++ /* If it's an OR, check its sub-clauses */+ if (restriction_is_or_clause(restrictinfo))+ {+ ListCell *lc;++ Assert(is_orclause(restrictinfo->orclause));+ foreach(lc, ((BoolExpr *) restrictinfo->orclause)->args)+ {+ Node *orarg = (Node *) lfirst(lc);++ if (!IsA(orarg, RestrictInfo))+ continue;++ if (restriction_is_always_true(root, (RestrictInfo *) orarg))+ return true;+ }+ }++ return false;+}+
/*
* distribute_restrictinfo_to_rels
* Push a completed RestrictInfo into the proper restriction or join
@@ -2632,58 +2736,39 @@ distribute_restrictinfo_to_rels(PlannerInfo *root,
RestrictInfo *restrictinfo)
{
Relids relids = restrictinfo->required_relids;
- RelOptInfo *rel;+ int relid;- switch (bms_membership(relids))+ if (relids == NULL)
{
- case BMS_SINGLETON:-- /*- * There is only one relation participating in the clause, so it- * is a restriction clause for that relation.- */- rel = find_base_rel(root, bms_singleton_member(relids));-- /* Add clause to rel's restriction list */- rel->baserestrictinfo = lappend(rel->baserestrictinfo,- restrictinfo);- /* Update security level info */- rel->baserestrict_min_security = Min(rel->baserestrict_min_security,- restrictinfo->security_level);- break;- case BMS_MULTIPLE:-- /*- * The clause is a join clause, since there is more than one rel- * in its relid set.- */-- /*- * Check for hashjoinable operators. (We don't bother setting the- * hashjoin info except in true join clauses.)- */- check_hashjoinable(restrictinfo);+ /*+ * clause references no rels, and therefore we have no place to+ * attach it. Shouldn't get here if callers are working properly.+ */+ elog(ERROR, "cannot cope with variable-free clause");+ }+ else if (bms_get_singleton_member(relids, &relid))+ add_baserestrictinfo_to_rel(root, relid, restrictinfo);+ else+ {+ /*+ * The clause is a join clause, since there is more than one rel in+ * its relid set.+ */- /*- * Likewise, check if the clause is suitable to be used with a- * Memoize node to cache inner tuples during a parameterized- * nested loop.- */- check_memoizable(restrictinfo);+ /*+ * Check for hashjoinable operators. (We don't bother setting the+ * hashjoin info except in true join clauses.)+ */+ check_hashjoinable(restrictinfo);- /*- * Add clause to the join lists of all the relevant relations.- */- add_join_clause_to_rels(root, restrictinfo, relids);- break;- default:+ /*+ * Likewise, check if the clause is suitable to be used with a Memoize+ * node to cache inner tuples during a parameterized nested loop.+ */+ check_memoizable(restrictinfo);- /*- * clause references no rels, and therefore we have no place to- * attach it. Shouldn't get here if callers are working properly.- */- elog(ERROR, "cannot cope with variable-free clause");- break;+ /* Add clause to the join lists of all the relevant relations. */+ add_join_clause_to_rels(root, restrictinfo, relids);
}
}
diff --git a/src/backend/optimizer/util/joininfo.c b/src/backend/optimizer/util/joininfo.cindex 968a5a488e..add124e395 100644--- a/src/backend/optimizer/util/joininfo.c+++ b/src/backend/optimizer/util/joininfo.c@@ -17,6 +17,7 @@
#include "optimizer/joininfo.h"
#include "optimizer/pathnode.h"
#include "optimizer/paths.h"
+#include "optimizer/planmain.h"
/*
@@ -98,6 +99,10 @@ add_join_clause_to_rels(PlannerInfo *root,
{
int cur_relid;
+ /* Don't add the clause if it is always true */+ if (restriction_is_always_true(root, restrictinfo))+ return;+
cur_relid = -1;
while ((cur_relid = bms_next_member(join_relids, cur_relid)) >= 0)
{
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.cindex 39932d3c2d..d810ad8127 100644--- a/src/backend/optimizer/util/plancat.c+++ b/src/backend/optimizer/util/plancat.c@@ -163,6 +163,15 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent,
rel->attr_widths = (int32 *)
palloc0((rel->max_attr - rel->min_attr + 1) * sizeof(int32));
+ /* record which columns are defined as NOT NULL */+ for (int i = 0; i < relation->rd_att->natts; i++)+ {+ FormData_pg_attribute *attr = &relation->rd_att->attrs[i];++ if (attr->attnotnull)+ rel->notnullattnums = bms_add_member(rel->notnullattnums, attr->attnum);+ }+
/*
* Estimate relation size --- unless it's an inheritance parent, in which
* case the size we want is not the rel's own size but the size of its
diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.cindex 15e3910b79..9b2b390c46 100644--- a/src/backend/optimizer/util/relnode.c+++ b/src/backend/optimizer/util/relnode.c@@ -221,6 +221,7 @@ build_simple_rel(PlannerInfo *root, int relid, RelOptInfo *parent)
rel->relid = relid;
rel->rtekind = rte->rtekind;
/* min_attr, max_attr, attr_needed, attr_widths are set below */
+ rel->notnullattnums = NULL;
rel->lateral_vars = NIL;
rel->indexlist = NIL;
rel->statlist = NIL;
@@ -707,6 +708,7 @@ build_join_rel(PlannerInfo *root,
joinrel->max_attr = 0;
joinrel->attr_needed = NULL;
joinrel->attr_widths = NULL;
+ joinrel->notnullattnums = NULL;
joinrel->nulling_relids = NULL;
joinrel->lateral_vars = NIL;
joinrel->lateral_referencers = NULL;
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.hindex c17b53f7ad..a0b602c9be 100644--- a/src/include/nodes/pathnodes.h+++ b/src/include/nodes/pathnodes.h@@ -913,6 +913,8 @@ typedef struct RelOptInfo
Relids *attr_needed pg_node_attr(read_write_ignore);
/* array indexed [min_attr .. max_attr] */
int32 *attr_widths pg_node_attr(read_write_ignore);
+ /* zero-based set containing attnums of NOT NULL columns */+ Bitmapset *notnullattnums;
/* relids of outer joins that can null this baserel */
Relids nulling_relids;
/* LATERAL Vars and PHVs referenced by rel */
diff --git a/src/include/optimizer/planmain.h b/src/include/optimizer/planmain.hindex 5fc900737d..c8582792d4 100644--- a/src/include/optimizer/planmain.h+++ b/src/include/optimizer/planmain.h@@ -75,6 +75,8 @@ extern void add_vars_to_targetlist(PlannerInfo *root, List *vars,
extern void find_lateral_references(PlannerInfo *root);
extern void create_lateral_join_info(PlannerInfo *root);
extern List *deconstruct_jointree(PlannerInfo *root);
+extern bool restriction_is_always_true(PlannerInfo *root,+ RestrictInfo *restrictinfo);
extern void distribute_restrictinfo_to_rels(PlannerInfo *root,
RestrictInfo *restrictinfo);
extern RestrictInfo *process_implied_equality(PlannerInfo *root,
diff --git a/src/test/regress/expected/predicate.out b/src/test/regress/expected/predicate.outnew file mode 100644index 0000000000..43c8985042--- /dev/null+++ b/src/test/regress/expected/predicate.out@@ -0,0 +1,107 @@+--+-- Tests for predicate handling+--+--+-- test that restrictions that are always true are ignored+--+-- currently we only check for NullTest quals and OR clauses that include+-- NullTest quals. We may extend it in the future.+--+create table pred_tab (a int not null, b int);+-- An IS_NOT_NULL qual in restriction clauses can be ignored if it's on a NOT+-- NULL column+explain (costs off)+select * from pred_tab t where t.a is not null;+ QUERY PLAN +------------------------+ Seq Scan on pred_tab t+(1 row)++-- On the contrary, an IS_NOT_NULL qual in restriction clauses can not be+-- ignored if it's not on a NOT NULL column+explain (costs off)+select * from pred_tab t where t.b is not null;+ QUERY PLAN +---------------------------+ Seq Scan on pred_tab t+ Filter: (b IS NOT NULL)+(2 rows)++-- Tests for OR clauses in restriction clauses+explain (costs off)+select * from pred_tab t where t.a is not null or t.b = 1;+ QUERY PLAN +------------------------+ Seq Scan on pred_tab t+(1 row)++explain (costs off)+select * from pred_tab t where t.b is not null or t.a = 1;+ QUERY PLAN +----------------------------------------+ Seq Scan on pred_tab t+ Filter: ((b IS NOT NULL) OR (a = 1))+(2 rows)++-- An IS_NOT_NULL qual in join clauses can be ignored if+-- a) it's on a NOT NULL column, and+-- b) its Var is not nulled by any outer joins+explain (costs off)+select * from pred_tab t1 left join pred_tab t2 on true left join pred_tab t3 on t2.a is not null;+ QUERY PLAN +-------------------------------------------------+ Nested Loop Left Join+ -> Seq Scan on pred_tab t1+ -> Materialize+ -> Nested Loop Left Join+ -> Seq Scan on pred_tab t2+ -> Materialize+ -> Seq Scan on pred_tab t3+(7 rows)++-- Otherwise the IS_NOT_NULL qual in join clauses cannot be ignored+explain (costs off)+select * from pred_tab t1 left join pred_tab t2 on t1.a = 1 left join pred_tab t3 on t2.a is not null;+ QUERY PLAN +-------------------------------------------+ Nested Loop Left Join+ Join Filter: (t2.a IS NOT NULL)+ -> Nested Loop Left Join+ Join Filter: (t1.a = 1)+ -> Seq Scan on pred_tab t1+ -> Materialize+ -> Seq Scan on pred_tab t2+ -> Materialize+ -> Seq Scan on pred_tab t3+(9 rows)++-- Tests for OR clauses in join clauses+explain (costs off)+select * from pred_tab t1 left join pred_tab t2 on true left join pred_tab t3 on t2.a is not null or t2.b = 1;+ QUERY PLAN +-------------------------------------------------+ Nested Loop Left Join+ -> Seq Scan on pred_tab t1+ -> Materialize+ -> Nested Loop Left Join+ -> Seq Scan on pred_tab t2+ -> Materialize+ -> Seq Scan on pred_tab t3+(7 rows)++explain (costs off)+select * from pred_tab t1 left join pred_tab t2 on t1.a = 1 left join pred_tab t3 on t2.a is not null or t2.b = 1;+ QUERY PLAN +---------------------------------------------------+ Nested Loop Left Join+ Join Filter: ((t2.a IS NOT NULL) OR (t2.b = 1))+ -> Nested Loop Left Join+ Join Filter: (t1.a = 1)+ -> Seq Scan on pred_tab t1+ -> Materialize+ -> Seq Scan on pred_tab t2+ -> Materialize+ -> Seq Scan on pred_tab t3+(9 rows)++drop table pred_tab;diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_scheduleindex cf46fa3359..25f0596971 100644--- a/src/test/regress/parallel_schedule+++ b/src/test/regress/parallel_schedule@@ -119,7 +119,7 @@ test: plancache limit plpgsql copy2 temp domain rangefuncs prepare conversion tr
# The stats test resets stats, so nothing else needing stats access can be in
# this group.
# ----------
-test: partition_join partition_prune reloptions hash_part indexing partition_aggregate partition_info tuplesort explain compression memoize stats+test: partition_join partition_prune reloptions hash_part indexing partition_aggregate partition_info tuplesort explain compression memoize stats predicate
# event_trigger cannot run concurrently with any test that runs DDL
# oidjoins is read-only, though, and should run late for best coverage
diff --git a/src/test/regress/sql/predicate.sql b/src/test/regress/sql/predicate.sqlnew file mode 100644index 0000000000..0b2e4cd0a6--- /dev/null+++ b/src/test/regress/sql/predicate.sql@@ -0,0 +1,48 @@+--+-- Tests for predicate handling+--++--+-- test that restrictions that are always true are ignored+--+-- currently we only check for NullTest quals and OR clauses that include+-- NullTest quals. We may extend it in the future.+--+create table pred_tab (a int not null, b int);++-- An IS_NOT_NULL qual in restriction clauses can be ignored if it's on a NOT+-- NULL column+explain (costs off)+select * from pred_tab t where t.a is not null;++-- On the contrary, an IS_NOT_NULL qual in restriction clauses can not be+-- ignored if it's not on a NOT NULL column+explain (costs off)+select * from pred_tab t where t.b is not null;++-- Tests for OR clauses in restriction clauses+explain (costs off)+select * from pred_tab t where t.a is not null or t.b = 1;++explain (costs off)+select * from pred_tab t where t.b is not null or t.a = 1;++-- An IS_NOT_NULL qual in join clauses can be ignored if+-- a) it's on a NOT NULL column, and+-- b) its Var is not nulled by any outer joins+explain (costs off)+select * from pred_tab t1 left join pred_tab t2 on true left join pred_tab t3 on t2.a is not null;++-- Otherwise the IS_NOT_NULL qual in join clauses cannot be ignored+explain (costs off)+select * from pred_tab t1 left join pred_tab t2 on t1.a = 1 left join pred_tab t3 on t2.a is not null;++-- Tests for OR clauses in join clauses+explain (costs off)+select * from pred_tab t1 left join pred_tab t2 on true left join pred_tab t3 on t2.a is not null or t2.b = 1;++explain (costs off)+select * from pred_tab t1 left join pred_tab t2 on t1.a = 1 left join pred_tab t3 on t2.a is not null or t2.b = 1;++drop table pred_tab;+--
2.31.0
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: guofenglinux@gmail.com, dgrowleyml@gmail.com, tgl@sss.pgh.pa.us, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17540: Prepared statement: PG switches to a generic query plan which is consistently much slower
In-Reply-To: <CAMbWs4-cWwe0whACTuKTLT+4jNcSJ+RLEcSp4us1vBAt0k7GAA@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox