agora inbox for [email protected]  

help / color / mirror / Atom feed

[PATCH 06/18] One fewer (not one less)
17+ messages / 2 participants
[nested] [flat]

* [PATCH 06/18] One fewer (not one less)
@ 2021-01-18 20:37  Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 17+ messages in thread

From: Justin Pryzby @ 2021-01-18 20:37 UTC (permalink / raw)

---
 contrib/pageinspect/heapfuncs.c            | 2 +-
 doc/src/sgml/charset.sgml                  | 2 +-
 doc/src/sgml/ref/create_type.sgml          | 2 +-
 doc/src/sgml/rules.sgml                    | 2 +-
 src/backend/access/common/heaptuple.c      | 4 ++--
 src/backend/commands/analyze.c             | 2 +-
 src/backend/utils/adt/jsonpath_exec.c      | 2 +-
 src/backend/utils/adt/selfuncs.c           | 2 +-
 src/backend/utils/cache/catcache.c         | 2 +-
 src/backend/utils/misc/sampling.c          | 2 +-
 src/backend/utils/mmgr/freepage.c          | 2 +-
 src/bin/pgbench/pgbench.c                  | 2 +-
 src/include/pg_config_manual.h             | 4 ++--
 src/interfaces/ecpg/include/sqlda-native.h | 2 +-
 src/test/regress/expected/geometry.out     | 2 +-
 src/test/regress/sql/geometry.sql          | 2 +-
 16 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/contrib/pageinspect/heapfuncs.c b/contrib/pageinspect/heapfuncs.c
index 9abcee32af..f6760eb31e 100644
--- a/contrib/pageinspect/heapfuncs.c
+++ b/contrib/pageinspect/heapfuncs.c
@@ -338,7 +338,7 @@ tuple_data_split_internal(Oid relid, char *tupdata,
 		attr = TupleDescAttr(tupdesc, i);
 
 		/*
-		 * Tuple header can specify less attributes than tuple descriptor as
+		 * Tuple header can specify fewer attributes than tuple descriptor as
 		 * ALTER TABLE ADD COLUMN without DEFAULT keyword does not actually
 		 * change tuples in pages, so attributes with numbers greater than
 		 * (t_infomask2 & HEAP_NATTS_MASK) should be treated as NULL.
diff --git a/doc/src/sgml/charset.sgml b/doc/src/sgml/charset.sgml
index cebc09ef91..1b00e543a6 100644
--- a/doc/src/sgml/charset.sgml
+++ b/doc/src/sgml/charset.sgml
@@ -619,7 +619,7 @@ SELECT * FROM test1 ORDER BY a || b COLLATE "fr_FR";
     name such as <literal>de_DE</literal> can be considered unique
     within a given database even though it would not be unique globally.
     Use of the stripped collation names is recommended, since it will
-    make one less thing you need to change if you decide to change to
+    make one fewer thing you need to change if you decide to change to
     another database encoding.  Note however that the <literal>default</literal>,
     <literal>C</literal>, and <literal>POSIX</literal> collations can be used regardless of
     the database encoding.
diff --git a/doc/src/sgml/ref/create_type.sgml b/doc/src/sgml/ref/create_type.sgml
index 0b24a55505..693423e524 100644
--- a/doc/src/sgml/ref/create_type.sgml
+++ b/doc/src/sgml/ref/create_type.sgml
@@ -867,7 +867,7 @@ CREATE TYPE <replaceable class="parameter">name</replaceable>
    Before <productname>PostgreSQL</productname> version 8.3, the name of
    a generated array type was always exactly the element type's name with one
    underscore character (<literal>_</literal>) prepended.  (Type names were
-   therefore restricted in length to one less character than other names.)
+   therefore restricted in length to one fewer character than other names.)
    While this is still usually the case, the array type name may vary from
    this in case of maximum-length names or collisions with user type names
    that begin with underscore.  Writing code that depends on this convention
diff --git a/doc/src/sgml/rules.sgml b/doc/src/sgml/rules.sgml
index e81addcfa9..aa172d102b 100644
--- a/doc/src/sgml/rules.sgml
+++ b/doc/src/sgml/rules.sgml
@@ -1266,7 +1266,7 @@ CREATE [ OR REPLACE ] RULE <replaceable class="parameter">name</replaceable> AS
 <para>
     The query trees generated from rule actions are thrown into the
     rewrite system again, and maybe more rules get applied resulting
-    in more or less query trees.
+    in additional or fewer query trees.
     So a rule's actions must have either a different
     command type or a different result relation than the rule itself is
     on, otherwise this recursive process will end up in an infinite loop.
diff --git a/src/backend/access/common/heaptuple.c b/src/backend/access/common/heaptuple.c
index 24a27e387d..0b56b0fa5a 100644
--- a/src/backend/access/common/heaptuple.c
+++ b/src/backend/access/common/heaptuple.c
@@ -719,11 +719,11 @@ heap_copytuple_with_tuple(HeapTuple src, HeapTuple dest)
 }
 
 /*
- * Expand a tuple which has less attributes than required. For each attribute
+ * Expand a tuple which has fewer attributes than required. For each attribute
  * not present in the sourceTuple, if there is a missing value that will be
  * used. Otherwise the attribute will be set to NULL.
  *
- * The source tuple must have less attributes than the required number.
+ * The source tuple must have fewer attributes than the required number.
  *
  * Only one of targetHeapTuple and targetMinimalTuple may be supplied. The
  * other argument must be NULL.
diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c
index 7295cf0215..64908ac39c 100644
--- a/src/backend/commands/analyze.c
+++ b/src/backend/commands/analyze.c
@@ -1003,7 +1003,7 @@ examine_attribute(Relation onerel, int attnum, Node *index_expr)
  * As of May 2004 we use a new two-stage method:  Stage one selects up
  * to targrows random blocks (or all blocks, if there aren't so many).
  * Stage two scans these blocks and uses the Vitter algorithm to create
- * a random sample of targrows rows (or less, if there are less in the
+ * a random sample of targrows rows (or fewer, if there are fewer in the
  * sample of blocks).  The two stages are executed simultaneously: each
  * block is processed as soon as stage one returns its number and while
  * the rows are read stage two controls which ones are to be inserted
diff --git a/src/backend/utils/adt/jsonpath_exec.c b/src/backend/utils/adt/jsonpath_exec.c
index 4d185c27b4..078aaef539 100644
--- a/src/backend/utils/adt/jsonpath_exec.c
+++ b/src/backend/utils/adt/jsonpath_exec.c
@@ -263,7 +263,7 @@ static int	compareDatetime(Datum val1, Oid typid1, Datum val2, Oid typid2,
  *		implement @? and @@ operators, which in turn are intended to have an
  *		index support.  Thus, it's desirable to make it easier to achieve
  *		consistency between index scan results and sequential scan results.
- *		So, we throw as less errors as possible.  Regarding this function,
+ *		So, we throw as few errors as possible.  Regarding this function,
  *		such behavior also matches behavior of JSON_EXISTS() clause of
  *		SQL/JSON.  Regarding jsonb_path_match(), this function doesn't have
  *		an analogy in SQL/JSON, so we define its behavior on our own.
diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c
index 47ca4ddbb5..52314d3aa1 100644
--- a/src/backend/utils/adt/selfuncs.c
+++ b/src/backend/utils/adt/selfuncs.c
@@ -645,7 +645,7 @@ scalarineqsel(PlannerInfo *root, Oid operator, bool isgt, bool iseq,
 
 			/*
 			 * The calculation so far gave us a selectivity for the "<=" case.
-			 * We'll have one less tuple for "<" and one additional tuple for
+			 * We'll have one fewer tuple for "<" and one additional tuple for
 			 * ">=", the latter of which we'll reverse the selectivity for
 			 * below, so we can simply subtract one tuple for both cases.  The
 			 * cases that need this adjustment can be identified by iseq being
diff --git a/src/backend/utils/cache/catcache.c b/src/backend/utils/cache/catcache.c
index fa2b49c676..55c9445898 100644
--- a/src/backend/utils/cache/catcache.c
+++ b/src/backend/utils/cache/catcache.c
@@ -1497,7 +1497,7 @@ GetCatCacheHashValue(CatCache *cache,
  *		It doesn't make any sense to specify all of the cache's key columns
  *		here: since the key is unique, there could be at most one match, so
  *		you ought to use SearchCatCache() instead.  Hence this function takes
- *		one less Datum argument than SearchCatCache() does.
+ *		one fewer Datum argument than SearchCatCache() does.
  *
  *		The caller must not modify the list object or the pointed-to tuples,
  *		and must call ReleaseCatCacheList() when done with the list.
diff --git a/src/backend/utils/misc/sampling.c b/src/backend/utils/misc/sampling.c
index 0c327e823f..7348b86682 100644
--- a/src/backend/utils/misc/sampling.c
+++ b/src/backend/utils/misc/sampling.c
@@ -42,7 +42,7 @@ BlockSampler_Init(BlockSampler bs, BlockNumber nblocks, int samplesize,
 	bs->N = nblocks;			/* measured table size */
 
 	/*
-	 * If we decide to reduce samplesize for tables that have less or not much
+	 * If we decide to reduce samplesize for tables that have fewer or not much
 	 * more than samplesize blocks, here is the place to do it.
 	 */
 	bs->n = samplesize;
diff --git a/src/backend/utils/mmgr/freepage.c b/src/backend/utils/mmgr/freepage.c
index e4ee1aab97..10a1effb74 100644
--- a/src/backend/utils/mmgr/freepage.c
+++ b/src/backend/utils/mmgr/freepage.c
@@ -495,7 +495,7 @@ FreePageManagerDump(FreePageManager *fpm)
  * if we search the parent page for the first key greater than or equal to
  * the first key on the current page, the downlink to this page will be either
  * the exact index returned by the search (if the first key decreased)
- * or one less (if the first key increased).
+ * or one fewer (if the first key increased).
  */
 static void
 FreePageBtreeAdjustAncestorKeys(FreePageManager *fpm, FreePageBtree *btp)
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index a4a3f40048..627a244fb7 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -6458,7 +6458,7 @@ threadRun(void *arg)
 
 			/*
 			 * If advanceConnectionState changed client to finished state,
-			 * that's one less client that remains.
+			 * that's one fewer client that remains.
 			 */
 			if (st->state == CSTATE_FINISHED || st->state == CSTATE_ABORTED)
 				remains--;
diff --git a/src/include/pg_config_manual.h b/src/include/pg_config_manual.h
index d27c8601fa..e3d2e751ea 100644
--- a/src/include/pg_config_manual.h
+++ b/src/include/pg_config_manual.h
@@ -21,7 +21,7 @@
 
 /*
  * Maximum length for identifiers (e.g. table names, column names,
- * function names).  Names actually are limited to one less byte than this,
+ * function names).  Names actually are limited to one fewer byte than this,
  * because the length must include a trailing zero byte.
  *
  * Changing this requires an initdb.
@@ -87,7 +87,7 @@
 
 /*
  * MAXPGPATH: standard size of a pathname buffer in PostgreSQL (hence,
- * maximum usable pathname length is one less).
+ * maximum usable pathname length is one fewer).
  *
  * We'd use a standard system header symbol for this, if there weren't
  * so many to choose from: MAXPATHLEN, MAX_PATH, PATH_MAX are all
diff --git a/src/interfaces/ecpg/include/sqlda-native.h b/src/interfaces/ecpg/include/sqlda-native.h
index 67d3c7b4e4..9e73f1f1b1 100644
--- a/src/interfaces/ecpg/include/sqlda-native.h
+++ b/src/interfaces/ecpg/include/sqlda-native.h
@@ -7,7 +7,7 @@
 
 /*
  * Maximum length for identifiers (e.g. table names, column names,
- * function names).  Names actually are limited to one less byte than this,
+ * function names).  Names actually are limited to one fewer byte than this,
  * because the length must include a trailing zero byte.
  *
  * This should be at least as much as NAMEDATALEN of the database the
diff --git a/src/test/regress/expected/geometry.out b/src/test/regress/expected/geometry.out
index 84f7eabb66..9799cfbdbd 100644
--- a/src/test/regress/expected/geometry.out
+++ b/src/test/regress/expected/geometry.out
@@ -4325,7 +4325,7 @@ SELECT f1, polygon(8, f1) FROM CIRCLE_TBL WHERE f1 >= '<(0,0),1>';
  <(100,1),115>  | ((-15,1),(18.6827201635,82.3172798365),(100,116),(181.317279836,82.3172798365),(215,1),(181.317279836,-80.3172798365),(100,-114),(18.6827201635,-80.3172798365))
 (6 rows)
 
--- Too less points error
+-- Too few points error
 SELECT f1, polygon(1, f1) FROM CIRCLE_TBL WHERE f1 >= '<(0,0),1>';
 ERROR:  must request at least 2 points
 -- Zero radius error
diff --git a/src/test/regress/sql/geometry.sql b/src/test/regress/sql/geometry.sql
index 96df0ab05a..b0ab6d03ec 100644
--- a/src/test/regress/sql/geometry.sql
+++ b/src/test/regress/sql/geometry.sql
@@ -424,7 +424,7 @@ SELECT f1, f1::polygon FROM CIRCLE_TBL WHERE f1 >= '<(0,0),1>';
 -- To polygon with less points
 SELECT f1, polygon(8, f1) FROM CIRCLE_TBL WHERE f1 >= '<(0,0),1>';
 
--- Too less points error
+-- Too few points error
 SELECT f1, polygon(1, f1) FROM CIRCLE_TBL WHERE f1 >= '<(0,0),1>';
 
 -- Zero radius error
-- 
2.17.0


--lc9FT7cWel8HagAv
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="0007-function-comment-get_am_name.patch"



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

* [PATCH v3 01/13] BitmapHeapScan begin scan after bitmap creation
@ 2024-02-12 23:50  Melanie Plageman <[email protected]>
  0 siblings, 0 replies; 17+ messages in thread

From: Melanie Plageman @ 2024-02-12 23:50 UTC (permalink / raw)

There is no reason for a BitmapHeapScan to begin the scan of the
underlying table in ExecInitBitmapHeapScan(). Instead, do so after
completing the index scan and building the bitmap.

ExecBitmapHeapInitializeWorker() overwrote the snapshot in the scan
descriptor with the correct one provided by the parallel leader. Since
ExecBitmapHeapInitializeWorker() is now called before the scan
descriptor has been created, save the worker's snapshot in the
BitmapHeapScanState and pass it to table_beginscan_bm().
---
 src/backend/access/table/tableam.c        | 11 ------
 src/backend/executor/nodeBitmapHeapscan.c | 47 ++++++++++++++++++-----
 src/include/access/tableam.h              | 10 ++---
 src/include/nodes/execnodes.h             |  2 +
 4 files changed, 42 insertions(+), 28 deletions(-)

diff --git a/src/backend/access/table/tableam.c b/src/backend/access/table/tableam.c
index 6ed8cca05a1..e78d793f69c 100644
--- a/src/backend/access/table/tableam.c
+++ b/src/backend/access/table/tableam.c
@@ -120,17 +120,6 @@ table_beginscan_catalog(Relation relation, int nkeys, struct ScanKeyData *key)
 											NULL, flags);
 }
 
-void
-table_scan_update_snapshot(TableScanDesc scan, Snapshot snapshot)
-{
-	Assert(IsMVCCSnapshot(snapshot));
-
-	RegisterSnapshot(snapshot);
-	scan->rs_snapshot = snapshot;
-	scan->rs_flags |= SO_TEMP_SNAPSHOT;
-}
-
-
 /* ----------------------------------------------------------------------------
  * Parallel table scan related functions.
  * ----------------------------------------------------------------------------
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index c1e81ebed63..44bf38be3c9 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -181,6 +181,34 @@ BitmapHeapNext(BitmapHeapScanState *node)
 			}
 #endif							/* USE_PREFETCH */
 		}
