Re: [PATCH] Check for TupleTableSlot nullness before dereferencing - Mailing list pgsql-hackers

From Jacob Champion
Subject Re: [PATCH] Check for TupleTableSlot nullness before dereferencing
Date
Msg-id CAOYmi+k9k8U_K=zE4067iRs=01r3XA8uBUr72LkcrjEYMzUO5A@mail.gmail.com
Whole thread Raw
In response to Re: [PATCH] Check for TupleTableSlot nullness before dereferencing  (Alexander Kuznetsov <kuznetsovam@altlinux.org>)
Responses Re: [PATCH] Check for TupleTableSlot nullness before dereferencing
List pgsql-hackers
On Fri, Dec 13, 2024 at 12:54 AM Alexander Kuznetsov
<kuznetsovam@altlinux.org> wrote:
> ping. What do you think about reasoning below? Maybe we should consider
> proposing different patch for removing redundant check there?

To move this forward a bit, your reasoning:

> 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.

(A test case producing a NULL dereference would be the fastest way to
prove that it can happen; are you able to construct one?
groupingsets.sql, in the regression tests, hits this code and could
provide a starting point.)

Thanks,
--Jacob



pgsql-hackers by date:

Previous
From: David Rowley
Date:
Subject: Re: Add estimated hit ratio to Memoize in EXPLAIN to explain cost adjustment
Next
From: Michael Paquier
Date:
Subject: Re: Support for 8-byte TOAST values (aka the TOAST infinite loop problem)