public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH v13 06/18] More refactoring
3+ messages / 3 participants
[nested] [flat]

* [PATCH v13 06/18] More refactoring
@ 2020-11-01 19:46  Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 3+ messages in thread

From: Justin Pryzby @ 2020-11-01 19:46 UTC (permalink / raw)

---
 src/backend/commands/indexcmds.c           | 201 ++++++++++-----------
 src/test/regress/expected/create_index.out |   4 +-
 2 files changed, 93 insertions(+), 112 deletions(-)

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index f5fea14ff4..5c5596cd28 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -102,8 +102,8 @@ static void ReindexMultipleInternal(List *relids,
 									ReindexParams *params);
 static bool ReindexRelationConcurrently(Oid relationOid,
 										ReindexParams *params);
-static List *ReindexIndexesConcurrently(List *indexIds, List *heapRelationIds,
-		ReindexParams *params, MemoryContext private_context);
+static List *ReindexIndexesConcurrently(List *indexIds, ReindexParams *params,
+		MemoryContext private_context);
 static void update_relispartition(Oid relationId, bool newval);
 static inline void set_indexsafe_procflags(void);
 
@@ -2652,8 +2652,8 @@ ReindexIndex(RangeVar *indexRelation, ReindexParams *params, bool isTopLevel)
 			.indexId = indOid,
 			/* other fields set later */
 		};
+
 		ReindexIndexesConcurrently(list_make1(&idxinfo),
-				list_make1_oid(IndexGetRelation(indOid, false)),
 				params, CurrentMemoryContext);
 	}
 	else
@@ -3023,12 +3023,11 @@ reindex_error_callback(void *arg)
 
 
 /*
- * Given a list of index oids, return a list of leaf partitions by removing
- * any intermediate parents.  heaprels is populated with the corresponding
- * tables.
+ * Given a list of index oids, return a new list of leaf partitions by
+ * excluding any intermediate parents.
  */
 static List *
-leaf_indexes(List *inhoids, int options, List **heaprels)
+leaf_indexes(List *inhoids, int options)
 {
 	List		*partitions = NIL;
 	ListCell	*lc;
@@ -3060,7 +3059,6 @@ leaf_indexes(List *inhoids, int options, List **heaprels)
 
 		/* Save partition OID in current MemoryContext */
 		partitions = lappend_oid(partitions, partoid);
-		*heaprels = lappend_oid(*heaprels, tableoid);
 	}
 
 	return partitions;
@@ -3072,13 +3070,11 @@ leaf_indexes(List *inhoids, int options, List **heaprels)
  *
  * Reindex a set of partitions, per the partitioned index or table given
  * by the caller.