+
+		/*
+		 * If this is the first scan of the underlying table, create the table
+		 * scan descriptor and begin the scan.
+		 */
+		if (!scan)
+		{
+			Snapshot	snapshot = node->ss.ps.state->es_snapshot;
+			uint32		extra_flags = 0;
+
+			/*
+			 * Parallel workers must use the snapshot initialized by the
+			 * parallel leader.
+			 */
+			if (node->worker_snapshot)
+			{
+				snapshot = node->worker_snapshot;
+				extra_flags |= SO_TEMP_SNAPSHOT;
+			}
+
+			scan = node->ss.ss_currentScanDesc = table_beginscan_bm(
+																	node->ss.ss_currentRelation,
+																	snapshot,
+																	0,
+																	NULL,
+																	extra_flags);
+		}
+
 		node->initialized = true;
 	}
 
@@ -604,7 +632,8 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
 	PlanState  *outerPlan = outerPlanState(node);
 
 	/* rescan to release any page pin */
-	table_rescan(node->ss.ss_currentScanDesc, NULL);
+	if (node->ss.ss_currentScanDesc)
+		table_rescan(node->ss.ss_currentScanDesc, NULL);
 
 	/* release bitmaps and buffers if any */
 	if (node->tbmiterator)
@@ -681,7 +710,9 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node)
 	/*
 	 * close heap scan
 	 */
-	table_endscan(scanDesc);
+	if (scanDesc)
+		table_endscan(scanDesc);
+
 }
 
 /* ----------------------------------------------------------------
@@ -739,6 +770,7 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
 	 */
 	scanstate->can_skip_fetch = (node->scan.plan.qual == NIL &&
 								 node->scan.plan.targetlist == NIL);
+	scanstate->worker_snapshot = NULL;
 
 	/*
 	 * Miscellaneous initialization
@@ -787,11 +819,6 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
 
 	scanstate->ss.ss_currentRelation = currentRelation;
 
-	scanstate->ss.ss_currentScanDesc = table_beginscan_bm(currentRelation,
-														  estate->es_snapshot,
-														  0,
-														  NULL);
-
 	/*
 	 * all done.
 	 */
