> On Sat, Aug 12, 2023 at 09:28:19AM +0800, Julien Rouhaud wrote: > On Fri, Aug 11, 2023 at 05:55:26PM +0200, Dmitry Dolgov wrote: > > > > Another confusing example was this one at the end of set_session_variable: > > > > + /* > > + * XXX While unlikely, an error here is possible. It wouldn't > > leak memory > > + * as the allocated chunk has already been correctly assigned > > to the > > + * session variable, but would contradict this function > > contract, which is > > + * that this function should either succeed or leave the > > current value > > + * untouched. > > + */ > > + elog(DEBUG1, "session variable \"%s.%s\" (oid:%u) has new > > value", > > + > > get_namespace_name(get_session_variable_namespace(svar->varid)), > > + get_session_variable_name(svar->varid), > > + svar->varid); > > > > It's not clear, which exactly error you're talking about, it's the last > > instruction in the function. > > FTR I think I'm the one that changed that. The error I was talking about is > elog() itself (in case of OOM for instance), or even one of the get_* call, if > running with log_level <= DEBUG1. It's clearly really unlikely but still > possible, thus this comment which also tries to explain why this elog() is not > done earlier.
I see, thanks for clarification. Absolutely nitpicking, but the crucial "that's why this elog is not done earlier" is only assumed in the comment between the lines, not stated out loud :)
