public inbox for [email protected]  

help / color / mirror / Atom feed

From: David Rowley <[email protected]>
To: Jacob Champion <[email protected]>
Cc: [email protected]
Cc: Daniel Gustafsson <[email protected]>
Cc: PostgreSQL Developers <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: Nikolay Shaplov <[email protected]>
Subject: Re: [PATCH] Check for TupleTableSlot nullness before dereferencing
Date: Tue, 22 Jul 2025 14:25:27 +1200
Message-ID: <CAApHDvo2N2SafdpgNEhny+GSq-AJLv=_MugFx0L0xAz1x+8dMg@mail.gmail.com> (raw)
In-Reply-To: <CAOYmi+k9k8U_K=zE4067iRs=01r3XA8uBUr72LkcrjEYMzUO5A@mail.gmail.com>
References: <[email protected]>
	<[email protected]>
	<[email protected]>
	<[email protected]>
	<CAOYmi+k9k8U_K=zE4067iRs=01r3XA8uBUr72LkcrjEYMzUO5A@mail.gmail.com>

On Tue, 22 Jul 2025 at 10:20, Jacob Champion
<[email protected]> 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

view thread (6+ messages)

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], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: [PATCH] Check for TupleTableSlot nullness before dereferencing
  In-Reply-To: <CAApHDvo2N2SafdpgNEhny+GSq-AJLv=_MugFx0L0xAz1x+8dMg@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