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+1-3
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