On Tue, May 31, 2016 at 10:10 PM, Robert Haas <robertmh...@gmail.com> wrote: > On Fri, May 27, 2016 at 6:07 PM, Andrew Gierth > <and...@tao11.riddles.org.uk> wrote: >> copyParamList does not respect from->paramMask, in what looks to me like >> an obvious oversight: >> >> retval->paramMask = NULL; >> [...] >> /* Ignore parameters we don't need, to save cycles and space. */ >> if (retval->paramMask != NULL && >> !bms_is_member(i, retval->paramMask)) >> >> retval->paramMask is never set to anything not NULL in this function, >> so surely that should either be initializing it to from->paramMask, or >> checking from->paramMask in the conditional? > > Oh, dear. I think you are right. I'm kind of surprised this didn't > provoke a test failure somewhere. >
The reason why it didn't provoked any test failure is that it doesn't seem to be possible that from->paramMask in copyParamList can ever be non-NULL. Params formed will always have paramMask set as NULL in the code paths where copyParamList is used. I think we can just assign from->paramMask to retval->paramMask to make sure that even if it gets used in future, the code works as expected. Alternatively, one might think of adding an Assert there, but that doesn't seem to be future-proof. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
set-parammask-copyparam-v1.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
