On Fri, Aug 26, 2016 at 11:26 AM, Tom Lane <t...@sss.pgh.pa.us> wrote:
> Or in short, this has confused edata and newedata.  Valid coding would
> be
>         oldcontext = MemoryContextSwitchTo(newedata->assoc_context);
> rather than what is there.

Oh, right.

>>> (Note that in the sole
>>> existing use-case, edata->assoc_context is going to have been set to
>>> CurrentMemoryContext by pq_parse_errornotice, and I see no good reason to
>>> assume that's very long-lived ... in fact, it looks like it's whatever
>>> happens to be active when ProcessInterrupts is called, which means there's
>>> probably a totally separate set of problems here having to do with
>>> potential leaks into long-lived contexts.)
>> Oops.  Yes.
> I'm not quite sure what to do about this.  It feels a tad wrong to use
> ErrorContext as the active context during HandleParallelMessages, but
> what else should we use?  Maybe this needs its very own private context
> that we can reset after each use?

If we use ErrorContext, will anything go wrong?

>>> I have little use for the name of that function either, as it's not
>>> necessarily going to "throw" anything.  Maybe ReportErrorUsingData,
>>> or something like that?
>> I deliberately avoided that sort of terminology because it need not be
>> an ERROR.  It can be, say, a NOTICE.  It is definitely something that
>> is coming from an ErrorData but it need not be an ERROR.
> Right, but to me, "throw" generally means a transfer of control, which
> is exactly what this isn't going to do if the message is only a notice.

Fair point.

Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to