public inbox for [email protected]  
help / color / mirror / Atom feed
From: Richard Guo <[email protected]>
To: PostgreSQL-development <[email protected]>
Subject: Inconsistent RestrictInfo serial numbers
Date: Tue, 8 Oct 2024 19:20:30 +0800
Message-ID: <CAMbWs4-B6kafn+LmPuh-TYFwFyEm-vVj3Qqv7Yo-69CEv14rRg@mail.gmail.com> (raw)

I ran into an "ERROR:  variable not found in subplan target lists"
error, which can be reproduced with the following query.

create table t (a int primary key, b int);
insert into t select i, i from generate_series(1, 10)i;
analyze t;

explain (costs off)
select 1 from t t1
    left join
        (t t2 left join t t3 on t2.a = t3.a) on true
    left join t t4 on t1.a is null and t1.b = 1
    right join t t5 on t1.b = t5.b;
ERROR:  variable not found in subplan target lists

The first bad commit is a3179ab69.

commit a3179ab692be4314d5ee5cd56598976c487d5ef2
Author: Tom Lane <[email protected]>
Date:   Fri Sep 27 16:04:04 2024 -0400

    Recalculate where-needed data accurately after a join removal.

However, after digging into it further, it seems to me that the issue
originated in b262ad440, and the changes in a3179ab69 made it easier
to surface.

commit b262ad440edecda0b1aba81d967ab560a83acb8a
Author: David Rowley <[email protected]>
Date:   Tue Jan 23 18:09:18 2024 +1300

    Add better handling of redundant IS [NOT] NULL quals

When we generate multiple clones of the same qual condition to cope
with outer join identity 3, we need to ensure that all the clones get
the same serial number.  To achieve this, we reset the
root->last_rinfo_serial counter each time we produce RestrictInfo(s)
from the qual (see deconstruct_distribute_oj_quals).  This approach
works only if we ensure that we are not changing the qual list in
any way that'd affect the number of RestrictInfos built from it.

However, with b262ad440, an IS NULL qual on a NOT NULL column might
result in an additional constant-FALSE RestrictInfo.  This can
unexpectedly increase root->last_rinfo_serial, causing inconsistent
RestrictInfo serial numbers across multiple clones of the same qual,
which can confuse users of 'rinfo_serial', such as
rebuild_joinclause_attr_needed, and lead to planner errors.

In the query above, the has_clone version of qual 't1.a is null' would
be reduced to constant-FALSE, while the is_clone version would not.
This results in differing serial numbers for subsequent quals (such as
qual 't1.b = 1') across different versions.

To fix, I think we can reset the root->last_rinfo_serial counter after
generating the additional constant-FALSE RestrictInfo.  Please see
attached.

Thanks
Richard


Attachments:

  [application/octet-stream] v1-0001-Fix-inconsistent-RestrictInfo-serial-numbers.patch (6.3K, ../CAMbWs4-B6kafn+LmPuh-TYFwFyEm-vVj3Qqv7Yo-69CEv14rRg@mail.gmail.com/2-v1-0001-Fix-inconsistent-RestrictInfo-serial-numbers.patch)
  download | inline diff:
From dbf9093c642a42d9831cad0d8914bb096750c625 Mon Sep 17 00:00:00 2001
From: Richard Guo <[email protected]>
Date: Tue, 8 Oct 2024 18:59:03 +0900
Subject: [PATCH v1] Fix inconsistent RestrictInfo serial numbers

When we generate multiple clones of the same qual condition to cope
with outer join identity 3, we need to ensure that all the clones get
the same serial number.  To achieve this, we reset the
root->last_rinfo_serial counter each time we produce RestrictInfo(s)
from the qual (see deconstruct_distribute_oj_quals).  This approach
works only if we ensure that we are not changing the qual list in
any way that'd affect the number of RestrictInfos built from it.

However, with b262ad440, an IS NULL qual on a NOT NULL column might
result in an additional constant-FALSE RestrictInfo.  This can
unexpectedly increase root->last_rinfo_serial, causing inconsistent
RestrictInfo serial numbers across multiple clones of the same qual,
which can confuse users of these serial numbers, such as
rebuild_joinclause_attr_needed, and lead to planner errors.

To fix, reset the root->last_rinfo_serial counter after generating the
additional constant-FALSE RestrictInfo.
---
 src/backend/optimizer/plan/initsplan.c  | 10 ++++++++-
 src/backend/optimizer/util/joininfo.c   | 12 +++++++++--
 src/test/regress/expected/predicate.out | 28 +++++++++++++++++++++++++
 src/test/regress/sql/predicate.sql      | 15 +++++++++++++
 4 files changed, 62 insertions(+), 3 deletions(-)

