When systable_beginscan() and systable_beginscan_ordered() choose an
index scan, they remap the attribute numbers in the passed-in scan keys
to the attribute numbers of the index, and then write those remapped
attribute numbers back into the scan key passed by the caller. This
second part is surprising and gratuitous. It means that a scan key
cannot safely be used more than once (but it might sometimes work,
depending on circumstances). Also, there is no value in providing these
remapped attribute numbers back to the caller, since they can't do
anything with that.
I propose to fix that by making a copy of the scan keys passed by the
caller and make the modifications there.
In order to prove to myself that there are no other cases where
caller-provided scan keys are modified, I went through and
const-qualified all the APIs. This works out correctly. Several levels
down in the stack, the access methods make their own copy of the scan
keys that they store in their scan descriptors, and they use those in
non-const-clean ways, but that's ok, that's their business. As far as
the top-level callers are concerned, they can rely on their scan keys to
be const after this.
I'm not proposing this second patch for committing at this time, since
that would modify the public access method APIs in an incompatible way.
I've made a proposal of a similar nature in [0]. At some point, it
might be worth batching these and other changes together and make the
change. I might come back to that later.
[0]:
https://www.postgresql.org/message-id/flat/14c31f4a-0347-0805-dce8-93a9072c05a5%40eisentraut.org
While researching how the scan keys get copied around, I noticed that
the index access methods all use memmove() to make the above-mentioned
copy into their own scan descriptor. This is fine, but memmove() is
usually only used when something special is going on that would prevent
memcpy() from working, which is not the case there. So to avoid the
confusion for future readers, I changed those to memcpy(). I suspect
that this code has been copied between the different index AM over time.
(The nbtree version of this code is literally unchanged since July 1996.)From d968667cf4808f0ff5ff2c3bd37b9bf85c9d4a11 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Thu, 8 Aug 2024 08:27:26 +0200
Subject: [PATCH 1/3] Don't overwrite scan key in systable_beginscan()
When systable_beginscan() and systable_beginscan_ordered() choose an
index scan, they remap the attribute numbers in the passed-in scan
keys to the attribute numbers of the index, and then write those
remapped attribute numbers back into the scan key passed by the
caller. This second part is surprising and gratuitous. It means that
a scan key cannot safely be used more than once (but it might
sometimes work, depending on circumstances). Also, there is no value
in providing these remapped attribute numbers back to the caller,
since they can't do anything with that.
Fix that by making a copy of the scan keys passed by the caller and
make the modifications there.
---
src/backend/access/index/genam.c | 24 +++++++++++++++++-------
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/src/backend/access/index/genam.c b/src/backend/access/index/genam.c
index de751e8e4a3..b5eba549d3e 100644
--- a/src/backend/access/index/genam.c
+++ b/src/backend/access/index/genam.c
@@ -371,7 +371,7 @@ index_compute_xid_horizon_for_tuples(Relation irel,
* nkeys, key: scan keys
*
* The attribute numbers in the scan key should be set for the heap case.
- * If we choose to index, we reset them to 1..n to reference the index
+ * If we choose to index, we convert them to 1..n to reference the index
* columns. Note this means there must be one scankey qualification per
* index column! This is checked by the Asserts in the normal, index-using
* case, but won't be checked if the heapscan path is taken.
@@ -419,17 +419,22 @@ systable_beginscan(Relation heapRelation,
if (irel)
{
int i;
+ ScanKey idxkey;
- /* Change attribute numbers to be index column numbers. */
+ idxkey = palloc_array(ScanKeyData, nkeys);
+
+ /* Convert attribute numbers to be index column numbers. */
for (i = 0; i < nkeys; i++)
{
int j;
+ memcpy(&idxkey[i], &key[i], sizeof(ScanKeyData));
+
for (j = 0; j <
IndexRelationGetNumberOfAttributes(irel); j++)
{
if (key[i].sk_attno ==
irel->rd_index->indkey.values[j])
{
- key[i].sk_attno = j + 1;
+ idxkey[i].sk_attno = j + 1;
break;
}
}
@@ -439,7 +444,7 @@ systable_beginscan(Relation heapRelation,
sysscan->iscan = index_beginscan(heapRelation, irel,
snapshot, nkeys, 0);
- index_rescan(sysscan->iscan, key, nkeys, NULL, 0);
+ index_rescan(sysscan->iscan, idxkey, nkeys, NULL, 0);
sysscan->scan = NULL;
}
else
@@ -647,6 +652,7 @@ systable_beginscan_ordered(Relation heapRelation,
{
SysScanDesc sysscan;
int i;
+ ScanKey idxkey;
/* REINDEX can probably be a hard error here ... */
if (ReindexIsProcessingIndex(RelationGetRelid(indexRelation)))
@@ -678,16 +684,20 @@ systable_beginscan_ordered(Relation heapRelation,
sysscan->snapshot = NULL;
}
- /* Change attribute numbers to be index column numbers. */
+ idxkey = palloc_array(ScanKeyData, nkeys);
+
+ /* Convert attribute numbers to be index column numbers. */
for (i = 0; i < nkeys; i++)
{
int j;
+ memcpy(&idxkey[i], &key[i], sizeof(ScanKeyData));
+
for (j = 0; j <
IndexRelationGetNumberOfAttributes(indexRelation); j++)
{
if (key[i].sk_attno ==
indexRelation->rd_index->indkey.values[j])
{
- key[i].sk_attno = j + 1;
+ idxkey[i].sk_attno = j + 1;
break;
}
}
@@ -697,7 +707,7 @@ systable_beginscan_ordered(Relation heapRelation,
sysscan->iscan = index_beginscan(heapRelation, indexRelation,
snapshot, nkeys, 0);
- index_rescan(sysscan->iscan, key, nkeys, NULL, 0);
+ index_rescan(sysscan->iscan, idxkey, nkeys, NULL, 0);
sysscan->scan = NULL;
return sysscan;
base-commit: e56ccc8e4204d9faf86f3bd2e435a0788b3d0429
--
2.46.0
From 02a9af6d25eb0ddcc18947285317f53ece00f838 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Thu, 8 Aug 2024 08:27:26 +0200
Subject: [PATCH 2/3] Augment ScanKey arguments with const qualifiers
In most cases, functions do not modify a ScanKey that is passed in.
This is a sensible expection for such APIs. But the API did not
communicate that. This adds const qualifiers to the arguments to
improve that.
There are some subtleties here. For example heapgettup() and
heapgettup_pagemode() cannot guarantee that the ScanKeys are not
modified, so they do not get a const qualifiers. But that is okay,
because they work on keys that are copied into the scan descriptor by
heap_beginscan() (initscan()), so that does not interfere with the
keys passed in from the caller of heap_beginscan(). So this all works
out correctly.
---
contrib/bloom/bloom.h | 4 ++--
contrib/bloom/blscan.c | 4 ++--
src/backend/access/brin/brin.c | 4 ++--
src/backend/access/gin/ginscan.c | 4 ++--
src/backend/access/gist/gistscan.c | 4 ++--
src/backend/access/hash/hash.c | 4 ++--
src/backend/access/heap/heapam.c | 6 +++---
src/backend/access/index/genam.c | 4 ++--
src/backend/access/index/indexam.c | 4 ++--
src/backend/access/nbtree/nbtree.c | 4 ++--
src/backend/access/spgist/spgscan.c | 4 ++--
src/backend/access/table/tableam.c | 2 +-
src/include/access/amapi.h | 4 ++--
src/include/access/brin_internal.h | 4 ++--
src/include/access/genam.h | 8 ++++----
src/include/access/gin_private.h | 4 ++--
src/include/access/gistscan.h | 4 ++--
src/include/access/hash.h | 4 ++--
src/include/access/heapam.h | 4 ++--
src/include/access/nbtree.h | 4 ++--
src/include/access/spgist.h | 4 ++--
src/include/access/tableam.h | 18 +++++++++---------
.../modules/dummy_index_am/dummy_index_am.c | 4 ++--
23 files changed, 55 insertions(+), 55 deletions(-)
diff --git a/contrib/bloom/bloom.h b/contrib/bloom/bloom.h
index fba3ba77711..fbd49c08cd4 100644
--- a/contrib/bloom/bloom.h
+++ b/contrib/bloom/bloom.h
@@ -195,8 +195,8 @@ extern bool blinsert(Relation index, Datum *values, bool
*isnull,
struct IndexInfo *indexInfo);
extern IndexScanDesc blbeginscan(Relation r, int nkeys, int norderbys);
extern int64 blgetbitmap(IndexScanDesc scan, TIDBitmap *tbm);
-extern void blrescan(IndexScanDesc scan, ScanKey scankey, int nscankeys,
- ScanKey orderbys, int norderbys);
+extern void blrescan(IndexScanDesc scan, const ScanKeyData *scankey, int
nscankeys,
+ const ScanKeyData *orderbys, int
norderbys);
extern void blendscan(IndexScanDesc scan);
extern IndexBuildResult *blbuild(Relation heap, Relation index,
struct
IndexInfo *indexInfo);
diff --git a/contrib/bloom/blscan.c b/contrib/bloom/blscan.c
index 0a303a49b24..e29dc93491a 100644
--- a/contrib/bloom/blscan.c
+++ b/contrib/bloom/blscan.c
@@ -45,8 +45,8 @@ blbeginscan(Relation r, int nkeys, int norderbys)
* Rescan a bloom index.
*/
void
-blrescan(IndexScanDesc scan, ScanKey scankey, int nscankeys,
- ScanKey orderbys, int norderbys)
+blrescan(IndexScanDesc scan, const ScanKeyData *scankey, int nscankeys,
+ const ScanKeyData *orderbys, int norderbys)
{
BloomScanOpaque so = (BloomScanOpaque) scan->opaque;
diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index 6467bed604a..3a739eed029 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -943,8 +943,8 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
* Re-initialize state for a BRIN index scan
*/
void
-brinrescan(IndexScanDesc scan, ScanKey scankey, int nscankeys,
- ScanKey orderbys, int norderbys)
+brinrescan(IndexScanDesc scan, const ScanKeyData *scankey, int nscankeys,
+ const ScanKeyData *orderbys, int norderbys)
{
/*
* Other index AMs preprocess the scan keys at this point, or sometime
diff --git a/src/backend/access/gin/ginscan.c b/src/backend/access/gin/ginscan.c
index af24d38544e..7dfaa866b43 100644
--- a/src/backend/access/gin/ginscan.c
+++ b/src/backend/access/gin/ginscan.c
@@ -439,8 +439,8 @@ ginNewScanKey(IndexScanDesc scan)
}
void
-ginrescan(IndexScanDesc scan, ScanKey scankey, int nscankeys,
- ScanKey orderbys, int norderbys)
+ginrescan(IndexScanDesc scan, const ScanKeyData *scankey, int nscankeys,
+ const ScanKeyData *orderbys, int norderbys)
{
GinScanOpaque so = (GinScanOpaque) scan->opaque;
diff --git a/src/backend/access/gist/gistscan.c
b/src/backend/access/gist/gistscan.c
index e05801e2f5b..1f1c122bf97 100644
--- a/src/backend/access/gist/gistscan.c
+++ b/src/backend/access/gist/gistscan.c
@@ -124,8 +124,8 @@ gistbeginscan(Relation r, int nkeys, int norderbys)
}
void
-gistrescan(IndexScanDesc scan, ScanKey key, int nkeys,
- ScanKey orderbys, int norderbys)
+gistrescan(IndexScanDesc scan, const ScanKeyData *key, int nkeys,
+ const ScanKeyData *orderbys, int norderbys)
{
/* nkeys and norderbys arguments are ignored */
GISTScanOpaque so = (GISTScanOpaque) scan->opaque;
diff --git a/src/backend/access/hash/hash.c b/src/backend/access/hash/hash.c
index 01d06b7c328..3bb621a893f 100644
--- a/src/backend/access/hash/hash.c
+++ b/src/backend/access/hash/hash.c
@@ -394,8 +394,8 @@ hashbeginscan(Relation rel, int nkeys, int norderbys)
* hashrescan() -- rescan an index relation
*/
void
-hashrescan(IndexScanDesc scan, ScanKey scankey, int nscankeys,
- ScanKey orderbys, int norderbys)
+hashrescan(IndexScanDesc scan, const ScanKeyData *scankey, int nscankeys,
+ const ScanKeyData *orderbys, int norderbys)
{
HashScanOpaque so = (HashScanOpaque) scan->opaque;
Relation rel = scan->indexRelation;
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 91b20147a00..6bb5805ec74 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -291,7 +291,7 @@ heap_scan_stream_read_next_serial(ReadStream *stream,
* ----------------
*/
static void
-initscan(HeapScanDesc scan, ScanKey key, bool keep_startblock)
+initscan(HeapScanDesc scan, const ScanKeyData *key, bool keep_startblock)
{
ParallelBlockTableScanDesc bpscan = NULL;
bool allow_strat;
@@ -1036,7 +1036,7 @@ heapgettup_pagemode(HeapScanDesc scan,
TableScanDesc
heap_beginscan(Relation relation, Snapshot snapshot,
- int nkeys, ScanKey key,
+ int nkeys, const ScanKeyData *key,
ParallelTableScanDesc parallel_scan,
uint32 flags)
{
@@ -1149,7 +1149,7 @@ heap_beginscan(Relation relation, Snapshot snapshot,
}
void
-heap_rescan(TableScanDesc sscan, ScanKey key, bool set_params,
+heap_rescan(TableScanDesc sscan, const ScanKeyData *key, bool set_params,
bool allow_strat, bool allow_sync, bool allow_pagemode)
{
HeapScanDesc scan = (HeapScanDesc) sscan;
diff --git a/src/backend/access/index/genam.c b/src/backend/access/index/genam.c
index b5eba549d3e..0f047e1fb73 100644
--- a/src/backend/access/index/genam.c
+++ b/src/backend/access/index/genam.c
@@ -385,7 +385,7 @@ systable_beginscan(Relation heapRelation,
Oid indexId,
bool indexOK,
Snapshot snapshot,
- int nkeys, ScanKey key)
+ int nkeys, const ScanKeyData *key)
{
SysScanDesc sysscan;
Relation irel;
@@ -648,7 +648,7 @@ SysScanDesc
systable_beginscan_ordered(Relation heapRelation,
Relation indexRelation,
Snapshot snapshot,
- int nkeys, ScanKey key)
+ int nkeys, const ScanKeyData
*key)
{
SysScanDesc sysscan;
int i;
diff --git a/src/backend/access/index/indexam.c
b/src/backend/access/index/indexam.c
index dcd04b813d8..f0b85155d54 100644
--- a/src/backend/access/index/indexam.c
+++ b/src/backend/access/index/indexam.c
@@ -350,8 +350,8 @@ index_beginscan_internal(Relation indexRelation,
*/
void
index_rescan(IndexScanDesc scan,
- ScanKey keys, int nkeys,
- ScanKey orderbys, int norderbys)
+ const ScanKeyData *keys, int nkeys,
+ const ScanKeyData *orderbys, int norderbys)
{
SCAN_CHECKS;
CHECK_SCAN_PROCEDURE(amrescan);
diff --git a/src/backend/access/nbtree/nbtree.c
b/src/backend/access/nbtree/nbtree.c
index 686a3206f72..5962596b06b 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -356,8 +356,8 @@ btbeginscan(Relation rel, int nkeys, int norderbys)
* btrescan() -- rescan an index relation
*/
void
-btrescan(IndexScanDesc scan, ScanKey scankey, int nscankeys,
- ScanKey orderbys, int norderbys)
+btrescan(IndexScanDesc scan, const ScanKeyData *scankey, int nscankeys,
+ const ScanKeyData *orderbys, int norderbys)
{
BTScanOpaque so = (BTScanOpaque) scan->opaque;
diff --git a/src/backend/access/spgist/spgscan.c
b/src/backend/access/spgist/spgscan.c
index 03293a7816e..d6bea65fde5 100644
--- a/src/backend/access/spgist/spgscan.c
+++ b/src/backend/access/spgist/spgscan.c
@@ -377,8 +377,8 @@ spgbeginscan(Relation rel, int keysz, int orderbysz)
}
void
-spgrescan(IndexScanDesc scan, ScanKey scankey, int nscankeys,
- ScanKey orderbys, int norderbys)
+spgrescan(IndexScanDesc scan, const ScanKeyData *scankey, int nscankeys,
+ const ScanKeyData *orderbys, int norderbys)
{
SpGistScanOpaque so = (SpGistScanOpaque) scan->opaque;
diff --git a/src/backend/access/table/tableam.c
b/src/backend/access/table/tableam.c
index e57a0b7ea31..4042087813d 100644
--- a/src/backend/access/table/tableam.c
+++ b/src/backend/access/table/tableam.c
@@ -109,7 +109,7 @@ table_slot_create(Relation relation, List **reglist)
*/
TableScanDesc
-table_beginscan_catalog(Relation relation, int nkeys, struct ScanKeyData *key)
+table_beginscan_catalog(Relation relation, int nkeys, const struct ScanKeyData
*key)
{
uint32 flags = SO_TYPE_SEQSCAN |
SO_ALLOW_STRAT | SO_ALLOW_SYNC | SO_ALLOW_PAGEMODE |
SO_TEMP_SNAPSHOT;
diff --git a/src/include/access/amapi.h b/src/include/access/amapi.h
index f25c9d58a7d..f10cabadce0 100644
--- a/src/include/access/amapi.h
+++ b/src/include/access/amapi.h
@@ -168,9 +168,9 @@ typedef IndexScanDesc (*ambeginscan_function) (Relation
indexRelation,
/* (re)start index scan */
typedef void (*amrescan_function) (IndexScanDesc scan,
- ScanKey keys,
+ const
ScanKeyData *keys,
int nkeys,
- ScanKey
orderbys,
+ const
ScanKeyData *orderbys,
int
norderbys);
/* next valid tuple */
diff --git a/src/include/access/brin_internal.h
b/src/include/access/brin_internal.h
index a5a9772621c..d226a76d93a 100644
--- a/src/include/access/brin_internal.h
+++ b/src/include/access/brin_internal.h
@@ -99,8 +99,8 @@ extern bool brininsert(Relation idxRel, Datum *values, bool
*nulls,
extern void brininsertcleanup(Relation index, struct IndexInfo *indexInfo);
extern IndexScanDesc brinbeginscan(Relation r, int nkeys, int norderbys);
extern int64 bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm);
-extern void brinrescan(IndexScanDesc scan, ScanKey scankey, int nscankeys,
- ScanKey orderbys, int norderbys);
+extern void brinrescan(IndexScanDesc scan, const ScanKeyData *scankey, int
nscankeys,
+ const ScanKeyData *orderbys, int
norderbys);
extern void brinendscan(IndexScanDesc scan);
extern IndexBulkDeleteResult *brinbulkdelete(IndexVacuumInfo *info,
IndexBulkDeleteResult *stats,
diff --git a/src/include/access/genam.h b/src/include/access/genam.h
index fdcfbe8db74..87bf823b045 100644
--- a/src/include/access/genam.h
+++ b/src/include/access/genam.h
@@ -160,8 +160,8 @@ extern IndexScanDesc index_beginscan_bitmap(Relation
indexRelation,
Snapshot snapshot,
int nkeys);
extern void index_rescan(IndexScanDesc scan,
- ScanKey keys, int nkeys,
- ScanKey orderbys, int
norderbys);
+ const ScanKeyData *keys, int
nkeys,
+ const ScanKeyData *orderbys,
int norderbys);
extern void index_endscan(IndexScanDesc scan);
extern void index_markpos(IndexScanDesc scan);
extern void index_restrpos(IndexScanDesc scan);
@@ -222,14 +222,14 @@ extern SysScanDesc systable_beginscan(Relation
heapRelation,
Oid
indexId,
bool
indexOK,
Snapshot snapshot,
- int
nkeys, ScanKey key);
+ int
nkeys, const ScanKeyData *key);
extern HeapTuple systable_getnext(SysScanDesc sysscan);
extern bool systable_recheck_tuple(SysScanDesc sysscan, HeapTuple tup);
extern void systable_endscan(SysScanDesc sysscan);
extern SysScanDesc systable_beginscan_ordered(Relation heapRelation,
Relation indexRelation,
Snapshot snapshot,
-
int nkeys, ScanKey key);
+
int nkeys, const ScanKeyData *key);
extern HeapTuple systable_getnext_ordered(SysScanDesc sysscan,
ScanDirection direction);
extern void systable_endscan_ordered(SysScanDesc sysscan);
diff --git a/src/include/access/gin_private.h b/src/include/access/gin_private.h
index 3013a44bae1..7cbc7b5ca4c 100644
--- a/src/include/access/gin_private.h
+++ b/src/include/access/gin_private.h
@@ -387,8 +387,8 @@ typedef GinScanOpaqueData *GinScanOpaque;
extern IndexScanDesc ginbeginscan(Relation rel, int nkeys, int norderbys);
extern void ginendscan(IndexScanDesc scan);
-extern void ginrescan(IndexScanDesc scan, ScanKey scankey, int nscankeys,
- ScanKey orderbys, int norderbys);
+extern void ginrescan(IndexScanDesc scan, const ScanKeyData *scankey, int
nscankeys,
+ const ScanKeyData *orderbys, int
norderbys);
extern void ginNewScanKey(IndexScanDesc scan);
extern void ginFreeScanKeys(GinScanOpaque so);
diff --git a/src/include/access/gistscan.h b/src/include/access/gistscan.h
index ba2153869a3..e6ea66d0457 100644
--- a/src/include/access/gistscan.h
+++ b/src/include/access/gistscan.h
@@ -17,8 +17,8 @@
#include "access/amapi.h"
extern IndexScanDesc gistbeginscan(Relation r, int nkeys, int norderbys);
-extern void gistrescan(IndexScanDesc scan, ScanKey key, int nkeys,
- ScanKey orderbys, int norderbys);
+extern void gistrescan(IndexScanDesc scan, const ScanKeyData *key, int nkeys,
+ const ScanKeyData *orderbys, int
norderbys);
extern void gistendscan(IndexScanDesc scan);
#endif /* GISTSCAN_H */
diff --git a/src/include/access/hash.h b/src/include/access/hash.h
index 9c7d81525b4..9ce53f48045 100644
--- a/src/include/access/hash.h
+++ b/src/include/access/hash.h
@@ -371,8 +371,8 @@ extern bool hashinsert(Relation rel, Datum *values, bool
*isnull,
extern bool hashgettuple(IndexScanDesc scan, ScanDirection dir);
extern int64 hashgetbitmap(IndexScanDesc scan, TIDBitmap *tbm);
extern IndexScanDesc hashbeginscan(Relation rel, int nkeys, int norderbys);
-extern void hashrescan(IndexScanDesc scan, ScanKey scankey, int nscankeys,
- ScanKey orderbys, int norderbys);
+extern void hashrescan(IndexScanDesc scan, const ScanKeyData *scankey, int
nscankeys,
+ const ScanKeyData *orderbys, int
norderbys);
extern void hashendscan(IndexScanDesc scan);
extern IndexBulkDeleteResult *hashbulkdelete(IndexVacuumInfo *info,
IndexBulkDeleteResult *stats,
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 9e9aec88a62..4c03ce7b000 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -287,13 +287,13 @@ typedef enum
#define HeapScanIsValid(scan) PointerIsValid(scan)
extern TableScanDesc heap_beginscan(Relation relation, Snapshot snapshot,
- int
nkeys, ScanKey key,
+ int
nkeys, const ScanKeyData *key,
ParallelTableScanDesc parallel_scan,
uint32
flags);
extern void heap_setscanlimits(TableScanDesc sscan, BlockNumber startBlk,
BlockNumber numBlks);
extern void heap_prepare_pagescan(TableScanDesc sscan);
-extern void heap_rescan(TableScanDesc sscan, ScanKey key, bool set_params,
+extern void heap_rescan(TableScanDesc sscan, const ScanKeyData *key, bool
set_params,
bool allow_strat, bool
allow_sync, bool allow_pagemode);
extern void heap_endscan(TableScanDesc sscan);
extern HeapTuple heap_getnext(TableScanDesc sscan, ScanDirection direction);
diff --git a/src/include/access/nbtree.h b/src/include/access/nbtree.h
index 74930433480..4519a64e072 100644
--- a/src/include/access/nbtree.h
+++ b/src/include/access/nbtree.h
@@ -1173,8 +1173,8 @@ extern Size btestimateparallelscan(int nkeys, int
norderbys);
extern void btinitparallelscan(void *target);
extern bool btgettuple(IndexScanDesc scan, ScanDirection dir);
extern int64 btgetbitmap(IndexScanDesc scan, TIDBitmap *tbm);
-extern void btrescan(IndexScanDesc scan, ScanKey scankey, int nscankeys,
- ScanKey orderbys, int norderbys);
+extern void btrescan(IndexScanDesc scan, const ScanKeyData *scankey, int
nscankeys,
+ const ScanKeyData *orderbys, int
norderbys);
extern void btparallelrescan(IndexScanDesc scan);
extern void btendscan(IndexScanDesc scan);
extern void btmarkpos(IndexScanDesc scan);
diff --git a/src/include/access/spgist.h b/src/include/access/spgist.h
index d6a49531200..53e2ff992dc 100644
--- a/src/include/access/spgist.h
+++ b/src/include/access/spgist.h
@@ -205,8 +205,8 @@ extern bool spginsert(Relation index, Datum *values, bool
*isnull,
/* spgscan.c */
extern IndexScanDesc spgbeginscan(Relation rel, int keysz, int orderbysz);
extern void spgendscan(IndexScanDesc scan);
-extern void spgrescan(IndexScanDesc scan, ScanKey scankey, int nscankeys,
- ScanKey orderbys, int norderbys);
+extern void spgrescan(IndexScanDesc scan, const ScanKeyData *scankey, int
nscankeys,
+ const ScanKeyData *orderbys, int
norderbys);
extern int64 spggetbitmap(IndexScanDesc scan, TIDBitmap *tbm);
extern bool spggettuple(IndexScanDesc scan, ScanDirection dir);
extern bool spgcanreturn(Relation index, int attno);
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index da661289c1f..9bf33e1127d 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -327,7 +327,7 @@ typedef struct TableAmRoutine
*/
TableScanDesc (*scan_begin) (Relation rel,
Snapshot
snapshot,
- int nkeys,
struct ScanKeyData *key,
+ int nkeys,
const struct ScanKeyData *key,
ParallelTableScanDesc pscan,
uint32 flags);
@@ -341,7 +341,7 @@ typedef struct TableAmRoutine
* Restart relation scan. If set_params is set to true, allow_{strat,
* sync, pagemode} (see scan_begin) changes should be taken into
account.
*/
- void (*scan_rescan) (TableScanDesc scan, struct ScanKeyData
*key,
+ void (*scan_rescan) (TableScanDesc scan, const struct
ScanKeyData *key,
bool
set_params, bool allow_strat,
bool
allow_sync, bool allow_pagemode);
@@ -906,7 +906,7 @@ extern TupleTableSlot *table_slot_create(Relation relation,
List **reglist);
*/
static inline TableScanDesc
table_beginscan(Relation rel, Snapshot snapshot,
- int nkeys, struct ScanKeyData *key)
+ int nkeys, const struct ScanKeyData *key)
{
uint32 flags = SO_TYPE_SEQSCAN |
SO_ALLOW_STRAT | SO_ALLOW_SYNC | SO_ALLOW_PAGEMODE;
@@ -919,7 +919,7 @@ table_beginscan(Relation rel, Snapshot snapshot,
* snapshot appropriate for scanning catalog relations.
*/
extern TableScanDesc table_beginscan_catalog(Relation relation, int nkeys,
-
struct ScanKeyData *key);
+
const struct ScanKeyData *key);
/*
* Like table_beginscan(), but table_beginscan_strat() offers an extended API
@@ -930,7 +930,7 @@ extern TableScanDesc table_beginscan_catalog(Relation
relation, int nkeys,
*/
static inline TableScanDesc
table_beginscan_strat(Relation rel, Snapshot snapshot,
- int nkeys, struct ScanKeyData *key,
+ int nkeys, const struct ScanKeyData
*key,
bool allow_strat, bool allow_sync)
{
uint32 flags = SO_TYPE_SEQSCAN | SO_ALLOW_PAGEMODE;
@@ -951,7 +951,7 @@ table_beginscan_strat(Relation rel, Snapshot snapshot,
*/
static inline TableScanDesc
table_beginscan_bm(Relation rel, Snapshot snapshot,
- int nkeys, struct ScanKeyData *key, bool
need_tuple)
+ int nkeys, const struct ScanKeyData *key,
bool need_tuple)
{
uint32 flags = SO_TYPE_BITMAPSCAN | SO_ALLOW_PAGEMODE;
@@ -970,7 +970,7 @@ table_beginscan_bm(Relation rel, Snapshot snapshot,
*/
static inline TableScanDesc
table_beginscan_sampling(Relation rel, Snapshot snapshot,
- int nkeys, struct ScanKeyData
*key,
+ int nkeys, const struct
ScanKeyData *key,
bool allow_strat, bool
allow_sync,
bool allow_pagemode)
{
@@ -1026,7 +1026,7 @@ table_endscan(TableScanDesc scan)
*/
static inline void
table_rescan(TableScanDesc scan,
- struct ScanKeyData *key)
+ const struct ScanKeyData *key)
{
scan->rs_rd->rd_tableam->scan_rescan(scan, key, false, false, false,
false);
}
@@ -1040,7 +1040,7 @@ table_rescan(TableScanDesc scan,
* previously selected startblock will be kept.
*/
static inline void
-table_rescan_set_params(TableScanDesc scan, struct ScanKeyData *key,
+table_rescan_set_params(TableScanDesc scan, const struct ScanKeyData *key,
bool allow_strat, bool
allow_sync, bool allow_pagemode)
{
scan->rs_rd->rd_tableam->scan_rescan(scan, key, true,
diff --git a/src/test/modules/dummy_index_am/dummy_index_am.c
b/src/test/modules/dummy_index_am/dummy_index_am.c
index 0b477116067..3fd041d5293 100644
--- a/src/test/modules/dummy_index_am/dummy_index_am.c
+++ b/src/test/modules/dummy_index_am/dummy_index_am.c
@@ -256,8 +256,8 @@ dibeginscan(Relation r, int nkeys, int norderbys)
* Rescan of index AM.
*/
static void
-direscan(IndexScanDesc scan, ScanKey scankey, int nscankeys,
- ScanKey orderbys, int norderbys)
+direscan(IndexScanDesc scan, const ScanKeyData *scankey, int nscankeys,
+ const ScanKeyData *orderbys, int norderbys)
{
/* nothing to do */
}
--
2.46.0
From 39b99d5fa0cd7eeeac48727e802ee97c0f8e61fe Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Thu, 8 Aug 2024 08:27:26 +0200
Subject: [PATCH 3/3] Replace gratuitous memmove() with memcpy()
The index access methods all had similar code that copied the
passed-in scan keys to local storage. They all used memmove() for
that, which is not wrong, but it seems confusing not to use memcpy()
when that would work. Presumably, this was all once copied from
ancient code and never adjusted.
---
contrib/bloom/blscan.c | 5 +----
src/backend/access/brin/brin.c | 3 +--
src/backend/access/gin/ginscan.c | 5 +----
src/backend/access/gist/gistscan.c | 6 ++----
src/backend/access/hash/hash.c | 6 +-----
src/backend/access/nbtree/nbtree.c | 4 +---
src/backend/access/spgist/spgscan.c | 6 ++----
7 files changed, 9 insertions(+), 26 deletions(-)
diff --git a/contrib/bloom/blscan.c b/contrib/bloom/blscan.c
index e29dc93491a..67899c1369a 100644
--- a/contrib/bloom/blscan.c
+++ b/contrib/bloom/blscan.c
@@ -55,10 +55,7 @@ blrescan(IndexScanDesc scan, const ScanKeyData *scankey, int
nscankeys,
so->sign = NULL;
if (scankey && scan->numberOfKeys > 0)
- {
- memmove(scan->keyData, scankey,
- scan->numberOfKeys * sizeof(ScanKeyData));
- }
+ memcpy(scan->keyData, scankey, scan->numberOfKeys *
sizeof(ScanKeyData));
}
/*
diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index 3a739eed029..d38441e05ab 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -955,8 +955,7 @@ brinrescan(IndexScanDesc scan, const ScanKeyData *scankey,
int nscankeys,
*/
if (scankey && scan->numberOfKeys > 0)
- memmove(scan->keyData, scankey,
- scan->numberOfKeys * sizeof(ScanKeyData));
+ memcpy(scan->keyData, scankey, scan->numberOfKeys *
sizeof(ScanKeyData));
}
/*
diff --git a/src/backend/access/gin/ginscan.c b/src/backend/access/gin/ginscan.c
index 7dfaa866b43..678a8ce075a 100644
--- a/src/backend/access/gin/ginscan.c
+++ b/src/backend/access/gin/ginscan.c
@@ -447,10 +447,7 @@ ginrescan(IndexScanDesc scan, const ScanKeyData *scankey,
int nscankeys,
ginFreeScanKeys(so);
if (scankey && scan->numberOfKeys > 0)
- {
- memmove(scan->keyData, scankey,
- scan->numberOfKeys * sizeof(ScanKeyData));
- }
+ memcpy(scan->keyData, scankey, scan->numberOfKeys *
sizeof(ScanKeyData));
}
diff --git a/src/backend/access/gist/gistscan.c
b/src/backend/access/gist/gistscan.c
index 1f1c122bf97..a10099cf150 100644
--- a/src/backend/access/gist/gistscan.c
+++ b/src/backend/access/gist/gistscan.c
@@ -233,8 +233,7 @@ gistrescan(IndexScanDesc scan, const ScanKeyData *key, int
nkeys,
fn_extras[i] =
scan->keyData[i].sk_func.fn_extra;
}
- memmove(scan->keyData, key,
- scan->numberOfKeys * sizeof(ScanKeyData));
+ memcpy(scan->keyData, key, scan->numberOfKeys *
sizeof(ScanKeyData));
/*
* Modify the scan key so that the Consistent method is called
for all
@@ -289,8 +288,7 @@ gistrescan(IndexScanDesc scan, const ScanKeyData *key, int
nkeys,
fn_extras[i] =
scan->orderByData[i].sk_func.fn_extra;
}
- memmove(scan->orderByData, orderbys,
- scan->numberOfOrderBys * sizeof(ScanKeyData));
+ memcpy(scan->orderByData, orderbys, scan->numberOfOrderBys *
sizeof(ScanKeyData));
so->orderByTypes = (Oid *) palloc(scan->numberOfOrderBys *
sizeof(Oid));
diff --git a/src/backend/access/hash/hash.c b/src/backend/access/hash/hash.c
index 3bb621a893f..ea0b95a2af3 100644
--- a/src/backend/access/hash/hash.c
+++ b/src/backend/access/hash/hash.c
@@ -414,11 +414,7 @@ hashrescan(IndexScanDesc scan, const ScanKeyData *scankey,
int nscankeys,
/* Update scan key, if a new one is given */
if (scankey && scan->numberOfKeys > 0)
- {
- memmove(scan->keyData,
- scankey,
- scan->numberOfKeys * sizeof(ScanKeyData));
- }
+ memcpy(scan->keyData, scankey, scan->numberOfKeys *
sizeof(ScanKeyData));
so->hashso_buc_populated = false;
so->hashso_buc_split = false;
diff --git a/src/backend/access/nbtree/nbtree.c
b/src/backend/access/nbtree/nbtree.c
index 5962596b06b..24f8a3a721c 100644
--- a/src/backend/access/nbtree/nbtree.c
+++ b/src/backend/access/nbtree/nbtree.c
@@ -403,9 +403,7 @@ btrescan(IndexScanDesc scan, const ScanKeyData *scankey,
int nscankeys,
* Reset the scan keys
*/
if (scankey && scan->numberOfKeys > 0)
- memmove(scan->keyData,
- scankey,
- scan->numberOfKeys * sizeof(ScanKeyData));
+ memcpy(scan->keyData, scankey, scan->numberOfKeys *
sizeof(ScanKeyData));
so->numberOfKeys = 0; /* until _bt_preprocess_keys sets it */
so->numArrayKeys = 0; /* ditto */
}
diff --git a/src/backend/access/spgist/spgscan.c
b/src/backend/access/spgist/spgscan.c
index d6bea65fde5..8a990fba1c3 100644
--- a/src/backend/access/spgist/spgscan.c
+++ b/src/backend/access/spgist/spgscan.c
@@ -384,16 +384,14 @@ spgrescan(IndexScanDesc scan, const ScanKeyData *scankey,
int nscankeys,
/* copy scankeys into local storage */
if (scankey && scan->numberOfKeys > 0)
- memmove(scan->keyData, scankey,
- scan->numberOfKeys * sizeof(ScanKeyData));
+ memcpy(scan->keyData, scankey, scan->numberOfKeys *
sizeof(ScanKeyData));
/* initialize order-by data if needed */
if (orderbys && scan->numberOfOrderBys > 0)
{
int i;
- memmove(scan->orderByData, orderbys,
- scan->numberOfOrderBys * sizeof(ScanKeyData));
+ memcpy(scan->orderByData, orderbys, scan->numberOfOrderBys *
sizeof(ScanKeyData));
for (i = 0; i < scan->numberOfOrderBys; i++)
{
--
2.46.0
