Re: [PATCH] Check for TupleTableSlot nullness before dereferencing

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