diff --git a/src/backend/optimizer/plan/initsplan.c b/src/backend/optimizer/plan/initsplan.c
index c5bc0f51e9..903c397d40 100644
--- a/src/backend/optimizer/plan/initsplan.c
+++ b/src/backend/optimizer/plan/initsplan.c
@@ -2767,11 +2767,18 @@ add_base_clause_to_rel(PlannerInfo *root, Index relid,
 
 		/*
 		 * Substitute the origin qual with constant-FALSE if it is provably
-		 * always false.  Note that we keep the same rinfo_serial.
+		 * always false.
+		 *
+		 * Note that we need to keep the same rinfo_serial, since it is in
+		 * practice the same condition.  We also need to reset the
+		 * last_rinfo_serial counter, which is essential to ensure that the
+		 * RestrictInfos for the "same" qual condition get identical serial
+		 * numbers (see deconstruct_distribute_oj_quals).
 		 */
 		if (restriction_is_always_false(root, restrictinfo))
 		{
 			int			save_rinfo_serial = restrictinfo->rinfo_serial;
+			int			save_last_rinfo_serial = root->last_rinfo_serial;
 
 			restrictinfo = make_restrictinfo(root,
 											 (Expr *) makeBoolConst(false, false),
@@ -2784,6 +2791,7 @@ add_base_clause_to_rel(PlannerInfo *root, Index relid,
 											 restrictinfo->incompatible_relids,
 											 restrictinfo->outer_relids);
 			restrictinfo->rinfo_serial = save_rinfo_serial;
+			root->last_rinfo_serial = save_last_rinfo_serial;
 		}
 	}
 
diff --git a/src/backend/optimizer/util/joininfo.c b/src/backend/optimizer/util/joininfo.c
index 5fb0c17630..65993bd659 100644
--- a/src/backend/optimizer/util/joininfo.c
+++ b/src/backend/optimizer/util/joininfo.c
@@ -106,12 +106,19 @@ add_join_clause_to_rels(PlannerInfo *root,
 		return;
 
 	/*
-	 * Substitute constant-FALSE for the origin qual if it is always false.
-	 * Note that we keep the same rinfo_serial.
+	 * Substitute the origin qual with constant-FALSE if it is provably always
+	 * false.
+	 *
+	 * Note that we need to keep the same rinfo_serial, since it is in
+	 * practice the same condition.  We also need to reset the
+	 * last_rinfo_serial counter, which is essential to ensure that the
+	 * RestrictInfos for the "same" qual condition get identical serial
+	 * numbers (see deconstruct_distribute_oj_quals).
 	 */
 	if (restriction_is_always_false(root, restrictinfo))
 	{
 		int			save_rinfo_serial = restrictinfo->rinfo_serial;
+		int			save_last_rinfo_serial = root->last_rinfo_serial;
 
 		restrictinfo = make_restrictinfo(root,
 										 (Expr *) makeBoolConst(false, false),
@@ -124,6 +131,7 @@ add_join_clause_to_rels(PlannerInfo *root,
 										 restrictinfo->incompatible_relids,
 										 restrictinfo->outer_relids);
 		restrictinfo->rinfo_serial = save_rinfo_serial;
+		root->last_rinfo_serial = save_last_rinfo_serial;
 	}
 
 	cur_relid = -1;
diff --git a/src/test/regress/expected/predicate.out b/src/test/regress/expected/predicate.out
index 6f1cc0d54c..965a3a7616 100644
--- a/src/test/regress/expected/predicate.out
+++ b/src/test/regress/expected/predicate.out
@@ -290,3 +290,31 @@ SELECT * FROM pred_parent WHERE a IS NULL;
 (2 rows)
 
 DROP TABLE pred_parent, pred_child;
+-- Validate the additional constant-FALSE qual does not cause inconsistent
+-- RestrictInfo serial numbers
+CREATE TABLE pred_tab (a int PRIMARY KEY, b int);
+INSERT INTO pred_tab SELECT i, i FROM generate_series(1, 10)i;
+ANALYZE pred_tab;
+EXPLAIN (COSTS OFF)
+SELECT 1 FROM pred_tab t1
+    LEFT JOIN
+        (pred_tab t2 LEFT JOIN pred_tab t3 ON t2.a = t3.a) ON TRUE
+    LEFT JOIN pred_tab t4 ON t1.a IS NULL AND t1.b = 1
+    RIGHT JOIN pred_tab t5 ON t1.b = t5.b;
+                    QUERY PLAN                     
+---------------------------------------------------
+ Hash Right Join
+   Hash Cond: (t1.b = t5.b)
+   ->  Nested Loop Left Join
+         ->  Nested Loop Left Join
+               Join Filter: (false AND (t1.b = 1))
+               ->  Seq Scan on pred_tab t1
+               ->  Result
+                     One-Time Filter: false
+         ->  Materialize
+               ->  Seq Scan on pred_tab t2
+   ->  Hash
+         ->  Seq Scan on pred_tab t5
+(12 rows)
+
+DROP TABLE pred_tab;
diff --git a/src/test/regress/sql/predicate.sql b/src/test/regress/sql/predicate.sql
index 63f6a7786f..661013ff7e 100644
--- a/src/test/regress/sql/predicate.sql
+++ b/src/test/regress/sql/predicate.sql
@@ -147,3 +147,18 @@ EXPLAIN (COSTS OFF)
 SELECT * FROM pred_parent WHERE a IS NULL;
 
 DROP TABLE pred_parent, pred_child;
+
+-- Validate the additional constant-FALSE qual does not cause inconsistent
+-- RestrictInfo serial numbers
+CREATE TABLE pred_tab (a int PRIMARY KEY, b int);
+INSERT INTO pred_tab SELECT i, i FROM generate_series(1, 10)i;
+ANALYZE pred_tab;
+
+EXPLAIN (COSTS OFF)
+SELECT 1 FROM pred_tab t1
+    LEFT JOIN
+        (pred_tab t2 LEFT JOIN pred_tab t3 ON t2.a = t3.a) ON TRUE
+    LEFT JOIN pred_tab t4 ON t1.a IS NULL AND t1.b = 1
+    RIGHT JOIN pred_tab t5 ON t1.b = t5.b;
+
+DROP TABLE pred_tab;
-- 
2.43.0



view thread (2+ messages)

reply

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: [email protected]
  Cc: [email protected]
  Subject: Re: Inconsistent RestrictInfo serial numbers
  In-Reply-To: <CAMbWs4-B6kafn+LmPuh-TYFwFyEm-vVj3Qqv7Yo-69CEv14rRg@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 agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox