v1-0001-Fix-negative-bitmapset-member-error.patch

From 5429590c0a058bc681b450b0130ecb1f848fb740 Mon Sep 17 00:00:00 2001
From: Richard Guo <guofenglinux@gmail.com>
Date: Thu, 28 Dec 2023 10:56:17 +0800
Subject: [PATCH v1] Fix 'negative bitmapset member' error

When removing a useless join, we'd remove PHVs that are not used at join
partner rels or above the join.  A PHV that references the join's relid
in ph_eval_at is logically "above" the join and thus should not be
removed.  We have the following check for that:

    !bms_is_member(ojrelid, phinfo->ph_eval_at)

However, in the case of SJE removing an useless inner join, 'ojrelid' is
set to -1, which would trigger the "negative bitmapset member not
allowed" error in bms_is_member().

We can fix it by skipping checking ojrelid for inner joins.  But this
patch chooses to modify bms_is_member() to return false for negative
numbers instead of emitting an error.
---
 src/backend/nodes/bitmapset.c      |  4 ++--
 src/test/regress/expected/join.out | 20 ++++++++++++++++++++
 src/test/regress/sql/join.sql      |  8 ++++++++
 3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/src/backend/nodes/bitmapset.c b/src/backend/nodes/bitmapset.c
index e13ecaa155..2a828ce709 100644
--- a/src/backend/nodes/bitmapset.c
+++ b/src/backend/nodes/bitmapset.c
@@ -477,9 +477,9 @@ bms_is_member(int x, const Bitmapset *a)
 	int			wordnum,
 				bitnum;
 
-	/* XXX better to just return false for x<0 ? */
+	/* negative number cannot be a member of the bitmapset */
 	if (x < 0)
-		elog(ERROR, "negative bitmapset member not allowed");
+		return false;
 	if (a == NULL)
 		return false;
 
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index 1557e17299..dc5d11317f 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -6821,6 +6821,26 @@ on true;
                            Filter: (id IS NOT NULL)
 (8 rows)
 
+-- Check that SJE removes the whole PHVs correctly
+explain (verbose, costs off)
+select 1 from emp1 t1 left join
+    (lateral (select 1 as x, * from emp1 t2) s1 inner join
+        (select * from emp1 t3) s2 on s1.id = s2.id)
+    on true
+where s1.x = 1;
+                       QUERY PLAN                        
+---------------------------------------------------------
+ Nested Loop
+   Output: 1
+   ->  Seq Scan on public.emp1 t1
+         Output: t1.id, t1.code
+   ->  Materialize
+         Output: t3.id
+         ->  Seq Scan on public.emp1 t3
+               Output: t3.id
+               Filter: ((t3.id IS NOT NULL) AND (1 = 1))
+(9 rows)
+
 -- Check that PHVs do not impose any constraints on removing self joins
 explain (verbose, costs off)
 select * from emp1 t1 join emp1 t2 on t1.id = t2.id left join
diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index fed9e83e31..e2ec5c1c12 100644
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -2600,6 +2600,14 @@ select * from emp1 t1 left join
         on true)
 on true;
 
+-- Check that SJE removes the whole PHVs correctly
+explain (verbose, costs off)
+select 1 from emp1 t1 left join
+    (lateral (select 1 as x, * from emp1 t2) s1 inner join
+        (select * from emp1 t3) s2 on s1.id = s2.id)
+    on true
+where s1.x = 1;
+
 -- Check that PHVs do not impose any constraints on removing self joins
 explain (verbose, costs off)
 select * from emp1 t1 join emp1 t2 on t1.id = t2.id left join
-- 
2.31.0