Re: Error-safe user functions

From: Andres Freund <andres(at)anarazel(dot)de>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Joe Conway <mail(at)joeconway(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Corey Huinker <corey(dot)huinker(at)gmail(dot)com>, Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Error-safe user functions
Date: 2022-12-06 02:23:23
Message-ID: 20221206022323.p5y6s4q57ngzrwqx@awork3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On 2022-12-05 20:19:26 -0500, Tom Lane wrote:
> That seems like kind of a problematic requirement, unless we leave some
> datatype around that's intentionally not ever going to be converted. For
> datatypes that we do convert, there shouldn't be any easy way to get to a
> hard error.

I suspect there are going to be types we can't convert. But even if not - that
actually makes a *stronger* case for ensuring the path is tested, because
certainly some out of core types aren't going to be converted.

This made me look at fmgr/README again:

> +Considering datatype input functions as examples, typical "safe" error
> +conditions include input syntax errors and out-of-range values. An input
> +function typically detects such cases with simple if-tests and can easily
> +change the following ereport call to errsave. Error conditions that
> +should NOT be handled this way include out-of-memory, internal errors, and
> +anything where there is any question about our ability to continue normal
> +processing of the transaction. Those should still be thrown with ereport.

I wonder if we should provide more guidance around what kind of catalogs
access are acceptable before avoiding throwing an error.

This in turn make me look at record_in() in 0002 - I think we might be leaking
a tupledesc refcount in case of errors. Yup:

DROP TABLE IF EXISTS tbl_as_record, tbl_with_record;

CREATE TABLE tbl_as_record(a int, b int);
CREATE TABLE tbl_with_record(composite_col tbl_as_record, non_composite_col int);

COPY tbl_with_record FROM stdin WITH (warn_on_error);
kdjkdf 212
\.

WARNING: 22P02: invalid input for column composite_col: malformed record literal: "kdjkdf"
WARNING: 01000: TupleDesc reference leak: TupleDesc 0x7fb1c5fd0c58 (159584,-1) still referenced

> I don't really quite understand why you're worried about that
> though. The hard-error code paths are well tested already.

Afaict they're not tested when going through InputFunctionCallSafe() / with an
ErrorSaveContext. To me that does seem worth testing.

Greetings,

Andres Freund

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2022-12-06 02:32:23 Re: Error-safe user functions
Previous Message Richard Guo 2022-12-06 01:58:01 Re: MemoizePath fails to work for partitionwise join