Make unboxing args sound with ArgAbi

[workingjubilee]

This makes moving arguments into pgrx's pg_extern functions sound, by using a new trait, ArgAbi, to describe this behavior. The supporting Arg and Args types allow effectively treating the arguments of the function like an iterator, but the behavior is much more unsafe.

The victories

• A #[pg_extern] fn must be sound in its arguments and return types.
• A large amount of our code generation for argument unboxing is now driven entirely by type inference, which simplifies the code analysis we must do.

The collateral damage

• This kills a ton of our aggregate examples because they were relying on 'static lifetimes and #[pg_aggregate] does not yet understand how to pass through lifetimes correctly.
• Arguments borrowed for 'static now fail to compile, even in a hypothetical case it might be okay. This is because pgrx attempts to translate an argument from a lifetime-bound FcInfo into the 'static lifetime, which is correctly rejected by Rust's type system.
• Returning borrows with the 'static lifetime also fails, for similar reasons. This affects even hypothetical cases it would be okay (e.g. if the return was actually a &'static str). Types that are inherently 'static like String are still fine.

Future work

• We're going to bump into a lot of bad ergonomics very shortly as people test the new bounds, and will have to hammer out new API to make the desired patterns easy to write.
• This does not retire FromDatum and thus doesn't hammer the nail in the coffin on the unsoundness in the numerous SPI functions that use FromDatum.
• I have not yet begun to cull all the now-unnecessary code that tries to parse out types, but given that arguments are now driven from inference? There's a lot of it.

[workingjubilee]

oh, right, tests.

[workingjubilee]

Most of the current failures are from this, which will require the most thought. https://blog.rust-lang.org/2022/10/28/gats-stabilization.html#implied-static-requirement-from-higher-ranked-trait-bounds

[NotGyro]

I've read through this, I'm very impressed with how much cleaner / more sensible the lifetimes are with these changes. Looks good to me apart from a few documentation nitpicks.

[NotGyro]

Nit - could this be within the scope of this pull request? "Argument" does describe it, though, I don't mind this.

[workingjubilee]

Oh, so AnyElement has a special type definition relative to Postgres. Our AnyElement is here:

pgrx/pgrx/src/datum/anyelement.rs

Lines 22 to 26 in 7ea2480

	#[derive(Debug, Clone, Copy)]
	pub struct AnyElement {
	datum: pg_sys::Datum,
	typoid: pg_sys::Oid,
	}

And it's modeling this type: https://www.postgresql.org/docs/current/datatype-pseudo.html

But there may have to be a bit of additional work to make a wrapper around Argument usable as AnyElement. It's based on the use-cases.

[eeeebbbbrrrr]

My only question is: What's the plan for Aggregates?

Also, do you have a simple example of what a #[pg_extern] function expands to now? I feel like I just recently wrote some docs that kinda outlines that, and we should probably update those too.

[eeeebbbbrrrr]

What's the plan for this? Aggregate support is a pretty important feature

[workingjubilee]

The problem isn't with Aggregate, per se, it's with certain types.

These are aggregates that are supposed to manage composite types. But we know composite types are allocated in Postgres, thus are effectively lifetime-bound. But #[pg_aggregate] and Aggregate don't know how to convey the lifetimes through. Thus even annotating it with something like this does not actually fix the problem:

