On Fri, 23 Dec 2022 at 00:36, Ted Yu wrote:
>>
>> Hi,
>> I was looking at commit 941aa6a6268a6a66f6895401aad6b5329111d412 .
>>
>> I think it would be better to move the assertion into
>> `index_beginscan_internal` because it is called by index_beginscan and
>> index_beginscan_bitmap
>>
>> In the future, when a new variant of `index_beginscan_XX` is added, the
>> assertion would be effective for the new variant.
>>
>> Please let me know what you think.
Hi, Ted!
The two asserts out of four could be combined as you proposed in the patch.
Assert added by 941aa6a6268a6a66f6 to index_parallelscan_initialize
should remain anyway as otherwise, we can have Segfault in
SerializeSnapshot called from this function.
Assert in index_parallelscan_estimate can be omitted as there is the
same assert inside EstimateSnapshotSpace called from this function.
I've included it into version 2 of a patch.
Not sure it's worth the effort but IMO the patch is right and can be committed.
Kind regards,
Pavel Borisov,
Supabase.
From ea3f6ca585156d0b90eb56516d3f62102b939932 Mon Sep 17 00:00:00 2001
From: Pavel Borisov
Date: Fri, 23 Dec 2022 01:04:40 +0400
Subject: [PATCH v2] Optimize asserts introduced in 941aa6a6268a6a66f68
Two asserts combined, one unnecessary removed.
Authors: Ted Yu , Pavel Borsiov
Reviewed-by: Pavel Borisov
---
src/backend/access/index/indexam.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/src/backend/access/index/indexam.c b/src/backend/access/index/indexam.c
index dc303995e5b..673395893ba 100644
--- a/src/backend/access/index/indexam.c
+++ b/src/backend/access/index/indexam.c
@@ -209,8 +209,6 @@ index_beginscan(Relation heapRelation,
{
IndexScanDesc scan;
- Assert(snapshot != InvalidSnapshot);
-
scan = index_beginscan_internal(indexRelation, nkeys, norderbys, snapshot, NULL, false);
/*
@@ -239,8 +237,6 @@ index_beginscan_bitmap(Relation indexRelation,
{
IndexScanDesc scan;
- Assert(snapshot != InvalidSnapshot);
-
scan = index_beginscan_internal(indexRelation, nkeys, 0, snapshot, NULL, false);
/*
@@ -265,6 +261,8 @@ index_beginscan_internal(Relation indexRelation,
RELATION_CHECKS;
CHECK_REL_PROCEDURE(ambeginscan);
+ Assert(snapshot != InvalidSnapshot);
+
if (!(indexRelation->rd_indam->ampredlocks))
PredicateLockRelation(indexRelation, snapshot);
@@ -407,8 +405,6 @@ index_parallelscan_estimate(Relation indexRelation, Snapshot snapshot)
{
Size nbytes;
- Assert(snapshot != InvalidSnapshot);
-
RELATION_CHECKS;
nbytes = offsetof(ParallelIndexScanDescData, ps_snapshot_data);
--
2.24.3 (Apple Git-128)
