public inbox for [email protected]
help / color / mirror / Atom feedFrom: Xuneng Zhou <[email protected]>
To: Japin Li <[email protected]>
Cc: Chao Li <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: heapam_tuple_complete_speculative : remove unnecessary tuple fetch
Date: Tue, 31 Mar 2026 09:09:47 +0800
Message-ID: <CABPTF7XJ3gv+R+4+eGMX=ggN1m6bD7=rbxDi-cHN+GJUV5jSYg@mail.gmail.com> (raw)
In-Reply-To: <SY7PR01MB1092127415D38E676B1B0BAA0B648A@SY7PR01MB10921.ausprd01.prod.outlook.com>
References: <[email protected]>
<SY7PR01MB1092127415D38E676B1B0BAA0B648A@SY7PR01MB10921.ausprd01.prod.outlook.com>
On Tue, Mar 24, 2026 at 8:16 PM Japin Li <[email protected]> wrote:
>
>
> > 在 2026年3月24日，14:57，Chao Li <[email protected]> 写道：
> >
> > Hi,
> >
> > While reviewing another patch, I noticed this:
> > ```
> > static void
> > heapam_tuple_complete_speculative(Relation relation, TupleTableSlot *slot,
> > uint32 specToken, bool succeeded)
> > {
> > bool shouldFree = true;
> > HeapTuple tuple = ExecFetchSlotHeapTuple(slot, true, &shouldFree); // <== tuple is not used
> >
> > /* adjust the tuple's state accordingly */
> > if (succeeded)
> > heap_finish_speculative(relation, &slot->tts_tid);
> > else
> > heap_abort_speculative(relation, &slot->tts_tid);
> >
> > if (shouldFree)
> > pfree(tuple);
> > }
> > ```
> >
> > In this function, tuple is not used at all, so there seems to be no need to fetch it, and shouldFree is thus not needed either.
> >
> > This appears to have been there since 5db6df0c011, where the function was introduced. It looks like a copy-pasto from the previous function, heapam_tuple_insert_speculative(), which does need to fetch and possibly free the tuple.
> >
> > I tried simply removing ExecFetchSlotHeapTuple(), and "make check" still passes. But I may be missing something, so I’d like to confirm.
> >
> > The attached patch just removes the unused tuple and shouldFree from this function. As touching the file, I also fixed a typo in the file header comment.
>
> Makes sense! All test cases passed with make check-world.
>
+1, looks like a simple copy-pasto and the patch LGTM.
--
Best,
Xuneng
view thread (4+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected]
Subject: Re: heapam_tuple_complete_speculative : remove unnecessary tuple fetch
In-Reply-To: <CABPTF7XJ3gv+R+4+eGMX=ggN1m6bD7=rbxDi-cHN+GJUV5jSYg@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox