Horton

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix outdated comments regarding TupleTableSlots

  1. A structure has changed but comment modifications maybe missed

    myzhen <zhenmingyang@yeah.net> — 2023-03-25T14:43:33Z

    Recently when I was reading the TupleTableSlot code, I noticed that the field "tts_tuple" mentioned in the comment has been removed in the higher version, but it still exists in the comment.
location：
https://github.com/postgres/postgres/blob/master/src/include/executor/tuptable.h
src/include/executor/tuptable.h：71
src/include/executor/tuptable.h：271

  2. Re: A structure has changed but comment modifications maybe missed

    Richard Guo <guofenglinux@gmail.com> — 2023-03-27T02:05:55Z

    On Sat, Mar 25, 2023 at 10:43 PM 甄明洋 <zhenmingyang@yeah.net> wrote:

> Recently when I was reading the TupleTableSlot code, I noticed that the
> field "tts_tuple" mentioned in the comment has been removed in the higher
> version, but it still exists in the comment.
> location：
>
> https://github.com/postgres/postgres/blob/master/src/include/executor/tuptable.h
> src/include/executor/tuptable.h：71
> src/include/executor/tuptable.h：271
>

Good catch.  This should be a minor oversight in 4da597ed, in which the
TupleTableSlot implementation has been splitted into different slot
types, and tts_tuple has been removed from TupleTableSlot struct.

Besides, at tuptable.h:71, I think TTS_SHOULDFREE should be
TTS_FLAG_SHOULDFREE.

Thanks
Richard

  3. Re: A structure has changed but comment modifications maybe missed

    Richard Guo <guofenglinux@gmail.com> — 2023-03-27T02:19:59Z

    On Mon, Mar 27, 2023 at 10:05 AM Richard Guo <guofenglinux@gmail.com> wrote:

> Besides, at tuptable.h:71, I think TTS_SHOULDFREE should be
> TTS_FLAG_SHOULDFREE.
>

A further search shows several other places referring to TTS_SHOULDFREE,
which I think should be TTS_FLAG_SHOULDFREE.

    tuptable.h:71
    tuptable.h:82
    execTuples.c:388
    execTuples.c:557

Thanks
Richard

  4. Re: A structure has changed but comment modifications maybe missed

    David Rowley <dgrowleyml@gmail.com> — 2023-03-30T01:55:53Z

    On Mon, 27 Mar 2023 at 15:20, Richard Guo <guofenglinux@gmail.com> wrote:
> A further search shows several other places referring to TTS_SHOULDFREE,
> which I think should be TTS_FLAG_SHOULDFREE.
>
>     tuptable.h:71
>     tuptable.h:82
>     execTuples.c:388
>     execTuples.c:557

I think the attached is what you have in mind. Can you confirm?

David

  5. Re: A structure has changed but comment modifications maybe missed

    Tom Lane <tgl@sss.pgh.pa.us> — 2023-03-30T02:02:09Z

    David Rowley <dgrowleyml@gmail.com> writes:
> I think the attached is what you have in mind. Can you confirm?

tts_nvalid is still a thing, so I question your edit here:

--- a/src/include/executor/tuptable.h
+++ b/src/include/executor/tuptable.h
@@ -68,8 +68,7 @@
  * A TupleTableSlot can also be "empty", indicated by flag TTS_FLAG_EMPTY set
  * in tts_flags, holding no valid data.  This is the only valid state for a
  * freshly-created slot that has not yet had a tuple descriptor assigned to
- * it.  In this state, TTS_SHOULDFREE should not be set in tts_flags, tts_tuple
- * must be NULL and tts_nvalid zero.
+ * it.  In this state, TTS_FLAG_SHOULDFREE should not be set in tts_flags.
  *
  * The tupleDescriptor is simply referenced, not copied, by the TupleTableSlot
  * code.  The caller of ExecSetSlotDescriptor() is responsible for providing

Andres would really be the expert about these changes though.

			regards, tom lane

  6. Re: A structure has changed but comment modifications maybe missed

    David Rowley <dgrowleyml@gmail.com> — 2023-03-30T03:00:23Z

    On Thu, 30 Mar 2023 at 15:02, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> tts_nvalid is still a thing, so I question your edit here:

Yeah, that should be kept.  See the adjusted patch attached.

David

  7. Re: A structure has changed but comment modifications maybe missed

    Andres Freund <andres@anarazel.de> — 2023-03-30T03:07:40Z

    Hi,

On 2023-03-30 16:00:23 +1300, David Rowley wrote:
> See the adjusted patch attached.

Looks reasonable to me. We could try to reference TTS_SHOULDFREE(), without
mentioning flags, instead - but I think your version is better.

Greetings,

Andres Freund

  8. Re: A structure has changed but comment modifications maybe missed

    Richard Guo <guofenglinux@gmail.com> — 2023-03-30T03:33:43Z

    On Thu, Mar 30, 2023 at 11:07 AM Andres Freund <andres@anarazel.de> wrote:

> On 2023-03-30 16:00:23 +1300, David Rowley wrote:
> > See the adjusted patch attached.
>
> Looks reasonable to me. We could try to reference TTS_SHOULDFREE(), without
> mentioning flags, instead - but I think your version is better.


+1.

Thanks
Richard

  9. Re: A structure has changed but comment modifications maybe missed

    David Rowley <dgrowleyml@gmail.com> — 2023-03-30T03:37:39Z

    On Thu, 30 Mar 2023 at 16:07, Andres Freund <andres@anarazel.de> wrote:
> Looks reasonable to me. We could try to reference TTS_SHOULDFREE(), without
> mentioning flags, instead - but I think your version is better.

Thanks. Pushed.

David