@@ -930,13 +957,13 @@ ExecBitmapHeapInitializeWorker(BitmapHeapScanState *node,
 							   ParallelWorkerContext *pwcxt)
 {
 	ParallelBitmapHeapState *pstate;
-	Snapshot	snapshot;
 
 	Assert(node->ss.ps.state->es_query_dsa != NULL);
 
 	pstate = shm_toc_lookup(pwcxt->toc, node->ss.ps.plan->plan_node_id, false);
 	node->pstate = pstate;
 
-	snapshot = RestoreSnapshot(pstate->phs_snapshot_data);
-	table_scan_update_snapshot(node->ss.ss_currentScanDesc, snapshot);
+	node->worker_snapshot = RestoreSnapshot(pstate->phs_snapshot_data);
+	Assert(IsMVCCSnapshot(node->worker_snapshot));
+	RegisterSnapshot(node->worker_snapshot);
 }
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index 5f8474871d2..5375dd7150f 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -944,9 +944,10 @@ table_beginscan_strat(Relation rel, Snapshot snapshot,
  */
 static inline TableScanDesc
 table_beginscan_bm(Relation rel, Snapshot snapshot,
-				   int nkeys, struct ScanKeyData *key)
+				   int nkeys, struct ScanKeyData *key,
+				   uint32 extra_flags)
 {
-	uint32		flags = SO_TYPE_BITMAPSCAN | SO_ALLOW_PAGEMODE;
+	uint32		flags = SO_TYPE_BITMAPSCAN | SO_ALLOW_PAGEMODE | extra_flags;
 
 	return rel->rd_tableam->scan_begin(rel, snapshot, nkeys, key, NULL, flags);
 }
@@ -1038,11 +1039,6 @@ table_rescan_set_params(TableScanDesc scan, struct ScanKeyData *key,
 										 allow_pagemode);
 }
 
-/*
- * Update snapshot used by the scan.
- */
-extern void table_scan_update_snapshot(TableScanDesc scan, Snapshot snapshot);
-
 /*
  * Return next tuple from `scan`, store in slot.
  */
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 444a5f0fd57..00c75fb10e2 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -1726,6 +1726,7 @@ typedef struct ParallelBitmapHeapState
  *		shared_tbmiterator	   shared iterator
  *		shared_prefetch_iterator shared iterator for prefetching
  *		pstate			   shared state for parallel bitmap scan
+ *		worker_snapshot	   snapshot for parallel worker
  * ----------------
  */
 typedef struct BitmapHeapScanState
@@ -1750,6 +1751,7 @@ typedef struct BitmapHeapScanState
 	TBMSharedIterator *shared_tbmiterator;
 	TBMSharedIterator *shared_prefetch_iterator;
 	ParallelBitmapHeapState *pstate;
+	Snapshot	worker_snapshot;
 } BitmapHeapScanState;
 
 /* ----------------
-- 
2.37.2


--fa5b4qdlc6zixc7z
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v3-0002-BitmapHeapScan-set-can_skip_fetch-later.patch"



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

* [PATCH v4 01/14] BitmapHeapScan begin scan after bitmap creation
@ 2024-02-12 23:50  Melanie Plageman <[email protected]>
  0 siblings, 0 replies; 17+ messages in thread

From: Melanie Plageman @ 2024-02-12 23:50 UTC (permalink / raw)

There is no reason for a BitmapHeapScan to begin the scan of the
underlying table in ExecInitBitmapHeapScan(). Instead, do so after
completing the index scan and building the bitmap.

ExecBitmapHeapInitializeWorker() overwrote the snapshot in the scan
descriptor with the correct one provided by the parallel leader. Since
ExecBitmapHeapInitializeWorker() is now called before the scan
descriptor has been created, save the worker's snapshot in the
BitmapHeapScanState and pass it to table_beginscan_bm().
---
 src/backend/access/table/tableam.c        | 11 ------
 src/backend/executor/nodeBitmapHeapscan.c | 47 ++++++++++++++++++-----
 src/include/access/tableam.h              | 10 ++---
 src/include/nodes/execnodes.h             |  2 +
 4 files changed, 42 insertions(+), 28 deletions(-)

diff --git a/src/backend/access/table/tableam.c b/src/backend/access/table/tableam.c
index 6ed8cca05a1..e78d793f69c 100644
--- a/src/backend/access/table/tableam.c
+++ b/src/backend/access/table/tableam.c
@@ -120,17 +120,6 @@ table_beginscan_catalog(Relation relation, int nkeys, struct ScanKeyData *key)
 											NULL, flags);
 }
 
-void
-table_scan_update_snapshot(TableScanDesc scan, Snapshot snapshot)
-{
-	Assert(IsMVCCSnapshot(snapshot));
-
-	RegisterSnapshot(snapshot);
-	scan->rs_snapshot = snapshot;
-	scan->rs_flags |= SO_TEMP_SNAPSHOT;
-}
-
-
 /* ----------------------------------------------------------------------------
  * Parallel table scan related functions.
  * ----------------------------------------------------------------------------
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index c1e81ebed63..44bf38be3c9 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -181,6 +181,34 @@ BitmapHeapNext(BitmapHeapScanState *node)
 			}
 #endif							/* USE_PREFETCH */
 		}
+
+		/*
+		 * If this is the first scan of the underlying table, create the table
+		 * scan descriptor and begin the scan.
+		 */
+		if (!scan)
+		{
+			Snapshot	snapshot = node->ss.ps.state->es_snapshot;
+			uint32		extra_flags = 0;
+
+			/*
+			 * Parallel workers must use the snapshot initialized by the
+			 * parallel leader.
+			 */
+			if (node->worker_snapshot)
+			{
+				snapshot = node->worker_snapshot;
+				extra_flags |= SO_TEMP_SNAPSHOT;
+			}
+
+			scan = node->ss.ss_currentScanDesc = table_beginscan_bm(
+																	node->ss.ss_currentRelation,
+																	snapshot,
+																	0,
+																	NULL,
+																	extra_flags);
+		}
+
 		node->initialized = true;
 	}
 
@@ -604,7 +632,8 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
 	PlanState  *outerPlan = outerPlanState(node);
 
 	/* rescan to release any page pin */
-	table_rescan(node->ss.ss_currentScanDesc, NULL);
+	if (node->ss.ss_currentScanDesc)
+		table_rescan(node->ss.ss_currentScanDesc, NULL);
 
 	/* release bitmaps and buffers if any */
 	if (node->tbmiterator)
@@ -681,7 +710,9 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node)
 	/*
 	 * close heap scan
 	 */
-	table_endscan(scanDesc);
+	if (scanDesc)
+		table_endscan(scanDesc);
+
 }
 
 /* ----------------------------------------------------------------
@@ -739,6 +770,7 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
 	 */
 	scanstate->can_skip_fetch = (node->scan.plan.qual == NIL &&
 								 node->scan.plan.targetlist == NIL);
+	scanstate->worker_snapshot = NULL;
 
 	/*
 	 * Miscellaneous initialization
@@ -787,11 +819,6 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
 
 	scanstate->ss.ss_currentRelation = currentRelation;
 
-	scanstate->ss.ss_currentScanDesc = table_beginscan_bm(currentRelation,
-														  estate->es_snapshot,
-														  0,
-														  NULL);
-
 	/*
 	 * all done.
 	 */
@@ -930,13 +957,13 @@ ExecBitmapHeapInitializeWorker(BitmapHeapScanState *node,
 							   ParallelWorkerContext *pwcxt)
 {
 	ParallelBitmapHeapState *pstate;
-	Snapshot	snapshot;
 
 	Assert(node->ss.ps.state->es_query_dsa != NULL);
 
 	pstate = shm_toc_lookup(pwcxt->toc, node->ss.ps.plan->plan_node_id, false);
 	node->pstate = pstate;
 
-	snapshot = RestoreSnapshot(pstate->phs_snapshot_data);
-	table_scan_update_snapshot(node->ss.ss_currentScanDesc, snapshot);
+	node->worker_snapshot = RestoreSnapshot(pstate->phs_snapshot_data);
+	Assert(IsMVCCSnapshot(node->worker_snapshot));
+	RegisterSnapshot(node->worker_snapshot);
 }
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index 5f8474871d2..5375dd7150f 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -944,9 +944,10 @@ table_beginscan_strat(Relation rel, Snapshot snapshot,
  */
 static inline TableScanDesc
 table_beginscan_bm(Relation rel, Snapshot snapshot,
-				   int nkeys, struct ScanKeyData *key)
+				   int nkeys, struct ScanKeyData *key,
+				   uint32 extra_flags)
 {
-	uint32		flags = SO_TYPE_BITMAPSCAN | SO_ALLOW_PAGEMODE;
+	uint32		flags = SO_TYPE_BITMAPSCAN | SO_ALLOW_PAGEMODE | extra_flags;
 
 	return rel->rd_tableam->scan_begin(rel, snapshot, nkeys, key, NULL, flags);
 }
@@ -1038,11 +1039,6 @@ table_rescan_set_params(TableScanDesc scan, struct ScanKeyData *key,
 										 allow_pagemode);
 }
 
-/*
- * Update snapshot used by the scan.
- */
-extern void table_scan_update_snapshot(TableScanDesc scan, Snapshot snapshot);
-
 /*
  * Return next tuple from `scan`, store in slot.
  */
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 444a5f0fd57..00c75fb10e2 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -1726,6 +1726,7 @@ typedef struct ParallelBitmapHeapState
  *		shared_tbmiterator	   shared iterator
  *		shared_prefetch_iterator shared iterator for prefetching
  *		pstate			   shared state for parallel bitmap scan
+ *		worker_snapshot	   snapshot for parallel worker
  * ----------------
  */
 typedef struct BitmapHeapScanState