- * XXX: should be further refactored with logic from ReindexRelationConcurrently
  */
 static void
 ReindexPartitions(Oid relid, ReindexParams *params, bool isTopLevel)
 {
-	List	   *partitions = NIL,
-			*heaprels = NIL;
+	List	   *partitions = NIL;
 	char		relkind = get_rel_relkind(relid);
 	char	   *relname = get_rel_name(relid);
 	char	   *relnamespace = get_namespace_name(get_rel_namespace(relid));
@@ -3132,7 +3128,7 @@ ReindexPartitions(Oid relid, ReindexParams *params, bool isTopLevel)
 	if (relkind == RELKIND_PARTITIONED_INDEX)
 	{
 		old_context = MemoryContextSwitchTo(reindex_context);
-		partitions = leaf_indexes(inhoids, params->options, &heaprels);
+		partitions = leaf_indexes(inhoids, params->options);
 		MemoryContextSwitchTo(old_context);
 	} else {
 		/* Loop over parent tables */
@@ -3145,7 +3141,7 @@ ReindexPartitions(Oid relid, ReindexParams *params, bool isTopLevel)
 			parttable = table_open(partoid, ShareLock);
 			old_context = MemoryContextSwitchTo(reindex_context);
 			partindexes = RelationGetIndexList(parttable);
-			partindexes = leaf_indexes(partindexes, params->options, &heaprels);
+			partindexes = leaf_indexes(partindexes, params->options);
 			partitions = list_concat(partitions, partindexes);
 
 			MemoryContextSwitchTo(old_context);
@@ -3154,10 +3150,9 @@ ReindexPartitions(Oid relid, ReindexParams *params, bool isTopLevel)
 	}
 
 	if ((params->options & REINDEXOPT_CONCURRENTLY) != 0 &&
-		relkind == RELKIND_PARTITIONED_INDEX &&
 		get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
 	{
-		List			   *idxinfos = NIL;
+		List			    *idxinfos = NIL;
 		ReindexIndexInfo	*idxinfo;
 
 		old_context = MemoryContextSwitchTo(reindex_context);
@@ -3172,7 +3167,7 @@ ReindexPartitions(Oid relid, ReindexParams *params, bool isTopLevel)
 		MemoryContextSwitchTo(old_context);
 
 		/* Process all indexes in a single loop */
-		ReindexIndexesConcurrently(idxinfos, heaprels, params, reindex_context);
+		ReindexIndexesConcurrently(idxinfos, params, reindex_context);
 	} else {
 		/*
 		 * Process each partition listed in a separate transaction.  Note that
@@ -3342,7 +3337,6 @@ ReindexMultipleInternal(List *relids, ReindexParams *params)
 static bool
 ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
 {
-	List	   *heapRelationIds = NIL;
 	List	   *indexIds = NIL;
 	List	   *newIndexIds = NIL;
 	ListCell   *lc,
@@ -3395,14 +3389,6 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
 				 */
 				Relation	heapRelation;
 
-				/* Save the list of relation OIDs in private context */
-				oldcontext = MemoryContextSwitchTo(private_context);
-
-				/* Track this relation for session locks */
-				heapRelationIds = lappend_oid(heapRelationIds, relationOid);
-
-				MemoryContextSwitchTo(oldcontext);
-
 				if (IsCatalogRelationOid(relationOid))
 					ereport(ERROR,
 							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
@@ -3415,7 +3401,7 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
 												  ShareUpdateExclusiveLock);
 					/* leave if relation does not exist */
 					if (!heapRelation)
-						break;
+						break; // XXX: lremove
 				}
 				else
 					heapRelation = table_open(relationOid,
@@ -3473,14 +3459,6 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
 					Relation	toastRelation = table_open(toastOid,
 														   ShareUpdateExclusiveLock);
 
-					/* Save the list of relation OIDs in private context */
-					oldcontext = MemoryContextSwitchTo(private_context);
-
-					/* Track this relation for session locks */
-					heapRelationIds = lappend_oid(heapRelationIds, toastOid);
-
-					MemoryContextSwitchTo(oldcontext);
-
 					foreach(lc2, RelationGetIndexList(toastRelation))
 					{
 						Oid			cellOid = lfirst_oid(lc2);
@@ -3521,78 +3499,6 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
 				break;
 			}
 		case RELKIND_INDEX:
-			{
-				Oid			heapId = IndexGetRelation(relationOid,
-													  (params->options & REINDEXOPT_MISSING_OK) != 0);
-				Relation	heapRelation;
-				ReindexIndexInfo *idx;
-
-				/* if relation is missing, leave */
-				if (!OidIsValid(heapId))
-					break;
-
-				if (IsCatalogRelationOid(heapId))
-					ereport(ERROR,
-							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-							 errmsg("cannot reindex system catalogs concurrently")));
-
-				/*
-				 * Don't allow reindex for an invalid index on TOAST table, as
-				 * if rebuilt it would not be possible to drop it.  Match
-				 * error message in reindex_index().
-				 */
-				if (IsToastNamespace(get_rel_namespace(relationOid)) &&
-					!get_index_isvalid(relationOid))
-					ereport(ERROR,
-							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-							 errmsg("cannot reindex invalid index on TOAST table")));
-
-				/*
-				 * Check if parent relation can be locked and if it exists,
-				 * this needs to be done at this stage as the list of indexes
-				 * to rebuild is not complete yet, and REINDEXOPT_MISSING_OK
-				 * should not be used once all the session locks are taken.
-				 */
-				if ((params->options & REINDEXOPT_MISSING_OK) != 0)
-				{
-					heapRelation = try_table_open(heapId,
-												  ShareUpdateExclusiveLock);
-					/* leave if relation does not exist */
-					if (!heapRelation)
-						break;
-				}
-				else
-					heapRelation = table_open(heapId,
-											  ShareUpdateExclusiveLock);
-
-				if (OidIsValid(params->tablespaceOid) &&
-					IsSystemRelation(heapRelation))
-					ereport(ERROR,
-							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-							 errmsg("cannot move system relation \"%s\"",
-									get_rel_name(relationOid))));
-
-				table_close(heapRelation, NoLock);
-
-				/* Save the list of relation OIDs in private context */
-				oldcontext = MemoryContextSwitchTo(private_context);
-
-				/* Track the heap relation of this index for session locks */
-				heapRelationIds = list_make1_oid(heapId);
-
-				/*
-				 * Save the list of relation OIDs in private context.  Note
-				 * that invalid indexes are allowed here.
-				 */
-				idx = palloc(sizeof(ReindexIndexInfo));
-				idx->indexId = relationOid;
-				indexIds = lappend(indexIds, idx);
-				/* other fields set later */
-
-				MemoryContextSwitchTo(oldcontext);
-				break;
-			}
-
 		case RELKIND_PARTITIONED_TABLE:
 		case RELKIND_PARTITIONED_INDEX:
 		default:
