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