@@ -1750,6 +1751,7 @@ typedef struct BitmapHeapScanState
 	TBMSharedIterator *shared_tbmiterator;
 	TBMSharedIterator *shared_prefetch_iterator;
 	ParallelBitmapHeapState *pstate;
+	Snapshot	worker_snapshot;
 } BitmapHeapScanState;
 
 /* ----------------
-- 
2.37.2


--5aaqsqqhbq27q3qo
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v4-0002-BitmapHeapScan-set-can_skip_fetch-later.patch"



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

* [PATCH v5 01/14] BitmapHeapScan begin scan after bitmap creation
@ 2024-02-12 23:50  Melanie Plageman <[email protected]>
  0 siblings, 0 replies; 17+ messages in thread

From: Melanie Plageman @ 2024-02-12 23:50 UTC (permalink / raw)

There is no reason for a BitmapHeapScan to begin the scan of the
underlying table in ExecInitBitmapHeapScan(). Instead, do so after
completing the index scan and building the bitmap.

ExecBitmapHeapInitializeWorker() overwrote the snapshot in the scan
descriptor with the correct one provided by the parallel leader. Since
ExecBitmapHeapInitializeWorker() is now called before the scan
descriptor has been created, save the worker's snapshot in the
BitmapHeapScanState and pass it to table_beginscan_bm().
---
 src/backend/access/table/tableam.c        | 11 ------
 src/backend/executor/nodeBitmapHeapscan.c | 47 ++++++++++++++++++-----
 src/include/access/tableam.h              | 10 ++---
 src/include/nodes/execnodes.h             |  2 +
 4 files changed, 42 insertions(+), 28 deletions(-)

diff --git a/src/backend/access/table/tableam.c b/src/backend/access/table/tableam.c
index 6ed8cca05a1..e78d793f69c 100644
--- a/src/backend/access/table/tableam.c
+++ b/src/backend/access/table/tableam.c
@@ -120,17 +120,6 @@ table_beginscan_catalog(Relation relation, int nkeys, struct ScanKeyData *key)
 											NULL, flags);
 }
 
-void
-table_scan_update_snapshot(TableScanDesc scan, Snapshot snapshot)
-{
-	Assert(IsMVCCSnapshot(snapshot));
-
-	RegisterSnapshot(snapshot);
-	scan->rs_snapshot = snapshot;
-	scan->rs_flags |= SO_TEMP_SNAPSHOT;
-}
-
-
 /* ----------------------------------------------------------------------------
  * Parallel table scan related functions.
  * ----------------------------------------------------------------------------
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index c1e81ebed63..44bf38be3c9 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -181,6 +181,34 @@ BitmapHeapNext(BitmapHeapScanState *node)
 			}
 #endif							/* USE_PREFETCH */
 		}
+
+		/*
+		 * If this is the first scan of the underlying table, create the table
+		 * scan descriptor and begin the scan.
+		 */
+		if (!scan)
+		{
+			Snapshot	snapshot = node->ss.ps.state->es_snapshot;
+			uint32		extra_flags = 0;
+
+			/*
+			 * Parallel workers must use the snapshot initialized by the
+			 * parallel leader.
+			 */
+			if (node->worker_snapshot)
+			{
+				snapshot = node->worker_snapshot;
+				extra_flags |= SO_TEMP_SNAPSHOT;
+			}
+
+			scan = node->ss.ss_currentScanDesc = table_beginscan_bm(
+																	node->ss.ss_currentRelation,
+																	snapshot,
+																	0,
+																	NULL,
+																	extra_flags);
+		}
+
 		node->initialized = true;
 	}
 
@@ -604,7 +632,8 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
 	PlanState  *outerPlan = outerPlanState(node);
 
 	/* rescan to release any page pin */
-	table_rescan(node->ss.ss_currentScanDesc, NULL);
+	if (node->ss.ss_currentScanDesc)
+		table_rescan(node->ss.ss_currentScanDesc, NULL);
 
 	/* release bitmaps and buffers if any */
 	if (node->tbmiterator)
@@ -681,7 +710,9 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node)
 	/*
 	 * close heap scan
 	 */
-	table_endscan(scanDesc);
+	if (scanDesc)
+		table_endscan(scanDesc);
+
 }
 
 /* ----------------------------------------------------------------
@@ -739,6 +770,7 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
 	 */
 	scanstate->can_skip_fetch = (node->scan.plan.qual == NIL &&
 								 node->scan.plan.targetlist == NIL);
+	scanstate->worker_snapshot = NULL;
 
 	/*
 	 * Miscellaneous initialization
@@ -787,11 +819,6 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
 
 	scanstate->ss.ss_currentRelation = currentRelation;
 
-	scanstate->ss.ss_currentScanDesc = table_beginscan_bm(currentRelation,
-														  estate->es_snapshot,
-														  0,
-														  NULL);
-
 	/*
 	 * all done.
 	 */
@@ -930,13 +957,13 @@ ExecBitmapHeapInitializeWorker(BitmapHeapScanState *node,
 							   ParallelWorkerContext *pwcxt)
 {
 	ParallelBitmapHeapState *pstate;
-	Snapshot	snapshot;
 
 	Assert(node->ss.ps.state->es_query_dsa != NULL);
 
 	pstate = shm_toc_lookup(pwcxt->toc, node->ss.ps.plan->plan_node_id, false);
 	node->pstate = pstate;
 
-	snapshot = RestoreSnapshot(pstate->phs_snapshot_data);
-	table_scan_update_snapshot(node->ss.ss_currentScanDesc, snapshot);
+	node->worker_snapshot = RestoreSnapshot(pstate->phs_snapshot_data);
+	Assert(IsMVCCSnapshot(node->worker_snapshot));
+	RegisterSnapshot(node->worker_snapshot);
 }
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index 5f8474871d2..5375dd7150f 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -944,9 +944,10 @@ table_beginscan_strat(Relation rel, Snapshot snapshot,
  */
 static inline TableScanDesc
 table_beginscan_bm(Relation rel, Snapshot snapshot,
-				   int nkeys, struct ScanKeyData *key)
+				   int nkeys, struct ScanKeyData *key,
+				   uint32 extra_flags)
 {
-	uint32		flags = SO_TYPE_BITMAPSCAN | SO_ALLOW_PAGEMODE;
+	uint32		flags = SO_TYPE_BITMAPSCAN | SO_ALLOW_PAGEMODE | extra_flags;
 
 	return rel->rd_tableam->scan_begin(rel, snapshot, nkeys, key, NULL, flags);
 }
@@ -1038,11 +1039,6 @@ table_rescan_set_params(TableScanDesc scan, struct ScanKeyData *key,
 										 allow_pagemode);
 }
 
-/*
- * Update snapshot used by the scan.
- */
-extern void table_scan_update_snapshot(TableScanDesc scan, Snapshot snapshot);
-
 /*
  * Return next tuple from `scan`, store in slot.
  */
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 444a5f0fd57..00c75fb10e2 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -1726,6 +1726,7 @@ typedef struct ParallelBitmapHeapState
  *		shared_tbmiterator	   shared iterator
  *		shared_prefetch_iterator shared iterator for prefetching
  *		pstate			   shared state for parallel bitmap scan
+ *		worker_snapshot	   snapshot for parallel worker
  * ----------------
  */
 typedef struct BitmapHeapScanState
@@ -1750,6 +1751,7 @@ typedef struct BitmapHeapScanState
 	TBMSharedIterator *shared_tbmiterator;
 	TBMSharedIterator *shared_prefetch_iterator;
 	ParallelBitmapHeapState *pstate;
+	Snapshot	worker_snapshot;
 } BitmapHeapScanState;
 
 /* ----------------
-- 
2.37.2


--6kjpcnqyi64ibp5i
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v5-0002-BitmapHeapScan-set-can_skip_fetch-later.patch"



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

* [PATCH v13 01/16] BitmapHeapScan begin scan after bitmap creation
@ 2024-02-12 23:50  Melanie Plageman <[email protected]>
  0 siblings, 0 replies; 17+ messages in thread

From: Melanie Plageman @ 2024-02-12 23:50 UTC (permalink / raw)

There is no reason for a BitmapHeapScan to begin the scan of the
underlying table in ExecInitBitmapHeapScan(). Instead, do so after
completing the index scan and building the bitmap.
---
 src/backend/access/table/tableam.c        |  1 -
 src/backend/executor/nodeBitmapHeapscan.c | 26 +++++++++++++++++------
 2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/src/backend/access/table/tableam.c b/src/backend/access/table/tableam.c
index 805d222ceb..b0f61c65f3 100644
--- a/src/backend/access/table/tableam.c
+++ b/src/backend/access/table/tableam.c
@@ -120,7 +120,6 @@ table_beginscan_catalog(Relation relation, int nkeys, struct ScanKeyData *key)
 											NULL, flags);
 }
 
-
 /* ----------------------------------------------------------------------------
  * Parallel table scan related functions.
  * ----------------------------------------------------------------------------
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index cee7f45aab..93fdcd226b 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -178,6 +178,20 @@ BitmapHeapNext(BitmapHeapScanState *node)
 			}
 #endif							/* USE_PREFETCH */
 		}
+
+		/*
+		 * If this is the first scan of the underlying table, create the table
+		 * scan descriptor and begin the scan.
+		 */
+		if (!scan)
+		{
+			scan = node->ss.ss_currentScanDesc = table_beginscan_bm(
+																	node->ss.ss_currentRelation,
+																	node->ss.ps.state->es_snapshot,
+																	0,
+																	NULL);
+		}
+
 		node->initialized = true;
 	}
 