@@ -3623,10 +3529,10 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
 				 errmsg("cannot move non-shared relation to tablespace \"%s\"",
 						get_tablespace_name(params->tablespaceOid))));
 
-	Assert(heapRelationIds != NIL);
+	// Assert(heapRelationIds != NIL);
 
 	/* Do the work */
-	newIndexIds = ReindexIndexesConcurrently(indexIds, heapRelationIds, params, private_context);
+	newIndexIds = ReindexIndexesConcurrently(indexIds, params, private_context);
 
 	/* Log what we did */
 	if ((params->options & REINDEXOPT_VERBOSE) != 0)
@@ -3668,9 +3574,10 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
  * This is called by ReindexRelationConcurrently and
  */
 static List *
-ReindexIndexesConcurrently(List *indexIds, List *heapRelationIds,
-		ReindexParams *params, MemoryContext private_context)
+ReindexIndexesConcurrently(List *indexIds, ReindexParams *params,
+		MemoryContext private_context)
 {
+	List		*heapRelationIds = NIL;
 	List	   *newIndexIds = NIL;
 	List	   *relationLocks = NIL;
 	List	   *lockTags = NIL;
@@ -3688,6 +3595,80 @@ ReindexIndexesConcurrently(List *indexIds, List *heapRelationIds,
 	};
 	int64		progress_vals[4];
 
+	/* It's not a shared catalog, so refuse to move it to shared tablespace */
+	if (params->tablespaceOid == GLOBALTABLESPACE_OID && false)
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("cannot move non-shared relation to tablespace \"%s\"",
+						get_tablespace_name(params->tablespaceOid))));
+
+	foreach(lc, indexIds)
+	{
+		ReindexIndexInfo	*idx = lfirst(lc);
+		Oid			indexrelid = idx->indexId;
+		Oid			heapId = IndexGetRelation(indexrelid,
+											  (params->options & REINDEXOPT_MISSING_OK) != 0);
+		Relation	heapRelation;
+
+		/* if relation is missing, leave */
+		if (!OidIsValid(heapId))
+			break; // XXX: ldelete?
+
+		if (IsCatalogRelationOid(heapId))
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					 errmsg("cannot reindex system catalogs concurrently")));
+
+		/*
+		 * Don't allow reindex for an invalid index on TOAST table, as
+		 * if rebuilt it would not be possible to drop it.  Match
+		 * error message in reindex_index().
+		 */
+		if (IsToastNamespace(get_rel_namespace(indexrelid)) &&
+			!get_index_isvalid(indexrelid))
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					 errmsg("cannot reindex invalid index on TOAST table")));
+
+		if (OidIsValid(params->tablespaceOid) &&
+			IsCatalogRelationOid(indexrelid))
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					 errmsg("cannot move system relation \"%s\"",
+							get_rel_name(indexrelid))));
+
+		/*
+		 * Check if parent relation can be locked and if it exists,
+		 * this needs to be done at this stage as the list of indexes
+		 * to rebuild is not complete yet, and REINDEXOPT_MISSING_OK
+		 * should not be used once all the session locks are taken.
+		 */
+		if ((params->options & REINDEXOPT_MISSING_OK) != 0)
+		{
+			heapRelation = try_table_open(heapId,
+										  ShareUpdateExclusiveLock);
+			/* leave if relation does not exist */
+			if (!heapRelation)
+				break; // ldelete
+		}
+		else
+			heapRelation = table_open(heapId,
+									  ShareUpdateExclusiveLock);
+		table_close(heapRelation, NoLock);
+
+		/* Save the list of relation OIDs in private context */
+		oldcontext = MemoryContextSwitchTo(private_context);
+
+		/* Track the heap relation of this index for session locks */
+		heapRelationIds = lappend_oid(heapRelationIds, heapId);
+		// heapRelationIds = list_make1_oid(heapId);
+
+		/* Note that invalid indexes are allowed here. */
+
+		MemoryContextSwitchTo(oldcontext);
+		// break;
+	}
+
 	/*-----
 	 * Now we have all the indexes we want to process in indexIds.
 	 *
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index 6f41adf736..830fdddf24 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -2470,12 +2470,12 @@ COMMIT;
 REINDEX TABLE CONCURRENTLY pg_class; -- no catalog relation
 ERROR:  cannot reindex system catalogs concurrently
 REINDEX INDEX CONCURRENTLY pg_class_oid_index; -- no catalog index
-ERROR:  concurrent index creation on system catalog tables is not supported
+ERROR:  cannot reindex system catalogs concurrently
 -- These are the toast table and index of pg_authid.
 REINDEX TABLE CONCURRENTLY pg_toast.pg_toast_1260; -- no catalog toast table
 ERROR:  cannot reindex system catalogs concurrently
 REINDEX INDEX CONCURRENTLY pg_toast.pg_toast_1260_index; -- no catalog toast index
-ERROR:  concurrent index creation on system catalog tables is not supported
+ERROR:  cannot reindex system catalogs concurrently
 REINDEX SYSTEM CONCURRENTLY postgres; -- not allowed for SYSTEM
 ERROR:  cannot reindex system catalogs concurrently
 -- Warns about catalog relations
-- 
2.17.0


--QTprm0S8XgL7H0Dt--





^ permalink  raw  reply  [nested|flat] 3+ messages in thread

* Wrong results with right-semi-joins
@ 2024-12-03 08:56  Richard Guo <[email protected]>
  0 siblings, 1 reply; 3+ messages in thread

From: Richard Guo @ 2024-12-03 08:56 UTC (permalink / raw)
  To: pgsql-hackers

I ran into $subject and it can be reproduced with the query below.

create temp table tbl_rs(a int, b int);
insert into tbl_rs select i, i from generate_series(1,10)i;
analyze tbl_rs;

set enable_nestloop to off;
set enable_hashagg to off;

select * from tbl_rs t1
where (select a from tbl_rs t2
       where exists (select 1 from
                     (select (b in (select b from tbl_rs t3)) as c
                      from tbl_rs t4 where t4.a = 1) s
                     where c in
                      (select t1.a = 1 from tbl_rs t5 union all select true))
       order by a limit 1) >= 0;
 a | b
---+---
 1 | 1
(1 row)

The expected output should be 10 rows, not 1.

I've traced the root cause to ExecReScanHashJoin, where we neglect to
reset the inner-tuple match flags in the hash table for right-semi
joins when reusing the hash table.  It was my oversight in commit
aa86129e1.  Attached is patch to fix it.

Thanks
Richard


Attachments:

  [application/octet-stream] v1-0001-Fix-right-semi-joins-in-HashJoin-rescans.patch (5.8K, ../../CAMbWs4-nQF9io2WL2SkD0eXvfPdyBc9Q=hRwfQHCGV2usa0jyA@mail.gmail.com/2-v1-0001-Fix-right-semi-joins-in-HashJoin-rescans.patch)
  download | inline diff:
From 9e2883eae1b88f3199a27ba4e90857988ff62d2b Mon Sep 17 00:00:00 2001
From: Richard Guo <[email protected]>
Date: Tue, 3 Dec 2024 17:27:35 +0900
Subject: [PATCH v1] Fix right-semi-joins in HashJoin rescans

When resetting a HashJoin node for rescans, if it is a single-batch
join and there are no parameter changes for the inner subnode, we can
just reuse the existing hash table without rebuilding it.  However,
for join types that depend on the inner-tuple match flags in the hash
table, we need to reset these match flags to avoid incorrect results.
This applies to right, right-anti, right-semi, and full joins.

When I introduced "Right Semi Join" plan shapes in aa86129e1, I failed
to reset the match flags in the hash table for right-semi joins in
rescans.  This oversight has been shown to produce incorrect results.
This patch fixes it.
---
 src/backend/executor/nodeHashjoin.c |  7 ++--
 src/test/regress/expected/join.out  | 63 +++++++++++++++++++++++++++++
 src/test/regress/sql/join.sql       | 30 ++++++++++++++
 3 files changed, 97 insertions(+), 3 deletions(-)

diff --git a/src/backend/executor/nodeHashjoin.c b/src/backend/executor/nodeHashjoin.c
index 6c3009fba0..ea0045bc0f 100644
--- a/src/backend/executor/nodeHashjoin.c
+++ b/src/backend/executor/nodeHashjoin.c
@@ -1511,10 +1511,11 @@ ExecReScanHashJoin(HashJoinState *node)
 			/*
 			 * Okay to reuse the hash table; needn't rescan inner, either.
 			 *
-			 * However, if it's a right/right-anti/full join, we'd better
-			 * reset the inner-tuple match flags contained in the table.
+			 * However, if it's a right/right-anti/right-semi/full join, we'd
+			 * better reset the inner-tuple match flags contained in the
+			 * table.
 			 */
