From: | Robert Haas <robertmhaas(at)gmail(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: PG_DIAG_SEVERITY and a possible bug in pq_parse_errornotice() |
Date: | 2016-08-26 15:04:16 |
Message-ID: | CA+Tgmob_sBvnxjdJCE2NLrpry0x942XqWva149uKGbyOOcmhUw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Fri, Aug 26, 2016 at 10:35 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> I wrote:
>> After sleeping on it, I think the right answer is to introduce the new
>> error-message field (and not worry about 9.5). Will work on a patch
>> for that, unless I hear objections pretty soon.
>
> BTW, while I'm looking at this: what on god's green earth is
> ThrowErrorData doing copying the supplied data into edata->assoc_context?
> Surely it should be putting the data into the ErrorContext, where it's not
> going to get flushed before it can be reported?
Uh, well, perhaps I misinterpreted the comment in elog.h. It says this:
/* context containing associated non-constant strings */
struct MemoryContextData *assoc_context;
That sure looks like it's saying that all of the pointers stored in
the ErrorData structure should be pointing into assoc_context, unless
they are constant. If that's not right, I suggest rewording that
comment, because I cannot think of a second interpretation of what's
written there.
> (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 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.
Also, I think "throwing an error" is pretty standard terminology that
is understandable to pretty much all programmers these days. It's not
a perfect name; maybe ReportErrorData would have been better, but
changing that seems like pointless tinkering at this stage, from my
point of view.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
From | Date | Subject | |
---|---|---|---|
Next Message | Joshua D. Drake | 2016-08-26 15:11:12 | Re: Renaming of pg_xlog and pg_clog |
Previous Message | Stephen Frost | 2016-08-26 14:50:33 | Re: Renaming of pg_xlog and pg_clog |