@@ -601,7 +615,8 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
 	PlanState  *outerPlan = outerPlanState(node);
 
 	/* rescan to release any page pin */
-	table_rescan(node->ss.ss_currentScanDesc, NULL);
+	if (node->ss.ss_currentScanDesc)
+		table_rescan(node->ss.ss_currentScanDesc, NULL);
 
 	/* release bitmaps and buffers if any */
 	if (node->tbmiterator)
@@ -678,7 +693,9 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node)
 	/*
 	 * close heap scan
 	 */
-	table_endscan(scanDesc);
+	if (scanDesc)
+		table_endscan(scanDesc);
+
 }
 
 /* ----------------------------------------------------------------
@@ -783,11 +800,6 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
 
 	scanstate->ss.ss_currentRelation = currentRelation;
 
-	scanstate->ss.ss_currentScanDesc = table_beginscan_bm(currentRelation,
-														  estate->es_snapshot,
-														  0,
-														  NULL);
-
 	/*
 	 * all done.
 	 */
-- 
2.40.1


--cuuqjeyokkhgd736
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v13-0002-BitmapHeapScan-set-can_skip_fetch-later.patch"



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

* [PATCH v15 01/13] BitmapHeapScan begin scan after bitmap creation
@ 2024-02-12 23:50  Melanie Plageman <[email protected]>
  0 siblings, 0 replies; 17+ messages in thread

From: Melanie Plageman @ 2024-02-12 23:50 UTC (permalink / raw)

It makes more sense for BitmapHeapScan to scan the index, build the
bitmap, and then begin the scan on the underlying table.

This is primarily a cosmetic change for now, but later commits will pass
parameters to table_beginscan_bm() that are unavailable in
ExecInitBitmapHeapScan().

Author: Melanie Plageman
Reviewed-by: Andres Freund, Heikki Linnakangas, Tomas Vondra
Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com
---
 src/backend/executor/nodeBitmapHeapscan.c | 27 +++++++++++++++++------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index cee7f45aab..c8c466e3c5 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -178,6 +178,21 @@ BitmapHeapNext(BitmapHeapScanState *node)
 			}
 #endif							/* USE_PREFETCH */
 		}
+
+		/*
+		 * If this is the first scan of the underlying table, create the table
+		 * scan descriptor and begin the scan.
+		 */
+		if (!scan)
+		{
+			scan = table_beginscan_bm(node->ss.ss_currentRelation,
+									  node->ss.ps.state->es_snapshot,
+									  0,
+									  NULL);
+
+			node->ss.ss_currentScanDesc = scan;
+		}
+
 		node->initialized = true;
 	}
 
@@ -601,7 +616,8 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
 	PlanState  *outerPlan = outerPlanState(node);
 
 	/* rescan to release any page pin */
-	table_rescan(node->ss.ss_currentScanDesc, NULL);
+	if (node->ss.ss_currentScanDesc)
+		table_rescan(node->ss.ss_currentScanDesc, NULL);
 
 	/* release bitmaps and buffers if any */
 	if (node->tbmiterator)
@@ -678,7 +694,9 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node)
 	/*
 	 * close heap scan
 	 */
-	table_endscan(scanDesc);
+	if (scanDesc)
+		table_endscan(scanDesc);
+
 }
 
 /* ----------------------------------------------------------------
@@ -783,11 +801,6 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
 
 	scanstate->ss.ss_currentRelation = currentRelation;
 
-	scanstate->ss.ss_currentScanDesc = table_beginscan_bm(currentRelation,
-														  estate->es_snapshot,
-														  0,
-														  NULL);
-
 	/*
 	 * all done.
 	 */
-- 
2.40.1


--pbix6fw3h4kvmjae
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v15-0002-BitmapHeapScan-set-can_skip_fetch-later.patch"



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

* [PATCH v16 01/18] BitmapHeapScan begin scan after bitmap creation
@ 2024-02-12 23:50  Melanie Plageman <[email protected]>
  0 siblings, 0 replies; 17+ messages in thread

From: Melanie Plageman @ 2024-02-12 23:50 UTC (permalink / raw)

It makes more sense for BitmapHeapScan to scan the index, build the
bitmap, and then begin the scan on the underlying table.

This is primarily a cosmetic change for now, but later commits will pass
parameters to table_beginscan_bm() that are unavailable in
ExecInitBitmapHeapScan().

Author: Melanie Plageman
Reviewed-by: Andres Freund, Heikki Linnakangas, Tomas Vondra
Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com
---
 src/backend/executor/nodeBitmapHeapscan.c | 27 +++++++++++++++++------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index cee7f45aab..c8c466e3c5 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -178,6 +178,21 @@ BitmapHeapNext(BitmapHeapScanState *node)
 			}
 #endif							/* USE_PREFETCH */
 		}
+
+		/*
+		 * If this is the first scan of the underlying table, create the table
+		 * scan descriptor and begin the scan.
+		 */
+		if (!scan)
+		{
+			scan = table_beginscan_bm(node->ss.ss_currentRelation,
+									  node->ss.ps.state->es_snapshot,
+									  0,
+									  NULL);
+
+			node->ss.ss_currentScanDesc = scan;
+		}
+
 		node->initialized = true;
 	}
 
@@ -601,7 +616,8 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
 	PlanState  *outerPlan = outerPlanState(node);
 
 	/* rescan to release any page pin */
-	table_rescan(node->ss.ss_currentScanDesc, NULL);
+	if (node->ss.ss_currentScanDesc)
+		table_rescan(node->ss.ss_currentScanDesc, NULL);
 
 	/* release bitmaps and buffers if any */
 	if (node->tbmiterator)
@@ -678,7 +694,9 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node)
 	/*
 	 * close heap scan
 	 */
-	table_endscan(scanDesc);
+	if (scanDesc)
+		table_endscan(scanDesc);
+
 }
 
 /* ----------------------------------------------------------------
@@ -783,11 +801,6 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
 
 	scanstate->ss.ss_currentRelation = currentRelation;
 
-	scanstate->ss.ss_currentScanDesc = table_beginscan_bm(currentRelation,
-														  estate->es_snapshot,
-														  0,
-														  NULL);
-
 	/*
 	 * all done.
 	 */
-- 
2.40.1


--e3xl7h75mzefinno
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v16-0002-BitmapHeapScan-set-can_skip_fetch-later.patch"



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

* [PATCH v17 1/9] BitmapHeapScan: begin scan after bitmap creation
@ 2024-02-12 23:50  Melanie Plageman <[email protected]>
  0 siblings, 0 replies; 17+ messages in thread

From: Melanie Plageman @ 2024-02-12 23:50 UTC (permalink / raw)

It makes more sense for BitmapHeapScan to scan the index, build the
bitmap, and only then begin the scan on the underlying table.

This is mostly a cosmetic change for now, but later commits will need
to pass parameters to table_beginscan_bm() that are unavailable in
ExecInitBitmapHeapScan().

Author: Melanie Plageman
Reviewed-by: Tomas Vondra, Andres Freund, Heikki Linnakangas
Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com
---
 src/backend/executor/nodeBitmapHeapscan.c | 27 +++++++++++++++++------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index cee7f45aabe..c8c466e3c5c 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -178,6 +178,21 @@ BitmapHeapNext(BitmapHeapScanState *node)
 			}
 #endif							/* USE_PREFETCH */
 		}
+
+		/*
+		 * If this is the first scan of the underlying table, create the table
+		 * scan descriptor and begin the scan.
+		 */
+		if (!scan)
+		{
+			scan = table_beginscan_bm(node->ss.ss_currentRelation,
+									  node->ss.ps.state->es_snapshot,
+									  0,
+									  NULL);
+
+			node->ss.ss_currentScanDesc = scan;
+		}
+
 		node->initialized = true;
 	}
 
@@ -601,7 +616,8 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
 	PlanState  *outerPlan = outerPlanState(node);
 
 	/* rescan to release any page pin */
-	table_rescan(node->ss.ss_currentScanDesc, NULL);
+	if (node->ss.ss_currentScanDesc)
+		table_rescan(node->ss.ss_currentScanDesc, NULL);
 
 	/* release bitmaps and buffers if any */
 	if (node->tbmiterator)
@@ -678,7 +694,9 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node)
 	/*
 	 * close heap scan
 	 */
-	table_endscan(scanDesc);
+	if (scanDesc)
+		table_endscan(scanDesc);
+
 }
 
 /* ----------------------------------------------------------------
@@ -783,11 +801,6 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
 
 	scanstate->ss.ss_currentRelation = currentRelation;
 
-	scanstate->ss.ss_currentScanDesc = table_beginscan_bm(currentRelation,
-														  estate->es_snapshot,
-														  0,
-														  NULL);
-
 	/*
 	 * all done.
 	 */
-- 
2.40.1


--etwsg7gfkf6ny266
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v17-0002-BitmapHeapScan-postpone-setting-can_skip_fetch.patch"



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

* [PATCH v18 01/10] BitmapHeapScan: begin scan after bitmap creation
@ 2024-02-12 23:50  Melanie Plageman <[email protected]>
  0 siblings, 0 replies; 17+ messages in thread

From: Melanie Plageman @ 2024-02-12 23:50 UTC (permalink / raw)