#[pg_aggregate]
impl<'a> Aggregate for SumScritches<'a> {
    type State = i32;
    const INITIAL_CONDITION: Option<&'static str> = Some("0");
    type Args = pgrx::name!(value, pgrx::composite_type!('a, "Dog"));

    fn state(
        current: Self::State,
        arg: Self::Args,
        _fcinfo: pg_sys::FunctionCallInfo,
    ) -> Self::State {
        todo!()
    }
}

Thus in order to make aggregates work for complex cases we must break the world for them as well, making all their functions use the correct, lifetime-bound types. Hopefully this will let us simplify the code expansion for them as well.

[eeeebbbbrrrr]

This has clearly been a ton of work! I'm excited!!

[workingjubilee]

@eeeebbbbrrrr

The wrapper function will always expand like this:

#[no_mangle]
#[doc(hidden)]
pub unsafe extern "C" fn substring_wrapper(
    fcinfo: ::pgrx::pg_sys::FunctionCallInfo,
) -> ::pgrx::pg_sys::Datum {
    fn _internal_wrapper<'fcx>(
        fcinfo: &mut ::pgrx::callconv::FcInfo<'fcx>,
    ) -> ::pgrx::datum::Datum<'fcx> {
        #[allow(unused_unsafe)]
        unsafe {
            let call_flow = <&str as ::pgrx::callconv::RetAbi>::check_and_prepare(fcinfo);
            let result = match call_flow {
                ::pgrx::callconv::CallCx::WrappedFn(mcx) => {
                    let mut mcx = ::pgrx::PgMemoryContexts::For(mcx);
                    let _args = &mut fcinfo.arguments();
                    let call_result = mcx.switch_to(|_| {
                        let input_ = _args.unbox_next_unchecked().unwrap_or_else(|| {
                            ::core::panicking::panic_fmt(format_args!(
                                "unboxing {0} argument failed",
                                "input_"
                            ));
                        });
                        let start_ = _args.unbox_next_unchecked().unwrap_or_else(|| {
                            ::core::panicking::panic_fmt(format_args!(
                                "unboxing {0} argument failed",
                                "start_"
                            ));
                        });
                        let end_ = _args.unbox_next_unchecked().unwrap_or_else(|| {
                            ::core::panicking::panic_fmt(format_args!(
                                "unboxing {0} argument failed",
                                "end_"
                            ));
                        });
                        substring(input_, start_, end_)
                    });
                    ::pgrx::callconv::RetAbi::to_ret(call_result)
                }
                ::pgrx::callconv::CallCx::RestoreCx => {
                    <&str as ::pgrx::callconv::RetAbi>::ret_from_fcx(fcinfo)
                }
            };
            ::core::mem::transmute(unsafe {
                <&str as ::pgrx::callconv::RetAbi>::box_ret_in(fcinfo, result)
            })
        }
    }
    let datum = unsafe {
        ::pgrx::pg_sys::submodules::panic::pgrx_extern_c_guard(|| {
            let mut fcinfo = ::pgrx::callconv::FcInfo::from_ptr(fcinfo);
            _internal_wrapper(&mut fcinfo)
        })
    };
    datum.sans_lifetime()
}

There is no meaningful variation except based on number of arguments.

[eeeebbbbrrrr]

...
::core::mem::transmute(unsafe {
                <&str as ::pgrx::callconv::RetAbi>::box_ret_in(fcinfo, result)
            })
...

What is the thing being transmuted here? Is it the lifetime of the return value?

[workingjubilee]

Correct.

I wonder if I can erase that...

[workingjubilee]

Yeah, that lifetime transmutation seems to be completely unnecessary now.

[workingjubilee]

lol, well, CI isn't going to prove it anytime soon.

[workingjubilee]

I wasn't quite happy with the remainder of the point of InOutFuncs being part of the collateral damage, so I adjusted it to better support the quirky behavior of Postgres on that count. Regardless of what I think about the API design, the fact that its test failed seemed bad for correctness, especially given how easy it was to incur. Now, if you ask for, say, a Nullable<Option<&'a CStr>>? Then everything should behave correctly, and you should get exactly what you would expect. This sequence isn't actually in the code due to the sheer levels of argument forwarding, but the result is this:

if nullable_datum.isnull {
    Nullable::None
} else if nullable_datum.value == pg_sys::Datum::null() {
    Nullable::Valid(None)
} else {
    Nullable::Valid(Some(the_conversion(nullable_datum.value)))
}

[workingjubilee]

argh.

[jamessewell]

So this lays the ground work for pg_agg, but that work isn't happening atm is it?

[workingjubilee]

correct.