Re: Error-safe user functions

Andres Freund <andres(at)anarazel(dot)de> writes:
> I wonder if there are potential use-cases for levels other than ERROR. I can
> potentially see us wanting to defer some FATALs, e.g. when they occur in
> process exit hooks.

I thought about that early on, and concluded not. The whole thing is
moot for levels less than ERROR, of course, and I'm having a hard
time seeing how it could be useful for FATAL or PANIC. Maybe I just
lack imagination, but if a call is specifying FATAL rather than just
ERROR then it seems to me it's already a special snowflake rather
than something we could fold into a generic non-error behavior.

> For a moment I was worried that it could lead to odd behaviour that we don't
> do get_error_stack_entry() when !details_wanted, due to not raising an error
> we'd otherwise raise. But that's a should-never-be-reached case, so ...

I don't see how. Returning false out of errsave_start causes the
errsave macro to immediately give control back to the caller, which
will go on about its business.

> It seems somewhat ugly transport this knowledge via edata->elevel, but it's
> not too bad.

The LOG-vs-ERROR business, you mean? Yeah. I considered adding another
bool flag to ErrorData, but couldn't convince myself it was worth the
trouble. If we find a problem we can do that sometime in future.

>> +/*
>> + * We cannot include nodes.h yet, so make a stub reference. (This is also
>> + * used by fmgr.h, which doesn't want to depend on nodes.h either.)
>> + */
>> +typedef struct Node *NodePtr;

> Seems like it'd be easier to just forward declare the struct, and use the
> non-typedef'ed name in the header than to have to deal with these
> interdependencies and the differing typenames.

Meh. I'm a little allergic to writing "struct foo *" in function argument
lists, because I so often see gcc pointing out that if struct foo isn't
yet known then that can silently mean something different than you
intended. With the typedef, it either works or is an error, no halfway
about it. And the struct way isn't really much better in terms of
having two different notations to use rather than only one.

> Perhaps worth noting here that the reason why the errsave_start/errsave_finish
> split exist differs a bit from the reason in ereport_domain()? "Over there"
> it's just about not wanting to incur overhead when the message isn't logged,
> but here we'll always have >= ERROR, but ->details_wanted can still lead to
> not wanting to incur the overhead.

Hmmm ... it seems like the same reason to me, we don't want to incur the
overhead if the "start" function says not to.

> Is it ok that we throw an error in lookup_rowtype_tupdesc()?

Yeah, that should fall in the category of internal errors I think.
I don't see how you'd reach that from a bad input string.

(Or to be more precise, the point of pg_input_is_valid is to tell
you whether the input string is valid, not to tell you whether the
type name is valid; if you're worried about the latter you need
a separate and earlier test.)

regards, tom lane