It makes more sense for BitmapHeapScan to scan the index, build the
bitmap, and only then begin the scan on the underlying table.

This is mostly a cosmetic change for now, but later commits will need
to pass parameters to table_beginscan_bm() that are unavailable in
ExecInitBitmapHeapScan().

Author: Melanie Plageman
Reviewed-by: Tomas Vondra, Andres Freund, Heikki Linnakangas
Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com
---
 src/backend/executor/nodeBitmapHeapscan.c | 27 +++++++++++++++++------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index cee7f45aabe..c8c466e3c5c 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -178,6 +178,21 @@ BitmapHeapNext(BitmapHeapScanState *node)
 			}
 #endif							/* USE_PREFETCH */
 		}
+
+		/*
+		 * If this is the first scan of the underlying table, create the table
+		 * scan descriptor and begin the scan.
+		 */
+		if (!scan)
+		{
+			scan = table_beginscan_bm(node->ss.ss_currentRelation,
+									  node->ss.ps.state->es_snapshot,
+									  0,
+									  NULL);
+
+			node->ss.ss_currentScanDesc = scan;
+		}
+
 		node->initialized = true;
 	}
 
@@ -601,7 +616,8 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
 	PlanState  *outerPlan = outerPlanState(node);
 
 	/* rescan to release any page pin */
-	table_rescan(node->ss.ss_currentScanDesc, NULL);
+	if (node->ss.ss_currentScanDesc)
+		table_rescan(node->ss.ss_currentScanDesc, NULL);
 
 	/* release bitmaps and buffers if any */
 	if (node->tbmiterator)
@@ -678,7 +694,9 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node)
 	/*
 	 * close heap scan
 	 */
-	table_endscan(scanDesc);
+	if (scanDesc)
+		table_endscan(scanDesc);
+
 }
 
 /* ----------------------------------------------------------------
@@ -783,11 +801,6 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
 
 	scanstate->ss.ss_currentRelation = currentRelation;
 
-	scanstate->ss.ss_currentScanDesc = table_beginscan_bm(currentRelation,
-														  estate->es_snapshot,
-														  0,
-														  NULL);
-
 	/*
 	 * all done.
 	 */
-- 
2.40.1


--zmcd37za4qx3kv73
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v18-0002-BitmapHeapScan-postpone-setting-can_skip_fetch.patch"



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

* [PATCH v19 01/21] BitmapHeapScan: begin scan after bitmap creation
@ 2024-02-12 23:50  Melanie Plageman <[email protected]>
  0 siblings, 0 replies; 17+ messages in thread

From: Melanie Plageman @ 2024-02-12 23:50 UTC (permalink / raw)

It makes more sense for BitmapHeapScan to scan the index, build the
bitmap, and only then begin the scan on the underlying table.

This is mostly a cosmetic change for now, but later commits will need
to pass parameters to table_beginscan_bm() that are unavailable in
ExecInitBitmapHeapScan().

Author: Melanie Plageman
Reviewed-by: Tomas Vondra, Andres Freund, Heikki Linnakangas
Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com
---
 src/backend/executor/nodeBitmapHeapscan.c | 27 +++++++++++++++++------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index cee7f45aabe..c8c466e3c5c 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -178,6 +178,21 @@ BitmapHeapNext(BitmapHeapScanState *node)
 			}
 #endif							/* USE_PREFETCH */
 		}
+
+		/*
+		 * If this is the first scan of the underlying table, create the table
+		 * scan descriptor and begin the scan.
+		 */
+		if (!scan)
+		{
+			scan = table_beginscan_bm(node->ss.ss_currentRelation,
+									  node->ss.ps.state->es_snapshot,
+									  0,
+									  NULL);
+
+			node->ss.ss_currentScanDesc = scan;
+		}
+
 		node->initialized = true;
 	}
 
@@ -601,7 +616,8 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
 	PlanState  *outerPlan = outerPlanState(node);
 
 	/* rescan to release any page pin */
-	table_rescan(node->ss.ss_currentScanDesc, NULL);
+	if (node->ss.ss_currentScanDesc)
+		table_rescan(node->ss.ss_currentScanDesc, NULL);
 
 	/* release bitmaps and buffers if any */
 	if (node->tbmiterator)
@@ -678,7 +694,9 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node)
 	/*
 	 * close heap scan
 	 */
-	table_endscan(scanDesc);
+	if (scanDesc)
+		table_endscan(scanDesc);
+
 }
 
 /* ----------------------------------------------------------------
@@ -783,11 +801,6 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
 
 	scanstate->ss.ss_currentRelation = currentRelation;
 
-	scanstate->ss.ss_currentScanDesc = table_beginscan_bm(currentRelation,
-														  estate->es_snapshot,
-														  0,
-														  NULL);
-
 	/*
 	 * all done.
 	 */
-- 
2.40.1


--clv5reqgj4vdihqz
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v19-0002-BitmapHeapScan-postpone-setting-can_skip_fetch.patch"



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

* [PATCH v6 01/14] BitmapHeapScan begin scan after bitmap creation
@ 2024-02-12 23:50  Melanie Plageman <[email protected]>
  0 siblings, 0 replies; 17+ messages in thread

From: Melanie Plageman @ 2024-02-12 23:50 UTC (permalink / raw)

There is no reason for a BitmapHeapScan to begin the scan of the
underlying table in ExecInitBitmapHeapScan(). Instead, do so after
completing the index scan and building the bitmap.
---
 src/backend/access/table/tableam.c        |  1 -
 src/backend/executor/nodeBitmapHeapscan.c | 26 +++++++++++++++++------
 2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/src/backend/access/table/tableam.c b/src/backend/access/table/tableam.c
index e57a0b7ea31..e78d793f69c 100644
--- a/src/backend/access/table/tableam.c
+++ b/src/backend/access/table/tableam.c
@@ -120,7 +120,6 @@ table_beginscan_catalog(Relation relation, int nkeys, struct ScanKeyData *key)
 											NULL, flags);
 }
 
-
 /* ----------------------------------------------------------------------------
  * Parallel table scan related functions.
  * ----------------------------------------------------------------------------
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index ca548e44eb4..6d756001fdb 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -178,6 +178,20 @@ BitmapHeapNext(BitmapHeapScanState *node)
 			}
 #endif							/* USE_PREFETCH */
 		}
+
+		/*
+		 * If this is the first scan of the underlying table, create the table
+		 * scan descriptor and begin the scan.
+		 */
+		if (!scan)
+		{
+			scan = node->ss.ss_currentScanDesc = table_beginscan_bm(
+																	node->ss.ss_currentRelation,
+																	node->ss.ps.state->es_snapshot,
+																	0,
+																	NULL);
+		}
+
 		node->initialized = true;
 	}
 
@@ -601,7 +615,8 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
 	PlanState  *outerPlan = outerPlanState(node);
 
 	/* rescan to release any page pin */
-	table_rescan(node->ss.ss_currentScanDesc, NULL);
+	if (node->ss.ss_currentScanDesc)
+		table_rescan(node->ss.ss_currentScanDesc, NULL);
 
 	/* release bitmaps and buffers if any */
 	if (node->tbmiterator)
@@ -678,7 +693,9 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node)
 	/*
 	 * close heap scan
 	 */
-	table_endscan(scanDesc);
+	if (scanDesc)
+		table_endscan(scanDesc);
+
 }
 
 /* ----------------------------------------------------------------
@@ -783,11 +800,6 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
 
 	scanstate->ss.ss_currentRelation = currentRelation;
 
-	scanstate->ss.ss_currentScanDesc = table_beginscan_bm(currentRelation,
-														  estate->es_snapshot,
-														  0,
-														  NULL);
-
 	/*
 	 * all done.
 	 */
-- 
2.40.1


--w4wcjcocxsm37usi
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v6-0002-BitmapHeapScan-set-can_skip_fetch-later.patch"



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

* [PATCH v7 01/13] BitmapHeapScan begin scan after bitmap creation
@ 2024-02-12 23:50  Melanie Plageman <[email protected]>
  0 siblings, 0 replies; 17+ messages in thread

From: Melanie Plageman @ 2024-02-12 23:50 UTC (permalink / raw)

There is no reason for a BitmapHeapScan to begin the scan of the
underlying table in ExecInitBitmapHeapScan(). Instead, do so after
completing the index scan and building the bitmap.
---
 src/backend/access/table/tableam.c        |  1 -
 src/backend/executor/nodeBitmapHeapscan.c | 26 +++++++++++++++++------
 2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/src/backend/access/table/tableam.c b/src/backend/access/table/tableam.c
index e57a0b7ea31..e78d793f69c 100644
--- a/src/backend/access/table/tableam.c
+++ b/src/backend/access/table/tableam.c
@@ -120,7 +120,6 @@ table_beginscan_catalog(Relation relation, int nkeys, struct ScanKeyData *key)
 											NULL, flags);
 }
 
-
 /* ----------------------------------------------------------------------------
  * Parallel table scan related functions.
  * ----------------------------------------------------------------------------
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index cee7f45aabe..93fdcd226bf 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -178,6 +178,20 @@ BitmapHeapNext(BitmapHeapScanState *node)
 			}
 #endif							/* USE_PREFETCH */
 		}
