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
