Unbox types properly from arrays #1476
Conversation
Add a lifetime bound to array iterators, so serialize works.
This reverts commit 6106a14.
They no longer pass borrowck.
| } | ||
| } | ||
|
|
||
| pub fn staticize_lifetimes(value: &mut syn::Type) { |
There was a problem hiding this comment.
goodby, staticize_lifetimes... you will not be missed!
| @@ -55,7 +55,7 @@ fn warning(s: &str) -> bool { | |||
| #[pg_extern] | |||
| fn exec<'a>( | |||
| command: &'a str, | |||
| args: default!(Vec<Option<&'a str>>, "ARRAY[]::text[]"), | |||
| args: default!(Vec<Option<String>>, "ARRAY[]::text[]"), | |||
There was a problem hiding this comment.
Just a question... why this change?
Could it not be Vec<Option<&'a str>>?
This was the only example you touched, so either that means this example is the only one that accepts a &str and that pattern is no longer valid, or you changed this for some other reason?
There was a problem hiding this comment.
Because this changes FromDatum in the impls in pgrx/src/datum/array.rs to be in terms of T: UnboxDatum with certain bounds, this PR currently breaks FromDatum specifically for Vec<Option<T>> where there is any doubt as to whether T implements UnboxDatum. Essentially, the trait resolver can't figure it out at that depth. This can be solved by involving additional traits which work differently and either allow Vec<Option<T>> to not depend directly on Array<'_, T> for its unboxing or to move pg_getarg (or its replacement) off T: FromDatum.
GitHub won't open it implicitly because it is a "large diff", but unfortunately, pgrx-tests/src/tests/heap_tuple.rs betrays how many cases won't work now, and so this isn't really finished work. I had a PR similar to this a couple months ago, which did (more) damage, but I needed all the work until now to understand whether it would be viable to even fix, and to fix a number of less-complex cases that hinged on that groundwork. I need to add in a bunch more changes to make all of the cases work that need to work properly. Some of those hinge on SPI, which started busted and remains busted. ...Ultimately I am unconcerned about making |
|
I added the currently-broken tests to the set of ui tests, and reorganized tests slightly, so that it will be easier for us to notice when those problems are fixed. |
workingjubilee
changed the title
Lifetimes should be named for their upper bound, and Datum<'dat> is bound on the source of the Datum.
Introducing
Datum<'src>allows using it with HRTB fns in schema generation to persist lifetimes all the way through#[pg_extern]'s wrapper-fn infrastructure without compromising the integrity of lifetime annotations. This is immediately implemented to prevent many types from exceeding the lifetime of the Array they are unboxed from, unless they are already "self-owned" and thus 'static.Unfortunately, this does not fix all instances using
pg_getarg, and those simply are left unsolved for now in some cases. I reorganized the UI tests to make it easier to notice these problems and when they get solved.