Is tuplesort meant to support bounded datum sorts?

Over on [1], Ronan is working on allowing Datum sorts for nodeSort.c
when we're just sorting a single Datum.

I was looking at his v4 patch and noticed that he'd modified
free_sort_tuple() to conditionally only free the sort tuple if it's
non-NULL. Without this change, the select.sql regression test fails
on:

select * from onek,
(values ((select i from
(values(10000), (2), (389), (1000), (2000), ((select 10029))) as foo(i)
order by i asc limit 1))) bar (i)
where onek.unique1 = bar.i;

The limit 1 makes this a bounded sort and we call free_sort_tuple()
during make_bounded_heap().

It looks like this has likely never come up before because the only
time we use tuplesort_set_bound() is in nodeSort.c and
nodeIncrementalSort.c, none of those currently use datum sorts.

However, I'm thinking this is still a bug that should be fixed
separately from Ronan's main patch.

Does anyone else have thoughts on this?

The fragment in question is:

@@ -4773,6 +4773,14 @@ leader_takeover_tapes(Tuplesortstate *state)
static void
free_sort_tuple(Tuplesortstate *state, SortTuple *stup)
{
- FREEMEM(state, GetMemoryChunkSpace(stup->tuple));
- pfree(stup->tuple);
+ /*
+ * If the SortTuple is actually only a single Datum, which was not copied
+ * as it is a byval type, do not try to free it nor account for it in
+ * memory used.
+ */
+ if (stup->tuple)
+ {
+ FREEMEM(state, GetMemoryChunkSpace(stup->tuple));
+ pfree(stup->tuple);
+ }
}

David

[1] https://www.postgresql.org/message-id/3060002.hb0XKQ11pn@aivenronan