Horton

Thread

Commits

  1. Remove an obsolete comment in gistinsert()

  2. Buffering GiST index build algorithm.

  3. Changes

  4. WAL for GiST. It work for online backup and so on, but on

  1. Remove an obsolete comment in gistinsert()

    Tender Wang <tndrwang@gmail.com> — 2024-11-05T11:48:37Z

    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

  2. Re: Remove an obsolete comment in gistinsert()

    Aleksander Alekseev <aleksander@timescale.com> — 2024-11-05T14:08:20Z

    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

  3. Re: Remove an obsolete comment in gistinsert()

    Tender Wang <tndrwang@gmail.com> — 2024-11-07T02:57:08Z

    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 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.
>

Thanks for reviewing this. I have added it to the 2015-01 commitfest.

-- 
Thanks,
Tender Wang

  4. Re: Remove an obsolete comment in gistinsert()

    Michael Paquier <michael@paquier.xyz> — 2024-11-07T06:10:57Z

    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

  5. Re: Remove an obsolete comment in gistinsert()

    Tender Wang <tndrwang@gmail.com> — 2024-11-07T06:27:40Z

    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