Useless parameter 'cur_skey' in IndexScanOK

Started by Hugo Zhangover 1 year ago4 messages
#1Hugo Zhang
hugo.zhang@openpie.com

Hi,
The 'cur_skey' parameter in `IndexScanOK` funciton doesn't seem to be useful.

The function does not use cur_skey for any operation. Is there any other consideration
for retaining the cur_skey parameter here?

Best wishes
Hugo zhang

#2Aleksander Alekseev
aleksander@timescale.com
In reply to: Hugo Zhang (#1)
1 attachment(s)
Re: Useless parameter 'cur_skey' in IndexScanOK

Hi,

The 'cur_skey' parameter in `IndexScanOK` funciton doesn't seem to be useful.

Good catch. As I understand it is not used for anything since
a78fcfb51243 (dated 2006) and this is a static function, so we
shouldn't worry about third-party extensions.

I wonder why none of the compilers complained about this.

Here is the patch.

--
Best regards,
Aleksander Alekseev

Attachments:

v1-0001-IndexScanOK-remove-unused-parameter-cur_skey.patchapplication/x-patch; name=v1-0001-IndexScanOK-remove-unused-parameter-cur_skey.patchDownload
From 7361a4b12262317e10c2203fed018be258beb16f Mon Sep 17 00:00:00 2001
From: Aleksander Alekseev <aleksander@timescale.com>
Date: Wed, 3 Jul 2024 16:22:14 +0300
Subject: [PATCH v1] IndexScanOK: remove unused parameter cur_skey

Oversight of a78fcfb51243.

Hugo Zhang, Aleksander Alekseev
---
 src/backend/utils/cache/catcache.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/backend/utils/cache/catcache.c b/src/backend/utils/cache/catcache.c
index 111d8a280a..492a033aa2 100644
--- a/src/backend/utils/cache/catcache.c
+++ b/src/backend/utils/cache/catcache.c
@@ -1194,7 +1194,7 @@ InitCatCachePhase2(CatCache *cache, bool touch_index)
  *		catalogs' indexes.
  */
 static bool
-IndexScanOK(CatCache *cache, ScanKey cur_skey)
+IndexScanOK(CatCache *cache)
 {
 	switch (cache->id)
 	{
@@ -1483,7 +1483,7 @@ SearchCatCacheMiss(CatCache *cache,
 
 		scandesc = systable_beginscan(relation,
 									  cache->cc_indexoid,
-									  IndexScanOK(cache, cur_skey),
+									  IndexScanOK(cache),
 									  NULL,
 									  nkeys,
 									  cur_skey);
@@ -1803,7 +1803,7 @@ SearchCatCacheList(CatCache *cache,
 
 			scandesc = systable_beginscan(relation,
 										  cache->cc_indexoid,
-										  IndexScanOK(cache, cur_skey),
+										  IndexScanOK(cache),
 										  NULL,
 										  nkeys,
 										  cur_skey);
-- 
2.45.2

#3Daniel Gustafsson
daniel@yesql.se
In reply to: Aleksander Alekseev (#2)
Re: Useless parameter 'cur_skey' in IndexScanOK

On 3 Jul 2024, at 15:41, Aleksander Alekseev <aleksander@timescale.com> wrote:

The 'cur_skey' parameter in `IndexScanOK` funciton doesn't seem to be useful.

Good catch. As I understand it is not used for anything since
a78fcfb51243 (dated 2006) and this is a static function, so we
shouldn't worry about third-party extensions.

Agreed, it seems reasonable to clean this up.

I wonder why none of the compilers complained about this.

Not to mention static analyzers.

--
Daniel Gustafsson

#4Heikki Linnakangas
hlinnaka@iki.fi
In reply to: Daniel Gustafsson (#3)
Re: Useless parameter 'cur_skey' in IndexScanOK

On 03/07/2024 16:46, Daniel Gustafsson wrote:

On 3 Jul 2024, at 15:41, Aleksander Alekseev <aleksander@timescale.com> wrote:

The 'cur_skey' parameter in `IndexScanOK` funciton doesn't seem to be useful.

Good catch. As I understand it is not used for anything since
a78fcfb51243 (dated 2006) and this is a static function, so we
shouldn't worry about third-party extensions.

Agreed, it seems reasonable to clean this up.

I wonder why none of the compilers complained about this.

Not to mention static analyzers.

Committed, thanks.

--
Heikki Linnakangas
Neon (https://neon.tech)