On Fri, Dec 13, 2024 at 12:54 AM Alexander Kuznetsov <kuznetso...@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
