Input and output functions should be strict

[willmurnane]

Per discussion in Discord this morning, the input and output functions (used to convert from {json, strings, varlena} to custom types) should either be strict or have Option<T> as parameters. Otherwise, statements like SELECT NULL::customtype will crash the server.

[willmurnane]

FYI: This will probably not build until #640 is merged.

[workingjubilee]

Are we sure these are not going to be Options on the return?
Or does that not matter and I am merely perplexed?

[willmurnane]

I think there are a couple of ways this could work:

• Functions are strict, return a concrete value, and panic! if they can't return a value
• Functions are non-strict, return a concrete value, and panic! if they can't return a value
• Functions are non-strict, return an Option, and error! or warn! or whatever if they decide they want to, and just return None sometimes.
• Functions are non-strict, and return a Result (or even Result<Option>) which is somehow transmuted into a panic! in the Err case.

In the first case, NULL always translates to NULL as a custom type. I think this is reasonable, but maybe there's some domain where NULL should translate to something else: an empty string, 0, banana flavor. If we think we need to support that case, then this option won't work.

In the second, everyone has to handle Options, but hey it's not too bad. This doesn't expose a way of explicitly returning NULL from non-null input text.

The third is probably my favorite of the non-strict options. Explicit handling of input and output NULLs, panic or error or whatever if you want to.

The fourth is maybe more rust-friendly, if this function is going to be called from non-pgx code. Something like this (and similar for PgVarlenaInOutFuncs and JsonInOutFuncs):

pub trait InOutFuncs<E> {
    fn input(input: &Option<crate::cstr_core::CStr>) -> Result<Option<Self>, E>
    where
        Self: Sized;
    fn output(&self, buffer: &mut StringInfo);
}

This would need a small shim to unwrap the result, but shouldn't be too bad.

[Hoverbear]

This is a good catch. I'm a bit puzzled why we didn't do this before...

[Hoverbear]

So one curiousity I have is should we make these in/out functions accept options and not be strict? Does making these strict prevent a desirable use case?

[willmurnane]

Closing in favor of #646, which provides a fully fleshed out implementation that exposes all of the PostgreSQL-allowed behavior to consumers of this library in a safe way.