v3-0001-ignore-is-not-null-quals-on-not-null-columns.patch

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.c
index 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.c
index 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.c
index 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.c
index 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.h
index 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.h
index 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.out
new file mode 100644
index 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_schedule
index 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.sql
new file mode 100644
index 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