+
+		/*
+		 * If this is the first scan of the underlying table, create the table
+		 * scan descriptor and begin the scan.
+		 */
+		if (!scan)
+		{
+			scan = node->ss.ss_currentScanDesc = table_beginscan_bm(
+																	node->ss.ss_currentRelation,
+																	node->ss.ps.state->es_snapshot,
+																	0,
+																	NULL);
+		}
+
 		node->initialized = true;
 	}
 
@@ -601,7 +615,8 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
 	PlanState  *outerPlan = outerPlanState(node);
 
 	/* rescan to release any page pin */
-	table_rescan(node->ss.ss_currentScanDesc, NULL);
+	if (node->ss.ss_currentScanDesc)
+		table_rescan(node->ss.ss_currentScanDesc, NULL);
 
 	/* release bitmaps and buffers if any */
 	if (node->tbmiterator)
@@ -678,7 +693,9 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node)
 	/*
 	 * close heap scan
 	 */
-	table_endscan(scanDesc);
+	if (scanDesc)
+		table_endscan(scanDesc);
+
 }
 
 /* ----------------------------------------------------------------
@@ -783,11 +800,6 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
 
 	scanstate->ss.ss_currentRelation = currentRelation;
 
-	scanstate->ss.ss_currentScanDesc = table_beginscan_bm(currentRelation,
-														  estate->es_snapshot,
-														  0,
-														  NULL);
-
 	/*
 	 * all done.
 	 */
-- 
2.40.1


--kqqpqghcwbcc3dt5
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v7-0002-BitmapHeapScan-set-can_skip_fetch-later.patch"



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

* [PATCH v8 01/17] BitmapHeapScan begin scan after bitmap creation
@ 2024-02-12 23:50  Melanie Plageman <[email protected]>
  0 siblings, 0 replies; 17+ messages in thread

From: Melanie Plageman @ 2024-02-12 23:50 UTC (permalink / raw)

There is no reason for a BitmapHeapScan to begin the scan of the
underlying table in ExecInitBitmapHeapScan(). Instead, do so after
completing the index scan and building the bitmap.
---
 src/backend/access/table/tableam.c        |  1 -
 src/backend/executor/nodeBitmapHeapscan.c | 26 +++++++++++++++++------
 2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/src/backend/access/table/tableam.c b/src/backend/access/table/tableam.c
index e57a0b7ea3..e78d793f69 100644
--- a/src/backend/access/table/tableam.c
+++ b/src/backend/access/table/tableam.c
@@ -120,7 +120,6 @@ table_beginscan_catalog(Relation relation, int nkeys, struct ScanKeyData *key)
 											NULL, flags);
 }
 
-
 /* ----------------------------------------------------------------------------
  * Parallel table scan related functions.
  * ----------------------------------------------------------------------------
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index cee7f45aab..93fdcd226b 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -178,6 +178,20 @@ BitmapHeapNext(BitmapHeapScanState *node)
 			}
 #endif							/* USE_PREFETCH */
 		}
+
+		/*
+		 * If this is the first scan of the underlying table, create the table
+		 * scan descriptor and begin the scan.
+		 */
+		if (!scan)
+		{
+			scan = node->ss.ss_currentScanDesc = table_beginscan_bm(
+																	node->ss.ss_currentRelation,
+																	node->ss.ps.state->es_snapshot,
+																	0,
+																	NULL);
+		}
+
 		node->initialized = true;
 	}
 
@@ -601,7 +615,8 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
 	PlanState  *outerPlan = outerPlanState(node);
 
 	/* rescan to release any page pin */
-	table_rescan(node->ss.ss_currentScanDesc, NULL);
+	if (node->ss.ss_currentScanDesc)
+		table_rescan(node->ss.ss_currentScanDesc, NULL);
 
 	/* release bitmaps and buffers if any */
 	if (node->tbmiterator)
@@ -678,7 +693,9 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node)
 	/*
 	 * close heap scan
 	 */
-	table_endscan(scanDesc);
+	if (scanDesc)
+		table_endscan(scanDesc);
+
 }
 
 /* ----------------------------------------------------------------
@@ -783,11 +800,6 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
 
 	scanstate->ss.ss_currentRelation = currentRelation;
 
-	scanstate->ss.ss_currentScanDesc = table_beginscan_bm(currentRelation,
-														  estate->es_snapshot,
-														  0,
-														  NULL);
-
 	/*
 	 * all done.
 	 */
-- 
2.40.1


--xqq4defy3uncu6k6
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v8-0002-BitmapHeapScan-set-can_skip_fetch-later.patch"



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

* [PATCH v9 01/17] BitmapHeapScan begin scan after bitmap creation
@ 2024-02-12 23:50  Melanie Plageman <[email protected]>
  0 siblings, 0 replies; 17+ messages in thread

From: Melanie Plageman @ 2024-02-12 23:50 UTC (permalink / raw)

There is no reason for a BitmapHeapScan to begin the scan of the
underlying table in ExecInitBitmapHeapScan(). Instead, do so after
completing the index scan and building the bitmap.
---
 src/backend/access/table/tableam.c        |  1 -
 src/backend/executor/nodeBitmapHeapscan.c | 26 +++++++++++++++++------
 2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/src/backend/access/table/tableam.c b/src/backend/access/table/tableam.c
index e57a0b7ea3..e78d793f69 100644
--- a/src/backend/access/table/tableam.c
+++ b/src/backend/access/table/tableam.c
@@ -120,7 +120,6 @@ table_beginscan_catalog(Relation relation, int nkeys, struct ScanKeyData *key)
 											NULL, flags);
 }
 
-
 /* ----------------------------------------------------------------------------
  * Parallel table scan related functions.
  * ----------------------------------------------------------------------------
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index cee7f45aab..93fdcd226b 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -178,6 +178,20 @@ BitmapHeapNext(BitmapHeapScanState *node)
 			}
 #endif							/* USE_PREFETCH */
 		}
+
+		/*
+		 * If this is the first scan of the underlying table, create the table
+		 * scan descriptor and begin the scan.
+		 */
+		if (!scan)
+		{
+			scan = node->ss.ss_currentScanDesc = table_beginscan_bm(
+																	node->ss.ss_currentRelation,
+																	node->ss.ps.state->es_snapshot,
+																	0,
+																	NULL);
+		}
+
 		node->initialized = true;
 	}
 
@@ -601,7 +615,8 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
 	PlanState  *outerPlan = outerPlanState(node);
 
 	/* rescan to release any page pin */
-	table_rescan(node->ss.ss_currentScanDesc, NULL);
+	if (node->ss.ss_currentScanDesc)
+		table_rescan(node->ss.ss_currentScanDesc, NULL);
 
 	/* release bitmaps and buffers if any */
 	if (node->tbmiterator)
@@ -678,7 +693,9 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node)
 	/*
 	 * close heap scan
 	 */
-	table_endscan(scanDesc);
+	if (scanDesc)
+		table_endscan(scanDesc);
+
 }
 
 /* ----------------------------------------------------------------
@@ -783,11 +800,6 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
 
 	scanstate->ss.ss_currentRelation = currentRelation;
 
-	scanstate->ss.ss_currentScanDesc = table_beginscan_bm(currentRelation,
-														  estate->es_snapshot,
-														  0,
-														  NULL);
-
 	/*
 	 * all done.
 	 */
-- 
2.40.1


--7mdtsjmrzitrgzgx
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v9-0002-BitmapHeapScan-set-can_skip_fetch-later.patch"



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

* [PATCH v10 01/17] BitmapHeapScan begin scan after bitmap creation
@ 2024-02-12 23:50  Melanie Plageman <[email protected]>
  0 siblings, 0 replies; 17+ messages in thread

From: Melanie Plageman @ 2024-02-12 23:50 UTC (permalink / raw)

There is no reason for a BitmapHeapScan to begin the scan of the
underlying table in ExecInitBitmapHeapScan(). Instead, do so after
completing the index scan and building the bitmap.
---
 src/backend/access/table/tableam.c        |  1 -
 src/backend/executor/nodeBitmapHeapscan.c | 26 +++++++++++++++++------
 2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/src/backend/access/table/tableam.c b/src/backend/access/table/tableam.c
index e57a0b7ea3..e78d793f69 100644
--- a/src/backend/access/table/tableam.c
+++ b/src/backend/access/table/tableam.c
@@ -120,7 +120,6 @@ table_beginscan_catalog(Relation relation, int nkeys, struct ScanKeyData *key)
 											NULL, flags);
 }
 
-
 /* ----------------------------------------------------------------------------
  * Parallel table scan related functions.
  * ----------------------------------------------------------------------------
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index cee7f45aab..93fdcd226b 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -178,6 +178,20 @@ BitmapHeapNext(BitmapHeapScanState *node)
 			}
 #endif							/* USE_PREFETCH */
 		}
+
+		/*
+		 * If this is the first scan of the underlying table, create the table
+		 * scan descriptor and begin the scan.
+		 */
+		if (!scan)
+		{
+			scan = node->ss.ss_currentScanDesc = table_beginscan_bm(
+																	node->ss.ss_currentRelation,
+																	node->ss.ps.state->es_snapshot,
+																	0,
+																	NULL);
+		}
+
 		node->initialized = true;
 	}
 
@@ -601,7 +615,8 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
 	PlanState  *outerPlan = outerPlanState(node);
 
 	/* rescan to release any page pin */
