Re: Check for tuplestorestate nullness before dereferencing

Hi!

On 14.10.2024 16:41, Ilia Evdokimov wrote:
>
> On 14.10.2024 12:25, Alexander Kuznetsov wrote:
>> Hello everyone,
>>
>> I'd like to propose adding a check for the nullness of
>> tuplestorestate before dereferencing it
>> in src/backend/executor/nodeModifier.c. The patch is attached.
>>
>> I am proposing this fix based on the assumption that tuplestorestate
>> could be NULL
>> since there is a check for it when calculating eof_tuplestore at line
>> 85.
>> However, since this code hasn't been changed since 2006 and hasn't
>> caused any issues,
>> it is possible that the check for (tuplestorestate == NULL) is
>> redundant when calculating eof_tuplestore.
>>
>
> Hi Alexander,
>
> The 'tuplestorestate' variable may be initialized at line 64 if it is
> NULL. You should consider initializing this variable earlier.
>
>
To be honest, I'm not sure this change is necessary. Looking at the
code, I see that in ExecMaterial it is possible to handle a
tuplestorestate of NULL, and this error can be accessed if the flags are
not zero, but I think these cases have been worked out.

As I see it, node->eflags can be zero if it passes the output of a
subquery, during the initialization of the Material node execution, and
when the subquery is rescanned.

After the subplan scan is complete, we see that the eof_underlying
variable becomes true, and this part of the code will no longer be
accessible. tuplestorestate also becomes Null.

I also noticed that tuplestorestate=NULL is an indicator that the scan
is complete, so if this section of code is called, something is wrong
earlier in the code.

--
Regards,
Alena Rybakina
Postgres Professional