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: http://www.postgresql.org/mailpref/pgsql-hackers