-	table_rescan(node->ss.ss_currentScanDesc, NULL);
+	if (node->ss.ss_currentScanDesc)
+		table_rescan(node->ss.ss_currentScanDesc, NULL);
 
 	/* release bitmaps and buffers if any */
 	if (node->tbmiterator)
@@ -678,7 +693,9 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node)
 	/*
 	 * close heap scan
 	 */
-	table_endscan(scanDesc);
+	if (scanDesc)
+		table_endscan(scanDesc);
+
 }
 
 /* ----------------------------------------------------------------
@@ -783,11 +800,6 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
 
 	scanstate->ss.ss_currentRelation = currentRelation;
 
-	scanstate->ss.ss_currentScanDesc = table_beginscan_bm(currentRelation,
-														  estate->es_snapshot,
-														  0,
-														  NULL);
-
 	/*
 	 * all done.
 	 */
-- 
2.40.1


--3o7pc6dfau5a5hry
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v10-0002-BitmapHeapScan-set-can_skip_fetch-later.patch"



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

* [PATCH v11 01/17] BitmapHeapScan begin scan after bitmap creation
@ 2024-02-12 23:50  Melanie Plageman <[email protected]>
  0 siblings, 0 replies; 17+ messages in thread

From: Melanie Plageman @ 2024-02-12 23:50 UTC (permalink / raw)

There is no reason for a BitmapHeapScan to begin the scan of the
underlying table in ExecInitBitmapHeapScan(). Instead, do so after
completing the index scan and building the bitmap.
---
 src/backend/access/table/tableam.c        |  1 -
 src/backend/executor/nodeBitmapHeapscan.c | 26 +++++++++++++++++------
 2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/src/backend/access/table/tableam.c b/src/backend/access/table/tableam.c
index 8d3675be95..a254e3175b 100644
--- a/src/backend/access/table/tableam.c
+++ b/src/backend/access/table/tableam.c
@@ -120,7 +120,6 @@ table_beginscan_catalog(Relation relation, int nkeys, struct ScanKeyData *key)
 											NULL, flags);
 }
 
-
 /* ----------------------------------------------------------------------------
  * Parallel table scan related functions.
  * ----------------------------------------------------------------------------
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index cee7f45aab..93fdcd226b 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -178,6 +178,20 @@ BitmapHeapNext(BitmapHeapScanState *node)
 			}
 #endif							/* USE_PREFETCH */
 		}
+
+		/*
+		 * If this is the first scan of the underlying table, create the table
+		 * scan descriptor and begin the scan.
+		 */
+		if (!scan)
+		{
+			scan = node->ss.ss_currentScanDesc = table_beginscan_bm(
+																	node->ss.ss_currentRelation,
+																	node->ss.ps.state->es_snapshot,
+																	0,
+																	NULL);
+		}
+
 		node->initialized = true;
 	}
 
@@ -601,7 +615,8 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
 	PlanState  *outerPlan = outerPlanState(node);
 
 	/* rescan to release any page pin */
-	table_rescan(node->ss.ss_currentScanDesc, NULL);
+	if (node->ss.ss_currentScanDesc)
+		table_rescan(node->ss.ss_currentScanDesc, NULL);
 
 	/* release bitmaps and buffers if any */
 	if (node->tbmiterator)
@@ -678,7 +693,9 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node)
 	/*
 	 * close heap scan
 	 */
-	table_endscan(scanDesc);
+	if (scanDesc)
+		table_endscan(scanDesc);
+
 }
 
 /* ----------------------------------------------------------------
@@ -783,11 +800,6 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
 
 	scanstate->ss.ss_currentRelation = currentRelation;
 
-	scanstate->ss.ss_currentScanDesc = table_beginscan_bm(currentRelation,
-														  estate->es_snapshot,
-														  0,
-														  NULL);
-
 	/*
 	 * all done.
 	 */
-- 
2.40.1


--owzzsiozz6hgpp7e
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v11-0002-BitmapHeapScan-set-can_skip_fetch-later.patch"



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

* [PATCH v12 01/17] BitmapHeapScan begin scan after bitmap creation
@ 2024-02-12 23:50  Melanie Plageman <[email protected]>
  0 siblings, 0 replies; 17+ messages in thread

From: Melanie Plageman @ 2024-02-12 23:50 UTC (permalink / raw)

There is no reason for a BitmapHeapScan to begin the scan of the
underlying table in ExecInitBitmapHeapScan(). Instead, do so after
completing the index scan and building the bitmap.
---
 src/backend/access/table/tableam.c        |  1 -
 src/backend/executor/nodeBitmapHeapscan.c | 26 +++++++++++++++++------
 2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/src/backend/access/table/tableam.c b/src/backend/access/table/tableam.c
index 805d222cebc..b0f61c65f36 100644
--- a/src/backend/access/table/tableam.c
+++ b/src/backend/access/table/tableam.c
@@ -120,7 +120,6 @@ table_beginscan_catalog(Relation relation, int nkeys, struct ScanKeyData *key)
 											NULL, flags);
 }
 
-
 /* ----------------------------------------------------------------------------
  * Parallel table scan related functions.
  * ----------------------------------------------------------------------------
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index cee7f45aabe..93fdcd226bf 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -178,6 +178,20 @@ BitmapHeapNext(BitmapHeapScanState *node)
 			}
 #endif							/* USE_PREFETCH */
 		}
+
+		/*
+		 * If this is the first scan of the underlying table, create the table
+		 * scan descriptor and begin the scan.
+		 */
+		if (!scan)
+		{
+			scan = node->ss.ss_currentScanDesc = table_beginscan_bm(
+																	node->ss.ss_currentRelation,
+																	node->ss.ps.state->es_snapshot,
+																	0,
+																	NULL);
+		}
+
 		node->initialized = true;
 	}
 
@@ -601,7 +615,8 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
 	PlanState  *outerPlan = outerPlanState(node);
 
 	/* rescan to release any page pin */
-	table_rescan(node->ss.ss_currentScanDesc, NULL);
+	if (node->ss.ss_currentScanDesc)
+		table_rescan(node->ss.ss_currentScanDesc, NULL);
 
 	/* release bitmaps and buffers if any */
 	if (node->tbmiterator)
@@ -678,7 +693,9 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node)
 	/*
 	 * close heap scan
 	 */
-	table_endscan(scanDesc);
+	if (scanDesc)
+		table_endscan(scanDesc);
+
 }
 
 /* ----------------------------------------------------------------
@@ -783,11 +800,6 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
 
 	scanstate->ss.ss_currentRelation = currentRelation;
 
-	scanstate->ss.ss_currentScanDesc = table_beginscan_bm(currentRelation,
-														  estate->es_snapshot,
-														  0,
-														  NULL);
-
 	/*
 	 * all done.
 	 */
-- 
2.40.1


--6jpz2j246qmht4bt
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v12-0002-BitmapHeapScan-set-can_skip_fetch-later.patch"



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

end of thread, other threads:[~2024-02-12 23:50 UTC | newest]

Thread overview: 17+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-01-18 20:37 [PATCH 06/18] One fewer (not one less) Justin Pryzby <[email protected]>
2024-02-12 23:50 [PATCH v12 01/17] BitmapHeapScan begin scan after bitmap creation Melanie Plageman <[email protected]>
2024-02-12 23:50 [PATCH v19 01/21] BitmapHeapScan: begin scan after bitmap creation Melanie Plageman <[email protected]>
2024-02-12 23:50 [PATCH v16 01/18] BitmapHeapScan begin scan after bitmap creation Melanie Plageman <[email protected]>
2024-02-12 23:50 [PATCH v9 01/17] BitmapHeapScan begin scan after bitmap creation Melanie Plageman <[email protected]>
2024-02-12 23:50 [PATCH v3 01/13] BitmapHeapScan begin scan after bitmap creation Melanie Plageman <[email protected]>
2024-02-12 23:50 [PATCH v7 01/13] BitmapHeapScan begin scan after bitmap creation Melanie Plageman <[email protected]>
2024-02-12 23:50 [PATCH v15 01/13] BitmapHeapScan begin scan after bitmap creation Melanie Plageman <[email protected]>
2024-02-12 23:50 [PATCH v11 01/17] BitmapHeapScan begin scan after bitmap creation Melanie Plageman <[email protected]>
2024-02-12 23:50 [PATCH v4 01/14] BitmapHeapScan begin scan after bitmap creation Melanie Plageman <[email protected]>
2024-02-12 23:50 [PATCH v10 01/17] BitmapHeapScan begin scan after bitmap creation Melanie Plageman <[email protected]>
2024-02-12 23:50 [PATCH v6 01/14] BitmapHeapScan begin scan after bitmap creation Melanie Plageman <[email protected]>
2024-02-12 23:50 [PATCH v8 01/17] BitmapHeapScan begin scan after bitmap creation Melanie Plageman <[email protected]>
2024-02-12 23:50 [PATCH v13 01/16] BitmapHeapScan begin scan after bitmap creation Melanie Plageman <[email protected]>
2024-02-12 23:50 [PATCH v5 01/14] BitmapHeapScan begin scan after bitmap creation Melanie Plageman <[email protected]>
2024-02-12 23:50 [PATCH v17 1/9] BitmapHeapScan: begin scan after bitmap creation Melanie Plageman <[email protected]>
2024-02-12 23:50 [PATCH v18 01/10] BitmapHeapScan: begin scan after bitmap creation 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