From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
cc: Andrew Dunstan <andrew@dunslane.net>,
        "David G. Johnston" <david.g.johnston@gmail.com>,
        Corey Huinker <corey.huinker@gmail.com>,
        Alvaro Herrera <alvherre@alvh.no-ip.org>,
        Joe Conway <mail@joeconway.com>, Robert Haas <robertmhaas@gmail.com>,
        Nikita Glukhov <n.gluhov@postgrespro.ru>,
        PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Error-safe user functions
In-reply-to: <20221207233518.sudtaxn6omtvpzvr@awork3.anarazel.de>
References: <3523582.1670288815@sss.pgh.pa.us>
 <40ff1ed1-4245-e343-bffa-c7755b031492@dunslane.net>
 <CADkLM=coVJ0Hy-yYtXbN_dhfc453zs_BUTTPHmJqv12Urkd79g@mail.gmail.com>
 <3889877.1670358069@sss.pgh.pa.us>
 <a6e7ac7e-30ec-5c9d-5134-16072aefc0d7@dunslane.net>
 <4009543.1670422833@sss.pgh.pa.us>
 <6acddf68-7cb7-e9e7-a5a7-0abc87b3c55b@dunslane.net>
 <CAKFQuwYdQqQru9tpMpY4JpCEJ8OeN8kwmBG6DY2QyzE+5znA6Q@mail.gmail.com>
 <4111458.1670444176@sss.pgh.pa.us> <3211.1670452341@sss.pgh.pa.us>
 <20221207233518.sudtaxn6omtvpzvr@awork3.anarazel.de>
Comments: In-reply-to Andres Freund <andres@anarazel.de>
	message dated "Wed, 07 Dec 2022 15:35:18 -0800"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <11183.1670457161.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Wed, 07 Dec 2022 18:52:41 -0500
Message-ID: <11184.1670457161@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/11184.1670457161%40sss.pgh.pa.us>
Precedence: bulk

Andres Freund <andres@anarazel.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 d=
on't
> do get_error_stack_entry() when !details_wanted, due to not raising an e=
rror
> 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 i=
t'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 th=
e
> 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 the=
re"
> it's just about not wanting to incur overhead when the message isn't log=
ged,
> but here we'll always have >=3D ERROR, but ->details_wanted can still le=
ad 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