Re: [PATCH] Check for TupleTableSlot nullness before dereferencing

On Tue, 22 Jul 2025 at 10:20, Jacob Champion
<jacob(dot)champion(at)enterprisedb(dot)com> wrote:
>
> On Fri, Dec 13, 2024 at 12:54 AM Alexander Kuznetsov
> > 1. slot2 is NULL at line 968,
> > 2. The while loop at line 971 executes once, filling slot1 (slot2 still remains NULL),
> > 3. No changes occur to slot2 thereafter, up to line 1003,
> > 4. At line 1003, slot2 is swapped with slot1 (assuming numDistinctCols > 0),
> > 5. At line 1016, slot1 is dereferenced with conent of slot2 (NULL).
>
> assumes that (numDistinctCols > 0) can be true at the same time that
> (slot2 == NULL), but it's not clear to me that this can happen in
> practice. See build_pertrans_for_aggref(), where these are assigned.

I also can't see how this could happen. uniqslot is created in
build_pertrans_for_aggref() when "numDistinctCols > 0", and the swap
of slot1 and slot2 also only occurs when "numDistinctCols > 0". i.e.
the uniqslot will always be allocated when the aggregate has DISTINCT
(unless it's a presorted aggregate, in which case
process_ordered_aggregate_multi() isn't used).

If numDistinctCols > 0 then so is numSortCols > 0 per "numSortCols =
numDistinctCols = list_length(sortlist);" in
build_pertrans_for_aggref().

The code in this area gets a bit of exercise with:

create table ab(a text, b text);
insert into ab values('1','1'),('1','1');
set enable_presorted_aggregate=0;
select string_agg(distinct a,b) from ab;

David