Is tuplesort meant to support bounded datum sorts? - Mailing list pgsql-hackers

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