-			if (HJ_FILL_INNER(node))
+			if (HJ_FILL_INNER(node) || node->js.jointype == JOIN_RIGHT_SEMI)
 				ExecHashTableResetMatchFlags(node->hj_HashTable);
 
 			/*
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index ebf2e3f851..51aeb1dae6 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -3036,6 +3036,69 @@ where not exists (select 1 from tbl_ra t2 where t2.b = t1.a) and t1.b < 2;
 reset enable_hashjoin;
 reset enable_nestloop;
 --
+-- regression test for bug with hash-right-semi join
+--
+create temp table tbl_rs(a int, b int);
+insert into tbl_rs select i, i from generate_series(1,10)i;
+analyze tbl_rs;
+set enable_nestloop to off;
+set enable_hashagg to off;
+-- ensure we get a hash right semi join with SubPlan in hash clauses
+explain (costs off)
+select * from tbl_rs t1
+where (select a from tbl_rs t2
+       where exists (select 1 from
+                     (select (b in (select b from tbl_rs t3)) as c from tbl_rs t4 where t4.a = 1) s
+                     where c in (select t1.a = 1 from tbl_rs t5 union all select true))
+       order by a limit 1) >= 0;
+                                      QUERY PLAN                                      
+--------------------------------------------------------------------------------------
+ Seq Scan on tbl_rs t1
+   Filter: ((SubPlan 3) >= 0)
+   SubPlan 3
+     ->  Limit
+           InitPlan 2
+             ->  Hash Right Semi Join
+                   Hash Cond: (((t1.a = 1)) = (ANY (t4.b = (hashed SubPlan 1).col1)))
+                   ->  Append
+                         ->  Seq Scan on tbl_rs t5
+                         ->  Result
+                   ->  Hash
+                         ->  Seq Scan on tbl_rs t4
+                               Filter: (a = 1)
+                         SubPlan 1
+                           ->  Seq Scan on tbl_rs t3
+           ->  Sort
+                 Sort Key: t2.a
+                 ->  Result
+                       One-Time Filter: (InitPlan 2).col1
+                       ->  Seq Scan on tbl_rs t2
+(20 rows)
+
+-- and check we get the expected results
+select * from tbl_rs t1
+where (select a from tbl_rs t2
+       where exists (select 1 from
+                     (select (b in (select b from tbl_rs t3)) as c from tbl_rs t4 where t4.a = 1) s
+                     where c in (select t1.a = 1 from tbl_rs t5 union all select true))
+       order by a limit 1) >= 0;
+ a  | b  
+----+----
+  1 |  1
+  2 |  2
+  3 |  3
+  4 |  4
+  5 |  5
+  6 |  6
+  7 |  7
+  8 |  8
+  9 |  9
+ 10 | 10
+(10 rows)
+
+reset enable_nestloop;
+reset enable_hashagg;
+--
 -- regression test for bug #13908 (hash join with skew tuples & nbatch increase)
 --
 set work_mem to '64kB';
diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index 1004fc0355..1e9dafca57 100644
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -738,6 +738,36 @@ where not exists (select 1 from tbl_ra t2 where t2.b = t1.a) and t1.b < 2;
 reset enable_hashjoin;
 reset enable_nestloop;
 
+--
+-- regression test for bug with hash-right-semi join
+--
+create temp table tbl_rs(a int, b int);
+insert into tbl_rs select i, i from generate_series(1,10)i;
+analyze tbl_rs;
+
+set enable_nestloop to off;
+set enable_hashagg to off;
+
+-- ensure we get a hash right semi join with SubPlan in hash clauses
+explain (costs off)
+select * from tbl_rs t1
+where (select a from tbl_rs t2
+       where exists (select 1 from
+                     (select (b in (select b from tbl_rs t3)) as c from tbl_rs t4 where t4.a = 1) s
+                     where c in (select t1.a = 1 from tbl_rs t5 union all select true))
+       order by a limit 1) >= 0;
+
+-- and check we get the expected results
+select * from tbl_rs t1
+where (select a from tbl_rs t2
+       where exists (select 1 from
+                     (select (b in (select b from tbl_rs t3)) as c from tbl_rs t4 where t4.a = 1) s
+                     where c in (select t1.a = 1 from tbl_rs t5 union all select true))
+       order by a limit 1) >= 0;
+
+reset enable_nestloop;
+reset enable_hashagg;
+
 --
 -- regression test for bug #13908 (hash join with skew tuples & nbatch increase)
 --
-- 
2.43.0



^ permalink  raw  reply  [nested|flat] 3+ messages in thread

* Re: Wrong results with right-semi-joins
@ 2024-12-09 14:01  Melanie Plageman <[email protected]>
  parent: Richard Guo <[email protected]>
  0 siblings, 0 replies; 3+ messages in thread

From: Melanie Plageman @ 2024-12-09 14:01 UTC (permalink / raw)
  To: Richard Guo <[email protected]>; +Cc: pgsql-hackers

On Tue, Dec 3, 2024 at 3:56 AM Richard Guo <[email protected]> wrote:
>
> I ran into $subject and it can be reproduced with the query below.
>
> create temp table tbl_rs(a int, b int);
> insert into tbl_rs select i, i from generate_series(1,10)i;
> analyze tbl_rs;
>
> set enable_nestloop to off;
> set enable_hashagg to off;
>
> select * from tbl_rs t1
> where (select a from tbl_rs t2
>        where exists (select 1 from
>                      (select (b in (select b from tbl_rs t3)) as c
>                       from tbl_rs t4 where t4.a = 1) s
>                      where c in
>                       (select t1.a = 1 from tbl_rs t5 union all select true))
>        order by a limit 1) >= 0;
>  a | b
> ---+---
>  1 | 1
> (1 row)
>
> The expected output should be 10 rows, not 1.

Thanks for finding and fixing this. Just for my own benefit, could you
explain more about the minimal repro? Specifically, if you just need a
subplan in the hash side of a right semi-join, why do you also need
the outer part of the query that produces the initplan?

 Seq Scan on tbl_rs t1
   Filter: ((SubPlan 3) >= 0)
   SubPlan 3
     ->  Limit
           InitPlan 2
             ->  Hash Right Semi Join

- Melanie






^ permalink  raw  reply  [nested|flat] 3+ messages in thread


end of thread, other threads:[~2024-12-09 14:01 UTC | newest]

Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-11-01 19:46 [PATCH v13 06/18] More refactoring Justin Pryzby <[email protected]>
2024-12-03 08:56 Wrong results with right-semi-joins Richard Guo <[email protected]>
2024-12-09 14:01 ` Re: Wrong results with right-semi-joins Melanie Plageman <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox