Remove an obsolete comment in gistinsert()
Hi,
While learning the GIST codes, I find an obsolete comment in gistinsert ().
itup = gistFormTuple(giststate, r,
values, isnull, true /* size is currently
bogus */ );
The comment "/* size is currently bogus */" is weird because it follows a
bool variable.
I tracked the commit history. The original gistFormTuple() prototype is as
below:
extern IndexTuple gistFormTuple(GISTSTATE *giststate,
Relation r, Datum *attdata, int *datumsize, bool
*isnull);
you can see this commit 37c8393 to check that.
After 1f7ef54, the function prototype changed, as below:
extern IndexTuple gistFormTuple(GISTSTATE *giststate,
Relation r, Datum *attdata, bool *isnull, bool
newValues);
As you can see, "int *datumsize" had been removed, but the comments in
gistbuildCallback() and gistinsert() were not removed together.
By the way, after commit 5edb24a, the comments in gistbuildCallback() was
removed.
So I think we can now remove the obsolete comment from gistinsert().
--
Thanks,
Tender Wang
Attachments:
0001-Remove-an-obsolete-comment-in-gistinsert.patchapplication/octet-stream; name=0001-Remove-an-obsolete-comment-in-gistinsert.patchDownload
From 63592e744e5166fd76fa5ad6498f98084df89494 Mon Sep 17 00:00:00 2001
From: Tender Wang <tndrwang@gmail.com>
Date: Tue, 5 Nov 2024 19:21:52 +0800
Subject: [PATCH] Remove an obsolete comment in gistinsert()
---
src/backend/access/gist/gist.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/backend/access/gist/gist.c b/src/backend/access/gist/gist.c
index 3ae913e023..177786a13f 100644
--- a/src/backend/access/gist/gist.c
+++ b/src/backend/access/gist/gist.c
@@ -179,8 +179,7 @@ gistinsert(Relation r, Datum *values, bool *isnull,
oldCxt = MemoryContextSwitchTo(giststate->tempCxt);
- itup = gistFormTuple(giststate, r,
- values, isnull, true /* size is currently bogus */ );
+ itup = gistFormTuple(giststate, r, values, isnull, true);
itup->t_tid = *ht_ctid;
gistdoinsert(r, itup, 0, giststate, heapRel, false);
--
2.25.1
Hi Tender,
While learning the GIST codes, I find an obsolete comment in gistinsert ().
itup = gistFormTuple(giststate, r,
values, isnull, true /* size is currently bogus */ );
Thanks for reporting. I agree that this is an oversight of commit 1f7ef54.
The commit changed the signature of gistFormTuple():
```
IndexTuple
gistFormTuple(GISTSTATE *giststate, Relation r,
- Datum attdata[], int datumsize[], bool isnull[])
+ Datum attdata[], bool isnull[], bool newValues)
```... but left the comment for the `datumsize` argument:
```
itup = gistFormTuple(&buildstate->giststate, index,
- values, NULL /* size is currently bogus */, isnull);
+ values, isnull, true /* size is currently bogus */);
```I checked the rest of gistFormTuple() calls and also looked for other
comments like this. There seems to be only one call like this to fix.
--
Best regards,
Aleksander Alekseev
Aleksander Alekseev <aleksander@timescale.com> 于2024年11月5日周二 22:08写道：
Hi Tender,
While learning the GIST codes, I find an obsolete comment in gistinsert
().
itup = gistFormTuple(giststate, r,
values, isnull, true /* size iscurrently bogus */ );
Thanks for reporting. I agree that this is an oversight of commit 1f7ef54.
The commit changed the signature of gistFormTuple():
``` IndexTuple gistFormTuple(GISTSTATE *giststate, Relation r, - Datum attdata[], int datumsize[], bool isnull[]) + Datum attdata[], bool isnull[], bool newValues) ```... but left the comment for the `datumsize` argument:
``` itup = gistFormTuple(&buildstate->giststate, index, - values, NULL /* size is currently bogus */, isnull); + values, isnull, true /* size is currently bogus */); ```I checked the rest of gistFormTuple() calls and also looked for other
comments like this. There seems to be only one call like this to fix.
Thanks for reviewing this. I have added it to the 2015-01 commitfest.
--
Thanks,
Tender Wang
On Thu, Nov 07, 2024 at 10:57:08AM +0800, Tender Wang wrote:
Thanks for reviewing this. I have added it to the 2015-01 commitfest.
Right. I don't quite see why this comment would apply anymore, and
the commit you are pointing to looks right. Will fix.
--
Michael
Michael Paquier <michael@paquier.xyz> 于2024年11月7日周四 14:11写道：
On Thu, Nov 07, 2024 at 10:57:08AM +0800, Tender Wang wrote:
Thanks for reviewing this. I have added it to the 2015-01 commitfest.
Right. I don't quite see why this comment would apply anymore, and
the commit you are pointing to looks right. Will fix.
Thanks for pushing. I have changed the status to committed on commitfest
2025-01
--
Thanks,
Tender Wang