From 4fa87bd57d5e673e280c987a75cd8129ff349f43 Mon Sep 17 00:00:00 2001 From: Jubilee <46493976+workingjubilee@users.noreply.github.com> Date: Wed, 17 Jan 2024 14:12:15 -0800 Subject: [PATCH] Unbox types properly from arrays (#1476) 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. --- pgrx-examples/bad_ideas/src/lib.rs | 2 +- pgrx-macros/src/lib.rs | 16 + pgrx-macros/src/rewriter.rs | 2 +- pgrx-sql-entity-graph/src/lifetimes.rs | 70 -- .../src/metadata/function_metadata.rs | 54 +- pgrx-sql-entity-graph/src/pg_extern/mod.rs | 27 +- .../src/postgres_type/mod.rs | 47 +- pgrx-sql-entity-graph/src/used_type.rs | 91 +-- pgrx-tests/src/tests/array_tests.rs | 53 +- pgrx-tests/src/tests/heap_tuple.rs | 686 +++++++++--------- pgrx-tests/src/tests/roundtrip_tests.rs | 92 ++- pgrx-tests/src/tests/variadic_tests.rs | 5 +- .../tests/compile-fail/arrays-dont-leak.rs | 39 + .../compile-fail/arrays-dont-leak.stderr | 108 +++ .../eq-for-postgres_hash.rs} | 0 .../eq-for-postgres_hash.stderr} | 4 +- .../escaping-spiclient-1209-cursor.rs | 0 .../escaping-spiclient-1209-cursor.stderr | 8 +- .../escaping-spiclient-1209-prep-stmt.rs | 0 .../escaping-spiclient-1209-prep-stmt.stderr | 2 +- .../compile-fail/heap-tuples-dont-leak.rs | 29 + .../compile-fail/heap-tuples-dont-leak.stderr | 26 + .../total-eq-for-postgres_eq.rs} | 0 .../total-eq-for-postgres_eq.stderr} | 2 +- .../total-ord-for-postgres_ord.rs} | 0 .../total-ord-for-postgres_ord.stderr} | 2 +- pgrx-tests/tests/todo/array-serialize-json.rs | 32 + .../tests/todo/array-serialize-json.stderr | 22 + .../tests/todo/busted-exotic-signature.rs | 38 + .../tests/todo/busted-exotic-signature.stderr | 15 + .../todo/composite-types-broken-on-spi.rs | 252 +++++++ .../todo/composite-types-broken-on-spi.stderr | 71 ++ pgrx-tests/tests/todo/for-dog-in-dogs.rs | 109 +++ pgrx-tests/tests/todo/for-dog-in-dogs.stderr | 47 ++ .../tests/todo/random-vec-strs-arent-okay.rs | 28 + .../todo/random-vec-strs-arent-okay.stderr | 15 + pgrx-tests/tests/todo/roundtrip-tests.rs | 86 +++ pgrx-tests/tests/todo/roundtrip-tests.stderr | 161 ++++ ...-option-composite_type-nesting-problems.rs | 102 +++ ...ion-composite_type-nesting-problems.stderr | 47 ++ pgrx-tests/tests/ui.rs | 12 +- pgrx/src/datum/array.rs | 178 +++-- pgrx/src/datum/mod.rs | 185 +++-- pgrx/src/datum/unbox.rs | 345 +++++++++ pgrx/src/fcinfo.rs | 1 + pgrx/src/heap_tuple.rs | 66 +- pgrx/src/pgbox.rs | 2 +- pgrx/src/spi.rs | 8 +- 48 files changed, 2427 insertions(+), 760 deletions(-) create mode 100644 pgrx-tests/tests/compile-fail/arrays-dont-leak.rs create mode 100644 pgrx-tests/tests/compile-fail/arrays-dont-leak.stderr rename pgrx-tests/tests/{ui/eq_for_postgres_hash.rs => compile-fail/eq-for-postgres_hash.rs} (100%) rename pgrx-tests/tests/{ui/eq_for_postgres_hash.stderr => compile-fail/eq-for-postgres_hash.stderr} (91%) rename pgrx-tests/tests/{ui => compile-fail}/escaping-spiclient-1209-cursor.rs (100%) rename pgrx-tests/tests/{ui => compile-fail}/escaping-spiclient-1209-cursor.stderr (88%) rename pgrx-tests/tests/{ui => compile-fail}/escaping-spiclient-1209-prep-stmt.rs (100%) rename pgrx-tests/tests/{ui => compile-fail}/escaping-spiclient-1209-prep-stmt.stderr (87%) create mode 100644 pgrx-tests/tests/compile-fail/heap-tuples-dont-leak.rs create mode 100644 pgrx-tests/tests/compile-fail/heap-tuples-dont-leak.stderr rename pgrx-tests/tests/{ui/total_eq_for_postgres_eq.rs => compile-fail/total-eq-for-postgres_eq.rs} (100%) rename pgrx-tests/tests/{ui/total_eq_for_postgres_eq.stderr => compile-fail/total-eq-for-postgres_eq.stderr} (90%) rename pgrx-tests/tests/{ui/total_ord_for_postgres_ord.rs => compile-fail/total-ord-for-postgres_ord.rs} (100%) rename pgrx-tests/tests/{ui/total_ord_for_postgres_ord.stderr => compile-fail/total-ord-for-postgres_ord.stderr} (90%) create mode 100644 pgrx-tests/tests/todo/array-serialize-json.rs create mode 100644 pgrx-tests/tests/todo/array-serialize-json.stderr create mode 100644 pgrx-tests/tests/todo/busted-exotic-signature.rs create mode 100644 pgrx-tests/tests/todo/busted-exotic-signature.stderr create mode 100644 pgrx-tests/tests/todo/composite-types-broken-on-spi.rs create mode 100644 pgrx-tests/tests/todo/composite-types-broken-on-spi.stderr create mode 100644 pgrx-tests/tests/todo/for-dog-in-dogs.rs create mode 100644 pgrx-tests/tests/todo/for-dog-in-dogs.stderr create mode 100644 pgrx-tests/tests/todo/random-vec-strs-arent-okay.rs create mode 100644 pgrx-tests/tests/todo/random-vec-strs-arent-okay.stderr create mode 100644 pgrx-tests/tests/todo/roundtrip-tests.rs create mode 100644 pgrx-tests/tests/todo/roundtrip-tests.stderr create mode 100644 pgrx-tests/tests/todo/vec-option-composite_type-nesting-problems.rs create mode 100644 pgrx-tests/tests/todo/vec-option-composite_type-nesting-problems.stderr create mode 100644 pgrx/src/datum/unbox.rs diff --git a/pgrx-examples/bad_ideas/src/lib.rs b/pgrx-examples/bad_ideas/src/lib.rs index 976083510..3394fe823 100644 --- a/pgrx-examples/bad_ideas/src/lib.rs +++ b/pgrx-examples/bad_ideas/src/lib.rs @@ -55,7 +55,7 @@ fn warning(s: &str) -> bool { #[pg_extern] fn exec<'a>( command: &'a str, - args: default!(Vec>, "ARRAY[]::text[]"), + args: default!(Vec>, "ARRAY[]::text[]"), ) -> TableIterator<'static, (name!(status, Option), name!(stdout, String))> { let mut command = &mut Command::new(command); diff --git a/pgrx-macros/src/lib.rs b/pgrx-macros/src/lib.rs index fe2425319..3b1181ac8 100644 --- a/pgrx-macros/src/lib.rs +++ b/pgrx-macros/src/lib.rs @@ -618,6 +618,7 @@ pub fn postgres_enum(input: TokenStream) -> TokenStream { fn impl_postgres_enum(ast: DeriveInput) -> syn::Result { let mut stream = proc_macro2::TokenStream::new(); let sql_graph_entity_ast = ast.clone(); + let generics = &ast.generics; let enum_ident = &ast.ident; let enum_name = enum_ident.to_string(); @@ -660,6 +661,14 @@ fn impl_postgres_enum(ast: DeriveInput) -> syn::Result } } + unsafe impl #generics ::pgrx::datum::UnboxDatum for #enum_ident #generics { + type As<'dat> = #enum_ident #generics where Self: 'dat; + #[inline] + unsafe fn unbox<'dat>(d: ::pgrx::datum::Datum<'dat>) -> Self::As<'dat> where Self: 'dat { + Self::from_datum(::core::mem::transmute(d), false).unwrap() + } + } + impl ::pgrx::datum::IntoDatum for #enum_ident { #[inline] fn into_datum(self) -> Option<::pgrx::pg_sys::Datum> { @@ -808,6 +817,13 @@ fn impl_postgres_type(ast: DeriveInput) -> syn::Result } } } + + unsafe impl #generics ::pgrx::datum::UnboxDatum for #name #generics { + type As<'dat> = Self where Self: 'dat; + unsafe fn unbox<'dat>(datum: ::pgrx::datum::Datum<'dat>) -> Self::As<'dat> where Self: 'dat { + ::from_datum(::core::mem::transmute(datum), false).unwrap() + } + } } ) } diff --git a/pgrx-macros/src/rewriter.rs b/pgrx-macros/src/rewriter.rs index 7954a032c..4c5932611 100644 --- a/pgrx-macros/src/rewriter.rs +++ b/pgrx-macros/src/rewriter.rs @@ -102,7 +102,7 @@ pub fn item_fn_without_rewrite(mut func: ItemFn) -> syn::Result syn::TypePath { - let mut ty = syn::Type::Path(value); - staticize_lifetimes(&mut ty); - match ty { - syn::Type::Path(type_path) => type_path, - - // shouldn't happen - _ => panic!("not a TypePath"), - } -} - -pub fn staticize_lifetimes(value: &mut syn::Type) { - match value { - syn::Type::Path(syn::TypePath { path: syn::Path { segments, .. }, .. }) => segments - .iter_mut() - .filter_map(|segment| match &mut segment.arguments { - syn::PathArguments::AngleBracketed(bracketed) => Some(bracketed), - _ => None, - }) - .flat_map(|bracketed| &mut bracketed.args) - .for_each(|arg| match arg { - // rename lifetimes to the static lifetime so the TypeIds match. - syn::GenericArgument::Lifetime(lifetime) => { - lifetime.ident = syn::Ident::new("static", lifetime.ident.span()); - } - // recurse - syn::GenericArgument::Type(ty) => staticize_lifetimes(ty), - syn::GenericArgument::Binding(binding) => staticize_lifetimes(&mut binding.ty), - syn::GenericArgument::Constraint(constraint) => { - constraint.bounds.iter_mut().for_each(|bound| { - if let syn::TypeParamBound::Lifetime(lifetime) = bound { - lifetime.ident = syn::Ident::new("static", lifetime.ident.span()) - } - }) - } - // nothing to do otherwise - _ => {} - }), - - syn::Type::Reference(type_ref) => { - if let Some(lifetime) = &mut type_ref.lifetime { - lifetime.ident = syn::Ident::new("static", lifetime.ident.span()); - } else { - type_ref.lifetime = Some(syn::parse_quote!('static)); - } - } - - syn::Type::Tuple(type_tuple) => type_tuple.elems.iter_mut().for_each(staticize_lifetimes), - - syn::Type::Macro(syn::TypeMacro { mac }) - if mac.path.segments.last().is_some_and(|seg| seg.ident == "name") => - { - // We don't particularly care what the identifier is, so we parse a - // raw TokenStream. Specifically, it's okay for the identifier String, - // which we end up using as a Postgres column name, to be nearly any - // string, which can include Rust reserved words such as "type" or "match" - let Ok(out) = mac.parse_body::() else { return }; - let Ok(ident) = syn::parse_str::(&out.ident) else { return }; - let mut ty = out.used_ty.resolved_ty; - - // rewrite the name!() macro's type so that it has a static lifetime, if any - staticize_lifetimes(&mut ty); - *mac = syn::parse_quote! {::pgrx::name!(#ident, #ty)}; - } - _ => {} - } -} pub fn anonymize_lifetimes_in_type_path(value: syn::TypePath) -> syn::TypePath { let mut ty = syn::Type::Path(value); diff --git a/pgrx-sql-entity-graph/src/metadata/function_metadata.rs b/pgrx-sql-entity-graph/src/metadata/function_metadata.rs index 9d57ab508..49e78051c 100644 --- a/pgrx-sql-entity-graph/src/metadata/function_metadata.rs +++ b/pgrx-sql-entity-graph/src/metadata/function_metadata.rs @@ -42,53 +42,30 @@ pub trait FunctionMetadata { fn entity(&self) -> FunctionMetadataEntity; } -impl FunctionMetadata<()> for fn() -> R -where - R: SqlTranslatable, -{ - fn entity(&self) -> FunctionMetadataEntity { - FunctionMetadataEntity { - arguments: vec![], - retval: { - let marker: PhantomData = PhantomData; - marker.entity() - }, - path: self.path(), - } - } -} - -impl FunctionMetadata<()> for unsafe fn() -> R -where - R: SqlTranslatable, -{ - fn entity(&self) -> FunctionMetadataEntity { - FunctionMetadataEntity { - arguments: vec![], - retval: { - let marker: PhantomData = PhantomData; - marker.entity() - }, - path: self.path(), - } - } -} - macro_rules! impl_fn { - ($($T:ident),* $(,)?) => { - impl<$($T: SqlTranslatable,)* R: SqlTranslatable> FunctionMetadata<($($T,)*)> for fn($($T,)*) -> R { + ($($A:ident),* $(,)?) => { + impl<$($A,)* R, F> FunctionMetadata<($($A,)*)> for F + where + $($A: SqlTranslatable,)* + R: SqlTranslatable, + F: FnMut($($A,)*) -> R, + { fn entity(&self) -> FunctionMetadataEntity { FunctionMetadataEntity { - arguments: vec![$(PhantomData::<$T>.entity()),+], + arguments: vec![$(PhantomData::<$A>.entity()),*], retval: PhantomData::.entity(), path: self.path(), } } } - impl<$($T: SqlTranslatable,)* R: SqlTranslatable> FunctionMetadata<($($T,)*)> for unsafe fn($($T,)*) -> R { + impl<$($A,)* R> FunctionMetadata<($($A,)*)> for unsafe fn($($A,)*) -> R + where + $($A: SqlTranslatable,)* + R: SqlTranslatable, + { fn entity(&self) -> FunctionMetadataEntity { FunctionMetadataEntity { - arguments: vec![$(PhantomData::<$T>.entity()),+], + arguments: vec![$(PhantomData::<$A>.entity()),*], retval: PhantomData::.entity(), path: self.path(), } @@ -96,7 +73,8 @@ macro_rules! impl_fn { } }; } -// empty tuples are above + +impl_fn!(); impl_fn!(T0); impl_fn!(T0, T1); impl_fn!(T0, T1, T2); diff --git a/pgrx-sql-entity-graph/src/pg_extern/mod.rs b/pgrx-sql-entity-graph/src/pg_extern/mod.rs index f0bc02275..44d485935 100644 --- a/pgrx-sql-entity-graph/src/pg_extern/mod.rs +++ b/pgrx-sql-entity-graph/src/pg_extern/mod.rs @@ -34,7 +34,6 @@ use search_path::SearchPathList; use crate::enrich::CodeEnrichment; use crate::enrich::ToEntityGraphTokens; use crate::enrich::ToRustCodeTokens; -use crate::lifetimes::staticize_lifetimes; use proc_macro2::{Ident, Span, TokenStream as TokenStream2}; use quote::{quote, quote_spanned, ToTokens}; use syn::parse::{Parse, ParseStream, Parser}; @@ -133,12 +132,11 @@ impl PgExtern { match v { syn::FnArg::Receiver(_) => None, syn::FnArg::Typed(pat_ty) => { - let mut static_ty = match UsedType::new(*pat_ty.ty.clone()) { + let ty = match UsedType::new(*pat_ty.ty.clone()) { Ok(v) => v.resolved_ty, Err(e) => return Some(Err(e)), }; - staticize_lifetimes(&mut static_ty); - Some(Ok(static_ty)) + Some(Ok(ty)) } } }) @@ -275,9 +273,8 @@ impl PgExtern { let return_type = match &self.func.sig.output { syn::ReturnType::Default => None, syn::ReturnType::Type(arrow, ty) => { - let mut static_ty = ty.clone(); - staticize_lifetimes(&mut static_ty); - Some(syn::ReturnType::Type(*arrow, static_ty)) + let ty = ty.clone(); + Some(syn::ReturnType::Type(*arrow, ty)) } }; @@ -287,6 +284,20 @@ impl PgExtern { Some(content) => ToSqlConfig { content: Some(content), ..self.to_sql_config.clone() }, }; + let lifetimes = self + .func + .sig + .generics + .params + .iter() + .filter_map(|generic| match generic { + // syn::GenericParam::Type(_) => todo!(), + syn::GenericParam::Lifetime(lt) => Some(lt), + _ => None, // syn::GenericParam::Const(_) => todo!(), + }) + .collect::>(); + let hrtb = if lifetimes.is_empty() { None } else { Some(quote! { for<#(#lifetimes),*> }) }; + let sql_graph_entity_fn_name = syn::Ident::new(&format!("__pgrx_internals_fn_{}", ident), Span::call_site()); quote_spanned! { self.func.sig.span() => @@ -297,7 +308,7 @@ impl PgExtern { extern crate alloc; #[allow(unused_imports)] use alloc::{vec, vec::Vec}; - type FunctionPointer = #unsafety fn(#( #input_types ),*) #return_type; + type FunctionPointer = #hrtb #unsafety fn (#( #input_types ),*) #return_type; let metadata: FunctionPointer = #ident; let submission = ::pgrx::pgrx_sql_entity_graph::PgExternEntity { name: #name, diff --git a/pgrx-sql-entity-graph/src/postgres_type/mod.rs b/pgrx-sql-entity-graph/src/postgres_type/mod.rs index da3ad8765..fffc79188 100644 --- a/pgrx-sql-entity-graph/src/postgres_type/mod.rs +++ b/pgrx-sql-entity-graph/src/postgres_type/mod.rs @@ -21,7 +21,7 @@ use crate::enrich::{ToEntityGraphTokens, ToRustCodeTokens}; use proc_macro2::{Ident, Span, TokenStream as TokenStream2}; use quote::quote; use syn::parse::{Parse, ParseStream}; -use syn::{DeriveInput, Generics, ItemStruct}; +use syn::{DeriveInput, Generics, ItemStruct, Lifetime, LifetimeDef}; use crate::{CodeEnrichment, ToSqlConfig}; @@ -103,37 +103,24 @@ impl PostgresTypeDerive { impl ToEntityGraphTokens for PostgresTypeDerive { fn to_entity_graph_tokens(&self) -> TokenStream2 { let name = &self.name; - let mut static_generics = self.generics.clone(); - static_generics.params = static_generics - .params - .clone() - .into_iter() - .flat_map(|param| match param { - item @ syn::GenericParam::Type(_) | item @ syn::GenericParam::Const(_) => { - Some(item) - } - syn::GenericParam::Lifetime(mut lifetime) => { - lifetime.lifetime.ident = Ident::new("static", Span::call_site()); - Some(syn::GenericParam::Lifetime(lifetime)) - } - }) - .collect(); - let mut staticless_generics = self.generics.clone(); - staticless_generics.params = static_generics + let generics = self.generics.clone(); + let (impl_generics, ty_generics, where_clauses) = generics.split_for_impl(); + + // We need some generics we can use inside a fn without a lifetime for qualified paths. + let mut anon_generics = generics.clone(); + anon_generics.params = anon_generics .params - .clone() .into_iter() .flat_map(|param| match param { item @ syn::GenericParam::Type(_) | item @ syn::GenericParam::Const(_) => { Some(item) } - syn::GenericParam::Lifetime(_) => None, + syn::GenericParam::Lifetime(lt_def) => Some(syn::GenericParam::Lifetime( + LifetimeDef::new(Lifetime::new("'_", lt_def.lifetime.span())), + )), }) .collect(); - let (staticless_impl_generics, _staticless_ty_generics, _staticless_where_clauses) = - staticless_generics.split_for_impl(); - let (_static_impl_generics, static_ty_generics, static_where_clauses) = - static_generics.split_for_impl(); + let (_, anon_ty_gen, _) = anon_generics.split_for_impl(); let in_fn = &self.in_fn; let out_fn = &self.out_fn; @@ -144,7 +131,7 @@ impl ToEntityGraphTokens for PostgresTypeDerive { let to_sql_config = &self.to_sql_config; quote! { - unsafe impl #staticless_impl_generics ::pgrx::pgrx_sql_entity_graph::metadata::SqlTranslatable for #name #static_ty_generics #static_where_clauses { + unsafe impl #impl_generics ::pgrx::pgrx_sql_entity_graph::metadata::SqlTranslatable for #name #ty_generics #where_clauses { fn argument_sql() -> core::result::Result<::pgrx::pgrx_sql_entity_graph::metadata::SqlMapping, ::pgrx::pgrx_sql_entity_graph::metadata::ArgumentError> { Ok(::pgrx::pgrx_sql_entity_graph::metadata::SqlMapping::As(String::from(stringify!(#name)))) } @@ -166,19 +153,19 @@ impl ToEntityGraphTokens for PostgresTypeDerive { use ::pgrx::datum::WithTypeIds; let mut mappings = Default::default(); - <#name #static_ty_generics as ::pgrx::datum::WithTypeIds>::register_with_refs( + <#name #anon_ty_gen as ::pgrx::datum::WithTypeIds>::register_with_refs( &mut mappings, stringify!(#name).to_string() ); - ::pgrx::datum::WithSizedTypeIds::<#name #static_ty_generics>::register_sized_with_refs( + ::pgrx::datum::WithSizedTypeIds::<#name #anon_ty_gen>::register_sized_with_refs( &mut mappings, stringify!(#name).to_string() ); - ::pgrx::datum::WithArrayTypeIds::<#name #static_ty_generics>::register_array_with_refs( + ::pgrx::datum::WithArrayTypeIds::<#name #anon_ty_gen>::register_array_with_refs( &mut mappings, stringify!(#name).to_string() ); - ::pgrx::datum::WithVarlenaTypeIds::<#name #static_ty_generics>::register_varlena_with_refs( + ::pgrx::datum::WithVarlenaTypeIds::<#name #anon_ty_gen>::register_varlena_with_refs( &mut mappings, stringify!(#name).to_string() ); @@ -187,7 +174,7 @@ impl ToEntityGraphTokens for PostgresTypeDerive { file: file!(), line: line!(), module_path: module_path!(), - full_path: core::any::type_name::<#name #static_ty_generics>(), + full_path: core::any::type_name::<#name #anon_ty_gen>(), mappings: mappings.into_iter().collect(), in_fn: stringify!(#in_fn), in_fn_module_path: { diff --git a/pgrx-sql-entity-graph/src/used_type.rs b/pgrx-sql-entity-graph/src/used_type.rs index 0da7e095b..925714f24 100644 --- a/pgrx-sql-entity-graph/src/used_type.rs +++ b/pgrx-sql-entity-graph/src/used_type.rs @@ -18,7 +18,7 @@ to the `pgrx` framework and very subject to change between versions. While you m use std::ops::Deref; use crate::composite_type::{handle_composite_type_macro, CompositeTypeMacro}; -use crate::lifetimes::staticize_lifetimes; +use crate::lifetimes::anonymize_lifetimes; use proc_macro2::Span; use quote::ToTokens; use syn::parse::{Parse, ParseStream}; @@ -82,7 +82,7 @@ impl UsedType { match archetype.ident.to_string().as_str() { "variadic" => { let ty: syn::Type = syn::parse2(mac.tokens.clone())?; - syn::parse_quote! { ::pgrx::datum::VariadicArray<'static, #ty>} + syn::parse_quote! { ::pgrx::datum::VariadicArray<'_, #ty>} } _ => syn::Type::Macro(macro_pat), } @@ -105,11 +105,9 @@ impl UsedType { ))?; } "composite_type" => { - let composite_type = Some(handle_composite_type_macro(mac)?); - let ty = syn::parse_quote! { - ::pgrx::heap_tuple::PgHeapTuple<'static, ::pgrx::pgbox::AllocatedByRust> - }; - (ty, composite_type) + let composite_macro = handle_composite_type_macro(mac)?; + let ty = composite_macro.expand_with_lifetime(); + (ty, Some(composite_macro)) } _ => (syn::Type::Macro(macro_pat), None), } @@ -313,9 +311,14 @@ impl UsedType { pub fn entity_tokens(&self) -> syn::Expr { let mut resolved_ty = self.resolved_ty.clone(); let mut resolved_ty_inner = self.resolved_ty_inner.clone().unwrap_or(resolved_ty.clone()); - staticize_lifetimes(&mut resolved_ty); - staticize_lifetimes(&mut resolved_ty_inner); - let resolved_ty_string = resolved_ty.to_token_stream().to_string().replace(' ', ""); + // The lifetimes of these are not relevant. Previously, we solved this by staticizing them + // but we want to avoid staticizing in this codebase going forward. Anonymization makes it + // easier to name the lifetime-bounded objects without the context for those lifetimes, + // without erasing all possible distinctions, since anon lifetimes may still be disunited. + // Non-static lifetimes, however, require the use of the NonStaticTypeId hack. + anonymize_lifetimes(&mut resolved_ty); + anonymize_lifetimes(&mut resolved_ty_inner); + let resolved_ty_string = resolved_ty.to_token_stream().to_string(); let composite_type = self.composite_type.clone().map(|v| v.expr); let composite_type_iter = composite_type.iter(); let variadic = &self.variadic; @@ -428,12 +431,6 @@ fn resolve_variadic_array_inner( match last.arguments { syn::PathArguments::AngleBracketed(ref mut path_arg) => { - match path_arg.args.first_mut() { - Some(syn::GenericArgument::Lifetime(lifetime)) => { - lifetime.ident = syn::Ident::new("static", lifetime.ident.span()) - } - _ => path_arg.args.insert(0, syn::parse_quote!('static)), - }; match path_arg.args.last() { // TODO: Lifetime???? Some(syn::GenericArgument::Type(ty)) => match ty.clone() { @@ -449,7 +446,7 @@ fn resolve_variadic_array_inner( let comp_ty = composite_mac.expand_with_lifetime(); let sql = Some(composite_mac); let ty = syn::parse_quote! { - ::pgrx::datum::VariadicArray<'static, #comp_ty> + ::pgrx::datum::VariadicArray<'_, #comp_ty> }; Ok((ty, sql)) } @@ -464,7 +461,7 @@ fn resolve_variadic_array_inner( if last.ident == "Option" { let (inner_ty, expr) = resolve_option_inner(arg_type_path)?; let wrapped_ty = syn::parse_quote! { - ::pgrx::datum::VariadicArray<'static, #inner_ty> + ::pgrx::datum::VariadicArray<'_, #inner_ty> }; Ok((wrapped_ty, expr)) } else { @@ -491,19 +488,12 @@ fn resolve_array_inner( .ok_or(syn::Error::new(original_span, "Could not read last segment of path"))?; match last.arguments { - syn::PathArguments::AngleBracketed(ref mut path_arg) => { - match path_arg.args.first_mut() { - Some(syn::GenericArgument::Lifetime(lifetime)) => { - lifetime.ident = syn::Ident::new("static", lifetime.ident.span()) - } - _ => path_arg.args.insert(0, syn::parse_quote!('static)), - }; - match path_arg.args.last() { - Some(syn::GenericArgument::Type(ty)) => match ty.clone() { - syn::Type::Macro(macro_pat) => { - let mac = ¯o_pat.mac; - let archetype = mac.path.segments.last().expect("No last segment"); - match archetype.ident.to_string().as_str() { + syn::PathArguments::AngleBracketed(ref mut path_arg) => match path_arg.args.last() { + Some(syn::GenericArgument::Type(ty)) => match ty.clone() { + syn::Type::Macro(macro_pat) => { + let mac = ¯o_pat.mac; + let archetype = mac.path.segments.last().expect("No last segment"); + match archetype.ident.to_string().as_str() { "default" => { Err(syn::Error::new(mac.span(), "`VariadicArray` not supported, choose `default!(VariadicArray, ident)` instead")) } @@ -512,34 +502,33 @@ fn resolve_array_inner( let comp_ty = composite_mac.expand_with_lifetime(); let sql = Some(composite_mac); let ty = syn::parse_quote! { - ::pgrx::datum::Array<'static, #comp_ty> + ::pgrx::datum::Array<'_, #comp_ty> }; Ok((ty, sql)) } _ => Ok((syn::Type::Path(original), None)), } - } - syn::Type::Path(arg_type_path) => { - let last = arg_type_path.path.segments.last().ok_or(syn::Error::new( - arg_type_path.span(), - "No last segment in type path", - ))?; - match last.ident.to_string().as_str() { - "Option" => { - let (inner_ty, expr) = resolve_option_inner(arg_type_path)?; - let wrapped_ty = syn::parse_quote! { - ::pgrx::datum::Array<'static, #inner_ty> - }; - Ok((wrapped_ty, expr)) - } - _ => Ok((syn::Type::Path(original), None)), + } + syn::Type::Path(arg_type_path) => { + let last = arg_type_path.path.segments.last().ok_or(syn::Error::new( + arg_type_path.span(), + "No last segment in type path", + ))?; + match last.ident.to_string().as_str() { + "Option" => { + let (inner_ty, expr) = resolve_option_inner(arg_type_path)?; + let wrapped_ty = syn::parse_quote! { + ::pgrx::datum::Array<'_, #inner_ty> + }; + Ok((wrapped_ty, expr)) } + _ => Ok((syn::Type::Path(original), None)), } - _ => Ok((syn::Type::Path(original), None)), - }, + } _ => Ok((syn::Type::Path(original), None)), - } - } + }, + _ => Ok((syn::Type::Path(original), None)), + }, _ => Ok((syn::Type::Path(original), None)), } } diff --git a/pgrx-tests/src/tests/array_tests.rs b/pgrx-tests/src/tests/array_tests.rs index 35e39c4fc..097ee6c9c 100644 --- a/pgrx-tests/src/tests/array_tests.rs +++ b/pgrx-tests/src/tests/array_tests.rs @@ -63,10 +63,11 @@ fn optional_array_with_default(values: default!(Option>, "NULL")) -> values.unwrap().iter().map(|v| v.unwrap_or(0)).sum() } -#[pg_extern] -fn serde_serialize_array(values: Array<&str>) -> Json { - Json(json! { { "values": values } }) -} +// TODO: fix this test by fixing serde impls for `Array<'a, &'a str> -> Json` +// #[pg_extern] +// fn serde_serialize_array<'dat>(values: Array<'dat, &'dat str>) -> Json { +// Json(json! { { "values": values } }) +// } #[pg_extern] fn serde_serialize_array_i32(values: Array) -> Json { @@ -161,19 +162,12 @@ fn enum_array_roundtrip(a: Array) -> Vec> { a.into_iter().collect() } -#[pg_extern] -fn array_echo<'a>(a: Array<&'a str>) -> Vec> { - let v = a.iter().collect(); - drop(a); - v -} - #[pg_extern] fn validate_cstring_array<'a>( a: Array<'a, &'a core::ffi::CStr>, ) -> std::result::Result> { assert_eq!( - a.into_iter().collect::>(), + a.iter().collect::>(), vec![ Some(core::ffi::CStr::from_bytes_with_nul(b"one\0")?), Some(core::ffi::CStr::from_bytes_with_nul(b"two\0")?), @@ -252,15 +246,16 @@ mod tests { Spi::run("SELECT iterate_array_with_deny_null(ARRAY[1,2,3, NULL]::int[])") } - #[pg_test] - fn test_serde_serialize_array() -> Result<(), pgrx::spi::Error> { - let json = Spi::get_one::( - "SELECT serde_serialize_array(ARRAY['one', null, 'two', 'three'])", - )? - .expect("returned json was null"); - assert_eq!(json.0, json! {{"values": ["one", null, "two", "three"]}}); - Ok(()) - } + // TODO: fix this test by redesigning SPI. + // #[pg_test] + // fn test_serde_serialize_array() -> Result<(), pgrx::spi::Error> { + // let json = Spi::get_one::( + // "SELECT serde_serialize_array(ARRAY['one', null, 'two', 'three'])", + // )? + // .expect("returned json was null"); + // assert_eq!(json.0, json! {{"values": ["one", null, "two", "three"]}}); + // Ok(()) + // } #[pg_test] fn test_optional_array_with_default() { @@ -438,22 +433,6 @@ mod tests { Ok(()) } - #[pg_test] - fn test_leak_after_drop() -> Result<(), Box> { - Spi::run("create table test_leak_after_drop (a text[]);")?; - Spi::run( - "insert into test_leak_after_drop (a) select array_agg(x::text) from generate_series(1, 10000) x;", - )?; - let array = Spi::get_one::>("SELECT array_echo(a) FROM test_leak_after_drop")? - .expect("datum was null"); - let top_5 = array.iter().take(5).collect::>(); - drop(array); - - // just check the top 5 values. Even the first will be wrong if the backing Array data is freed - assert_eq!(top_5, &[Some("1"), Some("2"), Some("3"), Some("4"), Some("5")]); - Ok(()) - } - #[pg_test] fn test_array_of_points() -> Result<(), Box> { let points: Array = Spi::get_one( diff --git a/pgrx-tests/src/tests/heap_tuple.rs b/pgrx-tests/src/tests/heap_tuple.rs index 6a6d3a938..a8be8de27 100644 --- a/pgrx-tests/src/tests/heap_tuple.rs +++ b/pgrx-tests/src/tests/heap_tuple.rs @@ -45,10 +45,13 @@ mod arguments { use super::*; mod singleton { - use super::*; + //! Originally, these tested taking HeapTuple<'a, _> -> &'a str, but that allows data to + //! escape the lifetime of its root datum which prevents correct reasoning about lifetimes. + //! Perhaps they could eventually test &'tup HeapTuple<'mcx, _> -> &'tup str? + use super::*; #[pg_extern] - fn gets_name_field(dog: Option) -> Option<&str> { + fn gets_name_field(dog: Option) -> Option { // Gets resolved to: let dog: Option> = dog; @@ -58,7 +61,7 @@ mod arguments { #[pg_extern] fn gets_name_field_default( dog: default!(pgrx::composite_type!("Dog"), "ROW('Nami', 0)::Dog"), - ) -> &str { + ) -> String { // Gets resolved to: let dog: PgHeapTuple = dog; @@ -66,7 +69,7 @@ mod arguments { } #[pg_extern] - fn gets_name_field_strict(dog: pgrx::composite_type!("Dog")) -> &str { + fn gets_name_field_strict(dog: pgrx::composite_type!("Dog")) -> String { // Gets resolved to: let dog: PgHeapTuple = dog; @@ -75,6 +78,12 @@ mod arguments { } mod variadic_array { + //! TODO: biggest problem here is the inability to use + //! ```ignore + //! for dog in dogs { + //! todo!() + //! } + //! ``` use super::*; #[pg_extern] @@ -85,7 +94,7 @@ mod arguments { let dogs: pgrx::VariadicArray> = dogs; let mut names = Vec::with_capacity(dogs.len()); - for dog in dogs { + for dog in dogs.iter() { let dog = dog.unwrap(); let name = dog.get_by_name("name").unwrap().unwrap(); names.push(name); @@ -93,6 +102,7 @@ mod arguments { names } + /// TODO: add test which actually calls this maybe??? #[pg_extern] fn gets_name_field_default_variadic( dogs: default!( @@ -104,7 +114,7 @@ mod arguments { let dogs: pgrx::VariadicArray> = dogs; let mut names = Vec::with_capacity(dogs.len()); - for dog in dogs { + for dog in dogs.iter() { let dog = dog.unwrap(); let name = dog.get_by_name("name").unwrap().unwrap(); names.push(name); @@ -130,168 +140,169 @@ mod arguments { } mod vec { - use super::*; - - #[pg_extern] - fn sum_scritches_for_names( - dogs: Option>, - ) -> i32 { - // Gets resolved to: - let dogs: Option>> = dogs; - - let dogs = dogs.unwrap(); - let mut sum_scritches = 0; - for dog in dogs { - let scritches: i32 = - dog.get_by_name("scritches").ok().unwrap_or_default().unwrap_or_default(); - sum_scritches += scritches; - } - sum_scritches - } - - #[pg_extern] - fn sum_scritches_for_names_default( - dogs: pgrx::default!(Vec, "ARRAY[ROW('Nami', 0)]::Dog[]"), - ) -> i32 { - // Gets resolved to: - let dogs: Vec> = dogs; - - let mut sum_scritches = 0; - for dog in dogs { - let scritches: i32 = - dog.get_by_name("scritches").ok().unwrap_or_default().unwrap_or_default(); - sum_scritches += scritches; - } - sum_scritches - } - - #[pg_extern] - fn sum_scritches_for_names_strict(dogs: Vec) -> i32 { - // Gets resolved to: - let dogs: Vec> = dogs; - - let mut sum_scritches = 0; - for dog in dogs { - let scritches: i32 = - dog.get_by_name("scritches").ok().unwrap_or_default().unwrap_or_default(); - sum_scritches += scritches; - } - sum_scritches - } - - #[pg_extern] - fn sum_scritches_for_names_strict_optional_items( - dogs: Vec>, - ) -> i32 { - // Gets resolved to: - let dogs: Vec>> = dogs; - - let mut sum_scritches = 0; - for dog in dogs { - let dog = dog.unwrap(); - let scritches: i32 = - dog.get_by_name("scritches").ok().unwrap_or_default().unwrap_or_default(); - sum_scritches += scritches; - } - sum_scritches - } - - #[pg_extern] - fn sum_scritches_for_names_default_optional_items( - dogs: pgrx::default!( - Vec>, - "ARRAY[ROW('Nami', 0)]::Dog[]" - ), - ) -> i32 { - // Gets resolved to: - let dogs: Vec>> = dogs; - - let mut sum_scritches = 0; - for dog in dogs { - let dog = dog.unwrap(); - let scritches: i32 = - dog.get_by_name("scritches").ok().unwrap_or_default().unwrap_or_default(); - sum_scritches += scritches; - } - sum_scritches - } - - #[pg_extern] - fn sum_scritches_for_names_optional_items( - dogs: Option>>, - ) -> i32 { - // Gets resolved to: - let dogs: Option>>> = dogs; - - let dogs = dogs.unwrap(); - let mut sum_scritches = 0; - for dog in dogs { - let dog = dog.unwrap(); - let scritches: i32 = dog.get_by_name("scritches").unwrap().unwrap(); - sum_scritches += scritches; - } - sum_scritches - } - } - - mod array { - use super::*; - - #[pg_extern] - fn sum_scritches_for_names_array( - dogs: Option>, - ) -> i32 { - // Gets resolved to: - let dogs: Option>> = dogs; - - let dogs = dogs.unwrap(); - let mut sum_scritches = 0; - for dog in dogs { - let dog = dog.unwrap(); - let scritches: i32 = - dog.get_by_name("scritches").ok().unwrap_or_default().unwrap_or_default(); - sum_scritches += scritches; - } - sum_scritches - } - - #[pg_extern] - fn sum_scritches_for_names_array_default( - dogs: pgrx::default!( - pgrx::Array, - "ARRAY[ROW('Nami', 0)]::Dog[]" - ), - ) -> i32 { - // Gets resolved to: - let dogs: pgrx::Array> = dogs; - - let mut sum_scritches = 0; - for dog in dogs { - let dog = dog.unwrap(); - let scritches: i32 = - dog.get_by_name("scritches").ok().unwrap_or_default().unwrap_or_default(); - sum_scritches += scritches; - } - sum_scritches - } - - #[pg_extern] - fn sum_scritches_for_names_array_strict( - dogs: pgrx::Array, - ) -> i32 { - // Gets resolved to: - let dogs: pgrx::Array> = dogs; - - let mut sum_scritches = 0; - for dog in dogs { - let dog = dog.unwrap(); - let scritches: i32 = - dog.get_by_name("scritches").ok().unwrap_or_default().unwrap_or_default(); - sum_scritches += scritches; - } - sum_scritches - } - } + //! TODO: This probably requires the FunctionArguments<'curr, 'fn> type + // use super::*; + + // #[pg_extern] + // fn sum_scritches_for_names( + // dogs: Option>, + // ) -> i32 { + // // Gets resolved to: + // let dogs: Option>> = dogs; + + // let dogs = dogs.unwrap(); + // let mut sum_scritches = 0; + // for dog in dogs { + // let scritches: i32 = + // dog.get_by_name("scritches").ok().unwrap_or_default().unwrap_or_default(); + // sum_scritches += scritches; + // } + // sum_scritches + // } + + // #[pg_extern] + // fn sum_scritches_for_names_default( + // dogs: pgrx::default!(Vec, "ARRAY[ROW('Nami', 0)]::Dog[]"), + // ) -> i32 { + // // Gets resolved to: + // let dogs: Vec> = dogs; + + // let mut sum_scritches = 0; + // for dog in dogs { + // let scritches: i32 = + // dog.get_by_name("scritches").ok().unwrap_or_default().unwrap_or_default(); + // sum_scritches += scritches; + // } + // sum_scritches + // } + + // #[pg_extern] + // fn sum_scritches_for_names_strict(dogs: Vec) -> i32 { + // // Gets resolved to: + // let dogs: Vec> = dogs; + + // let mut sum_scritches = 0; + // for dog in dogs { + // let scritches: i32 = + // dog.get_by_name("scritches").ok().unwrap_or_default().unwrap_or_default(); + // sum_scritches += scritches; + // } + // sum_scritches + // } + + // #[pg_extern] + // fn sum_scritches_for_names_strict_optional_items( + // dogs: Vec>, + // ) -> i32 { + // // Gets resolved to: + // let dogs: Vec>> = dogs; + + // let mut sum_scritches = 0; + // for dog in dogs { + // let dog = dog.unwrap(); + // let scritches: i32 = + // dog.get_by_name("scritches").ok().unwrap_or_default().unwrap_or_default(); + // sum_scritches += scritches; + // } + // sum_scritches + // } + + // #[pg_extern] + // fn sum_scritches_for_names_default_optional_items( + // dogs: pgrx::default!( + // Vec>, + // "ARRAY[ROW('Nami', 0)]::Dog[]" + // ), + // ) -> i32 { + // // Gets resolved to: + // let dogs: Vec>> = dogs; + + // let mut sum_scritches = 0; + // for dog in dogs { + // let dog = dog.unwrap(); + // let scritches: i32 = + // dog.get_by_name("scritches").ok().unwrap_or_default().unwrap_or_default(); + // sum_scritches += scritches; + // } + // sum_scritches + // } + + // #[pg_extern] + // fn sum_scritches_for_names_optional_items( + // dogs: Option>>, + // ) -> i32 { + // // Gets resolved to: + // let dogs: Option>>> = dogs; + + // let dogs = dogs.unwrap(); + // let mut sum_scritches = 0; + // for dog in dogs { + // let dog = dog.unwrap(); + // let scritches: i32 = dog.get_by_name("scritches").unwrap().unwrap(); + // sum_scritches += scritches; + // } + // sum_scritches + // } + } + + // mod array { + // use super::*; + + // #[pg_extern] + // fn sum_scritches_for_names_array( + // dogs: Option>, + // ) -> i32 { + // // Gets resolved to: + // let dogs: Option>> = dogs; + + // let dogs = dogs.unwrap(); + // let mut sum_scritches = 0; + // for dog in dogs { + // let dog = dog.unwrap(); + // let scritches: i32 = + // dog.get_by_name("scritches").ok().unwrap_or_default().unwrap_or_default(); + // sum_scritches += scritches; + // } + // sum_scritches + // } + + // #[pg_extern] + // fn sum_scritches_for_names_array_default( + // dogs: pgrx::default!( + // pgrx::Array, + // "ARRAY[ROW('Nami', 0)]::Dog[]" + // ), + // ) -> i32 { + // // Gets resolved to: + // let dogs: pgrx::Array> = dogs; + + // let mut sum_scritches = 0; + // for dog in dogs { + // let dog = dog.unwrap(); + // let scritches: i32 = + // dog.get_by_name("scritches").ok().unwrap_or_default().unwrap_or_default(); + // sum_scritches += scritches; + // } + // sum_scritches + // } + + // #[pg_extern] + // fn sum_scritches_for_names_array_strict( + // dogs: pgrx::Array, + // ) -> i32 { + // // Gets resolved to: + // let dogs: pgrx::Array> = dogs; + + // let mut sum_scritches = 0; + // for dog in dogs { + // let dog = dog.unwrap(); + // let scritches: i32 = + // dog.get_by_name("scritches").ok().unwrap_or_default().unwrap_or_default(); + // sum_scritches += scritches; + // } + // sum_scritches + // } + // } } // As return types @@ -340,62 +351,63 @@ mod returning { } mod vec { - use super::*; - - #[pg_extern] - fn scritch_all_vec( - maybe_dogs: Option>, - ) -> Option> { - // Gets resolved to: - let maybe_dogs: Option>> = maybe_dogs; - - let maybe_dogs = if let Some(mut dogs) = maybe_dogs { - for dog in dogs.iter_mut() { - dog.set_by_name("scritches", dog.get_by_name::("scritches").unwrap()) - .unwrap(); - } - Some(dogs) - } else { - None - }; - - maybe_dogs - } - - #[pg_extern] - fn scritch_all_vec_strict( - dogs: Vec<::pgrx::composite_type!("Dog")>, - ) -> Vec<::pgrx::composite_type!("Dog")> { - // Gets resolved to: - let mut dogs: Vec> = dogs; - - for dog in dogs.iter_mut() { - dog.set_by_name("scritches", dog.get_by_name::("scritches").unwrap()).unwrap(); - } - dogs - } - - #[pg_extern] - fn scritch_all_vec_optional_items( - maybe_dogs: Option>>, - ) -> Option>> { - // Gets resolved to: - let maybe_dogs: Option>>> = maybe_dogs; - - let maybe_dogs = if let Some(mut dogs) = maybe_dogs { - for dog in dogs.iter_mut() { - if let Some(ref mut dog) = dog { - dog.set_by_name("scritches", dog.get_by_name::("scritches").unwrap()) - .unwrap(); - } - } - Some(dogs) - } else { - None - }; - - maybe_dogs - } + //! TODO: This probably requires the FunctionArguments<'curr, 'fn> type + // use super::*; + + // #[pg_extern] + // fn scritch_all_vec( + // maybe_dogs: Option>, + // ) -> Option> { + // // Gets resolved to: + // let maybe_dogs: Option>> = maybe_dogs; + + // let maybe_dogs = if let Some(mut dogs) = maybe_dogs { + // for dog in dogs.iter_mut() { + // dog.set_by_name("scritches", dog.get_by_name::("scritches").unwrap()) + // .unwrap(); + // } + // Some(dogs) + // } else { + // None + // }; + + // maybe_dogs + // } + + // #[pg_extern] + // fn scritch_all_vec_strict<'a>( + // dogs: Vec<::pgrx::composite_type!('a, "Dog")>, + // ) -> Vec<::pgrx::composite_type!('a, "Dog")> { + // // Gets resolved to: + // let mut dogs: Vec> = dogs; + + // for dog in dogs.iter_mut() { + // dog.set_by_name("scritches", dog.get_by_name::("scritches").unwrap()).unwrap(); + // } + // dogs + // } + + // #[pg_extern] + // fn scritch_all_vec_optional_items( + // maybe_dogs: Option>>, + // ) -> Option>> { + // // Gets resolved to: + // let maybe_dogs: Option>>> = maybe_dogs; + + // let maybe_dogs = if let Some(mut dogs) = maybe_dogs { + // for dog in dogs.iter_mut() { + // if let Some(ref mut dog) = dog { + // dog.set_by_name("scritches", dog.get_by_name::("scritches").unwrap()) + // .unwrap(); + // } + // } + // Some(dogs) + // } else { + // None + // }; + + // maybe_dogs + // } } // Returning VariadicArray/Array isn't supported, use a Vec. @@ -406,34 +418,34 @@ mod returning { mod sql_generator_tests { use super::*; - #[pg_extern] - fn exotic_signature( - _cats: pgrx::default!( - Option>>, - "ARRAY[ROW('Sally', 0)]::Cat[]" - ), - _a_single_fish: pgrx::default!( - Option<::pgrx::composite_type!('static, "Fish")>, - "ROW('Bob', 0)::Fish" - ), - _dogs: pgrx::default!( - Option<::pgrx::VariadicArray<::pgrx::composite_type!('static, "Dog")>>, - "ARRAY[ROW('Nami', 0), ROW('Brandy', 0)]::Dog[]" - ), - ) -> TableIterator< - 'static, - ( - name!(dog, Option<::pgrx::composite_type!('static, "Dog")>), - name!(cat, Option<::pgrx::composite_type!('static, "Cat")>), - name!(fish, Option<::pgrx::composite_type!('static, "Fish")>), - name!( - related_edges, - Option> - ), - ), - > { - TableIterator::new(Vec::new().into_iter()) - } + // #[pg_extern] + // fn exotic_signature( + // _cats: pgrx::default!( + // Option>>, + // "ARRAY[ROW('Sally', 0)]::Cat[]" + // ), + // _a_single_fish: pgrx::default!( + // Option<::pgrx::composite_type!('static, "Fish")>, + // "ROW('Bob', 0)::Fish" + // ), + // _dogs: pgrx::default!( + // Option<::pgrx::VariadicArray<::pgrx::composite_type!('static, "Dog")>>, + // "ARRAY[ROW('Nami', 0), ROW('Brandy', 0)]::Dog[]" + // ), + // ) -> TableIterator< + // 'static, + // ( + // name!(dog, Option<::pgrx::composite_type!('static, "Dog")>), + // name!(cat, Option<::pgrx::composite_type!('static, "Cat")>), + // name!(fish, Option<::pgrx::composite_type!('static, "Fish")>), + // name!( + // related_edges, + // Option> + // ), + // ), + // > { + // TableIterator::new(Vec::new().into_iter()) + // } #[pg_extern] fn iterable_named_table() -> TableIterator< @@ -688,91 +700,91 @@ mod tests { assert_eq!(retval, Ok(Some(vec!["Nami".to_string(), "Brandy".to_string()]))); } - #[pg_test] - fn test_sum_scritches_for_names() { - let retval = Spi::get_one::( - " - SELECT sum_scritches_for_names(ARRAY[ROW('Nami', 1), ROW('Brandy', 42)]::Dog[]) - ", - ); - assert_eq!(retval, Ok(Some(43))); - } - - #[pg_test] - fn test_sum_scritches_for_names_default() { - let retval = Spi::get_one::( - " - SELECT sum_scritches_for_names_default() - ", - ); - assert_eq!(retval, Ok(Some(0))); - } - - #[pg_test] - fn test_sum_scritches_for_names_strict() { - let retval = Spi::get_one::( - " - SELECT sum_scritches_for_names_strict(ARRAY[ROW('Nami', 1), ROW('Brandy', 42)]::Dog[]) - ", - ); - assert_eq!(retval, Ok(Some(43))); - } - - #[pg_test] - fn test_sum_scritches_for_names_strict_optional_items() { - let retval = Spi::get_one::( - " - SELECT sum_scritches_for_names_strict(ARRAY[ROW('Nami', 1), ROW('Brandy', 42)]::Dog[]) - ", - ); - assert_eq!(retval, Ok(Some(43))); - } - - #[pg_test] - fn test_sum_scritches_for_names_default_optional_items() { - let retval = Spi::get_one::( - " - SELECT sum_scritches_for_names_default_optional_items() - ", - ); - assert_eq!(retval, Ok(Some(0))); - } - - #[pg_test] - fn test_sum_scritches_for_names_optional_items() { - let retval = Spi::get_one::(" - SELECT sum_scritches_for_names_optional_items(ARRAY[ROW('Nami', 1), ROW('Brandy', 42)]::Dog[]) - "); - assert_eq!(retval, Ok(Some(43))); - } - - #[pg_test] - fn test_sum_scritches_for_names_array() { - let retval = Spi::get_one::( - " - SELECT sum_scritches_for_names_array(ARRAY[ROW('Nami', 1), ROW('Brandy', 42)]::Dog[]) - ", - ); - assert_eq!(retval, Ok(Some(43))); - } - - #[pg_test] - fn test_sum_scritches_for_names_array_default() { - let retval = Spi::get_one::( - " - SELECT sum_scritches_for_names_array_default() - ", - ); - assert_eq!(retval, Ok(Some(0))); - } - - #[pg_test] - fn test_sum_scritches_for_names_array_strict() { - let retval = Spi::get_one::(" - SELECT sum_scritches_for_names_array_strict(ARRAY[ROW('Nami', 1), ROW('Brandy', 42)]::Dog[]) - "); - assert_eq!(retval, Ok(Some(43))); - } + // #[pg_test] + // fn test_sum_scritches_for_names() { + // let retval = Spi::get_one::( + // " + // SELECT sum_scritches_for_names(ARRAY[ROW('Nami', 1), ROW('Brandy', 42)]::Dog[]) + // ", + // ); + // assert_eq!(retval, Ok(Some(43))); + // } + + // #[pg_test] + // fn test_sum_scritches_for_names_default() { + // let retval = Spi::get_one::( + // " + // SELECT sum_scritches_for_names_default() + // ", + // ); + // assert_eq!(retval, Ok(Some(0))); + // } + + // #[pg_test] + // fn test_sum_scritches_for_names_strict() { + // let retval = Spi::get_one::( + // " + // SELECT sum_scritches_for_names_strict(ARRAY[ROW('Nami', 1), ROW('Brandy', 42)]::Dog[]) + // ", + // ); + // assert_eq!(retval, Ok(Some(43))); + // } + + // #[pg_test] + // fn test_sum_scritches_for_names_strict_optional_items() { + // let retval = Spi::get_one::( + // " + // SELECT sum_scritches_for_names_strict(ARRAY[ROW('Nami', 1), ROW('Brandy', 42)]::Dog[]) + // ", + // ); + // assert_eq!(retval, Ok(Some(43))); + // } + + // #[pg_test] + // fn test_sum_scritches_for_names_default_optional_items() { + // let retval = Spi::get_one::( + // " + // SELECT sum_scritches_for_names_default_optional_items() + // ", + // ); + // assert_eq!(retval, Ok(Some(0))); + // } + + // #[pg_test] + // fn test_sum_scritches_for_names_optional_items() { + // let retval = Spi::get_one::(" + // SELECT sum_scritches_for_names_optional_items(ARRAY[ROW('Nami', 1), ROW('Brandy', 42)]::Dog[]) + // "); + // assert_eq!(retval, Ok(Some(43))); + // } + + // #[pg_test] + // fn test_sum_scritches_for_names_array() { + // let retval = Spi::get_one::( + // " + // SELECT sum_scritches_for_names_array(ARRAY[ROW('Nami', 1), ROW('Brandy', 42)]::Dog[]) + // ", + // ); + // assert_eq!(retval, Ok(Some(43))); + // } + + // #[pg_test] + // fn test_sum_scritches_for_names_array_default() { + // let retval = Spi::get_one::( + // " + // SELECT sum_scritches_for_names_array_default() + // ", + // ); + // assert_eq!(retval, Ok(Some(0))); + // } + + // #[pg_test] + // fn test_sum_scritches_for_names_array_strict() { + // let retval = Spi::get_one::(" + // SELECT sum_scritches_for_names_array_strict(ARRAY[ROW('Nami', 1), ROW('Brandy', 42)]::Dog[]) + // "); + // assert_eq!(retval, Ok(Some(43))); + // } #[pg_test] fn test_create_dog() -> Result<(), pgrx::spi::Error> { diff --git a/pgrx-tests/src/tests/roundtrip_tests.rs b/pgrx-tests/src/tests/roundtrip_tests.rs index b3a2fdcb2..493fca9fb 100644 --- a/pgrx-tests/src/tests/roundtrip_tests.rs +++ b/pgrx-tests/src/tests/roundtrip_tests.rs @@ -53,12 +53,26 @@ mod tests { use super::RandomData; macro_rules! roundtrip { + ($fname:ident, $tname:ident, &$lt:lifetime $rtype:ty, $expected:expr) => { + #[pg_extern(requires = [ "create_complex_type" ])] // the "Complex" type comes from another crate, and we need its schema fully created before it can be used here + fn $fname<$lt>(i: &$lt $rtype) -> &$lt $rtype { + i + } + + roundtrip_test!($fname, $tname, &'_ $rtype, $expected); + }; ($fname:ident, $tname:ident, $rtype:ty, $expected:expr) => { #[pg_extern(requires = [ "create_complex_type" ])] // the "Complex" type comes from another crate, and we need its schema fully created before it can be used here fn $fname(i: $rtype) -> $rtype { i } + roundtrip_test!($fname, $tname, $rtype, $expected); + }; + } + + macro_rules! roundtrip_test { + ($fname:ident, $tname:ident, $rtype:ty, $expected:expr) => { #[pg_test] fn $tname() -> Result<(), Box> { let value: $rtype = $expected; @@ -75,7 +89,7 @@ mod tests { }; } - roundtrip!(rt_bytea, test_rt_bytea, &'static [u8], [b'a', b'b', b'c'].as_slice()); + roundtrip!(rt_bytea, test_rt_bytea, &'a [u8], [b'a', b'b', b'c'].as_slice()); roundtrip!(rt_char, test_rt_char, char, 'a'); roundtrip!(rt_i8, test_rt_i8, i8, i8::MAX); roundtrip!(rt_point, test_rt_point, pg_sys::Point, pg_sys::Point { x: 1.0, y: 2.0 }); @@ -94,7 +108,7 @@ mod tests { ); roundtrip!(rt_i64, test_rt_i64, i64, i64::MAX); roundtrip!(rt_i32, test_rt_i32, i32, i32::MAX); - roundtrip!(rt_refstr, test_rt_refstr, &'static str, "foo"); + roundtrip!(rt_refstr, test_rt_refstr, &'a str, "foo"); roundtrip!(rt_bool, test_rt_bool, bool, true); roundtrip!(rt_f32, test_rt_f32, f32, f32::MAX); roundtrip!(rt_numeric, test_rt_numeric, Numeric<100,0>, Numeric::from_str("31241234123412341234").unwrap()); @@ -104,7 +118,7 @@ mod tests { AnyNumeric, AnyNumeric::from_str("31241234123412341234").unwrap() ); - roundtrip!(rt_cstr, test_rt_cstr, &'static CStr, unsafe { + roundtrip!(rt_cstr, test_rt_cstr, &'a CStr, unsafe { CStr::from_bytes_with_nul_unchecked(b"&cstr\0") }); @@ -139,19 +153,46 @@ mod tests { // arrays of the above // ----------- - roundtrip!( - rt_array_bytea, - test_rt_array_bytea, - Vec>, - vec![ - None, - Some([b'a', b'b', b'c'].as_slice()), - Some([b'd', b'e', b'f'].as_slice()), - None, - Some([b'g', b'h', b'i'].as_slice()), - None - ] - ); + // TODO: Need to fix these array-of-bytea, array-of-text, and array-of-cstr tests, + // because `impl FromDatum for >` is broken enough pg_getarg does not work. + // This likely requires changing the glue code that fetches arguments. + + // roundtrip!( + // rt_array_bytea, + // test_rt_array_bytea, + // 'a, + // Vec>, + // vec![ + // None, + // Some([b'a', b'b', b'c'].as_slice()), + // Some([b'd', b'e', b'f'].as_slice()), + // None, + // Some([b'g', b'h', b'i'].as_slice()), + // None + // ] + // ); + + // roundtrip!( + // rt_array_refstr, + // test_rt_array_refstr, + // 'a, + // Vec>, + // vec![None, Some("foo"), Some("bar"), None, Some("baz"), None] + // ); + + // roundtrip!( + // rt_array_cstr, + // test_rt_array_cstr, + // Vec>, + // vec![ + // None, + // Some(unsafe { CStr::from_bytes_with_nul_unchecked(b"&one\0") }), + // Some(unsafe { CStr::from_bytes_with_nul_unchecked(b"&two\0") }), + // None, + // Some(unsafe { CStr::from_bytes_with_nul_unchecked(b"&three\0") }), + // None, + // ] + // ); roundtrip!( rt_array_char, @@ -250,12 +291,6 @@ mod tests { Vec>, vec![None, Some(i32::MIN), Some(i32::MAX), None, Some(42), None] ); - roundtrip!( - rt_array_refstr, - test_rt_array_refstr, - Vec>, - vec![None, Some("foo"), Some("bar"), None, Some("baz"), None] - ); roundtrip!( rt_array_bool, test_rt_array_bool, @@ -294,19 +329,6 @@ mod tests { None ] ); - roundtrip!( - rt_array_cstr, - test_rt_array_cstr, - Vec>, - vec![ - None, - Some(unsafe { CStr::from_bytes_with_nul_unchecked(b"&one\0") }), - Some(unsafe { CStr::from_bytes_with_nul_unchecked(b"&two\0") }), - None, - Some(unsafe { CStr::from_bytes_with_nul_unchecked(b"&three\0") }), - None, - ] - ); roundtrip!( rt_array_date, test_rt_array_date, diff --git a/pgrx-tests/src/tests/variadic_tests.rs b/pgrx-tests/src/tests/variadic_tests.rs index 6e0cd0bf2..4421e2fbb 100644 --- a/pgrx-tests/src/tests/variadic_tests.rs +++ b/pgrx-tests/src/tests/variadic_tests.rs @@ -13,7 +13,10 @@ mod test { use pgrx::VariadicArray; #[pg_extern] - fn func_with_variadic_array_args(_field: &str, values: VariadicArray<&str>) -> String { + fn func_with_variadic_array_args<'dat>( + _field: &str, + values: VariadicArray<'dat, &'dat str>, + ) -> String { values.get(0).unwrap().unwrap().to_string() } } diff --git a/pgrx-tests/tests/compile-fail/arrays-dont-leak.rs b/pgrx-tests/tests/compile-fail/arrays-dont-leak.rs new file mode 100644 index 000000000..4ef3bd235 --- /dev/null +++ b/pgrx-tests/tests/compile-fail/arrays-dont-leak.rs @@ -0,0 +1,39 @@ +use pgrx::prelude::*; + +fn main() {} + +#[pg_extern] +fn array_echo_a<'a>(a: Array<'a, &'a str>) -> Vec> { + let v = a.iter().collect(); + drop(a); + v +} + +#[pg_extern] +fn array_echo_aba<'a, 'b>(a: Array<'a, &'b str>) -> Vec> { + let v = a.iter().collect(); + drop(a); + v +} + +#[pg_extern] +fn array_echo_baa<'a, 'b>(a: Array<'b, &'a str>) -> Vec> { + let v = a.iter().collect(); + drop(a); + v +} + +fn test_leak_after_drop() -> Result<(), Box> { + Spi::run("create table test_leak_after_drop (a text[]);")?; + Spi::run( + "insert into test_leak_after_drop (a) select array_agg(x::text) from generate_series(1, 10000) x;", + )?; + let array = Spi::get_one::>("SELECT array_echo(a) FROM test_leak_after_drop")? + .expect("datum was null"); + let top_5 = array.iter().take(5).collect::>(); + drop(array); + + // just check the top 5 values. Even the first will be wrong if the backing Array data is freed + assert_eq!(top_5, &[Some("1"), Some("2"), Some("3"), Some("4"), Some("5")]); + Ok(()) +} diff --git a/pgrx-tests/tests/compile-fail/arrays-dont-leak.stderr b/pgrx-tests/tests/compile-fail/arrays-dont-leak.stderr new file mode 100644 index 000000000..016b59c7b --- /dev/null +++ b/pgrx-tests/tests/compile-fail/arrays-dont-leak.stderr @@ -0,0 +1,108 @@ +error[E0505]: cannot move out of `a` because it is borrowed + --> tests/compile-fail/arrays-dont-leak.rs:8:10 + | +6 | fn array_echo_a<'a>(a: Array<'a, &'a str>) -> Vec> { + | -- - binding `a` declared here + | | + | lifetime `'a` defined here +7 | let v = a.iter().collect(); + | - borrow of `a` occurs here +8 | drop(a); + | ^ move out of `a` occurs here +9 | v + | - returning this value requires that `a` is borrowed for `'a` + +error[E0515]: cannot return value referencing function parameter `a` + --> tests/compile-fail/arrays-dont-leak.rs:9:5 + | +7 | let v = a.iter().collect(); + | - `a` is borrowed here +8 | drop(a); +9 | v + | ^ returns a value referencing data owned by the current function + +error[E0505]: cannot move out of `a` because it is borrowed + --> tests/compile-fail/arrays-dont-leak.rs:15:10 + | +13 | fn array_echo_aba<'a, 'b>(a: Array<'a, &'b str>) -> Vec> { + | -- - binding `a` declared here + | | + | lifetime `'a` defined here +14 | let v = a.iter().collect(); + | - borrow of `a` occurs here +15 | drop(a); + | ^ move out of `a` occurs here +16 | v + | - returning this value requires that `a` is borrowed for `'a` + +error: lifetime may not live long enough + --> tests/compile-fail/arrays-dont-leak.rs:16:5 + | +13 | fn array_echo_aba<'a, 'b>(a: Array<'a, &'b str>) -> Vec> { + | -- -- lifetime `'b` defined here + | | + | lifetime `'a` defined here +... +16 | v + | ^ function was supposed to return data with lifetime `'a` but it is returning data with lifetime `'b` + | + = help: consider adding the following bound: `'b: 'a` + +error[E0515]: cannot return value referencing function parameter `a` + --> tests/compile-fail/arrays-dont-leak.rs:16:5 + | +14 | let v = a.iter().collect(); + | - `a` is borrowed here +15 | drop(a); +16 | v + | ^ returns a value referencing data owned by the current function + +error[E0505]: cannot move out of `a` because it is borrowed + --> tests/compile-fail/arrays-dont-leak.rs:22:10 + | +20 | fn array_echo_baa<'a, 'b>(a: Array<'b, &'a str>) -> Vec> { + | -- - binding `a` declared here + | | + | lifetime `'a` defined here +21 | let v = a.iter().collect(); + | - borrow of `a` occurs here +22 | drop(a); + | ^ move out of `a` occurs here +23 | v + | - returning this value requires that `a` is borrowed for `'a` + +error: lifetime may not live long enough + --> tests/compile-fail/arrays-dont-leak.rs:23:5 + | +20 | fn array_echo_baa<'a, 'b>(a: Array<'b, &'a str>) -> Vec> { + | -- -- lifetime `'b` defined here + | | + | lifetime `'a` defined here +... +23 | v + | ^ function was supposed to return data with lifetime `'a` but it is returning data with lifetime `'b` + | + = help: consider adding the following bound: `'b: 'a` + +error[E0515]: cannot return value referencing function parameter `a` + --> tests/compile-fail/arrays-dont-leak.rs:23:5 + | +21 | let v = a.iter().collect(); + | - `a` is borrowed here +22 | drop(a); +23 | v + | ^ returns a value referencing data owned by the current function + +error[E0505]: cannot move out of `array` because it is borrowed + --> tests/compile-fail/arrays-dont-leak.rs:34:10 + | +31 | let array = Spi::get_one::>("SELECT array_echo(a) FROM test_leak_after_drop")? + | ----- binding `array` declared here +32 | .expect("datum was null"); +33 | let top_5 = array.iter().take(5).collect::>(); + | ----- borrow of `array` occurs here +34 | drop(array); + | ^^^^^ move out of `array` occurs here +... +37 | assert_eq!(top_5, &[Some("1"), Some("2"), Some("3"), Some("4"), Some("5")]); + | --------------------------------------------------------------------------- borrow later used here diff --git a/pgrx-tests/tests/ui/eq_for_postgres_hash.rs b/pgrx-tests/tests/compile-fail/eq-for-postgres_hash.rs similarity index 100% rename from pgrx-tests/tests/ui/eq_for_postgres_hash.rs rename to pgrx-tests/tests/compile-fail/eq-for-postgres_hash.rs diff --git a/pgrx-tests/tests/ui/eq_for_postgres_hash.stderr b/pgrx-tests/tests/compile-fail/eq-for-postgres_hash.stderr similarity index 91% rename from pgrx-tests/tests/ui/eq_for_postgres_hash.stderr rename to pgrx-tests/tests/compile-fail/eq-for-postgres_hash.stderr index 34894d1d4..32aaa1579 100644 --- a/pgrx-tests/tests/ui/eq_for_postgres_hash.stderr +++ b/pgrx-tests/tests/compile-fail/eq-for-postgres_hash.stderr @@ -1,5 +1,5 @@ error[E0277]: the trait bound `BrokenType: std::cmp::Eq` is not satisfied - --> tests/ui/eq_for_postgres_hash.rs:4:48 + --> tests/compile-fail/eq-for-postgres_hash.rs:4:48 | 4 | #[derive(Serialize, Deserialize, PostgresType, PostgresHash)] | ^^^^^^^^^^^^ the trait `std::cmp::Eq` is not implemented for `BrokenType` @@ -13,7 +13,7 @@ help: consider annotating `BrokenType` with `#[derive(Eq)]` | error[E0277]: the trait bound `BrokenType: std::hash::Hash` is not satisfied - --> tests/ui/eq_for_postgres_hash.rs:4:48 + --> tests/compile-fail/eq-for-postgres_hash.rs:4:48 | 4 | #[derive(Serialize, Deserialize, PostgresType, PostgresHash)] | ^^^^^^^^^^^^ the trait `std::hash::Hash` is not implemented for `BrokenType` diff --git a/pgrx-tests/tests/ui/escaping-spiclient-1209-cursor.rs b/pgrx-tests/tests/compile-fail/escaping-spiclient-1209-cursor.rs similarity index 100% rename from pgrx-tests/tests/ui/escaping-spiclient-1209-cursor.rs rename to pgrx-tests/tests/compile-fail/escaping-spiclient-1209-cursor.rs diff --git a/pgrx-tests/tests/ui/escaping-spiclient-1209-cursor.stderr b/pgrx-tests/tests/compile-fail/escaping-spiclient-1209-cursor.stderr similarity index 88% rename from pgrx-tests/tests/ui/escaping-spiclient-1209-cursor.stderr rename to pgrx-tests/tests/compile-fail/escaping-spiclient-1209-cursor.stderr index 94fbebec3..49e00e101 100644 --- a/pgrx-tests/tests/ui/escaping-spiclient-1209-cursor.stderr +++ b/pgrx-tests/tests/compile-fail/escaping-spiclient-1209-cursor.stderr @@ -1,5 +1,5 @@ error: lifetime may not live long enough - --> tests/ui/escaping-spiclient-1209-cursor.rs:8:9 + --> tests/compile-fail/escaping-spiclient-1209-cursor.rs:8:9 | 7 | let mut res = Spi::connect(|c| { | -- return type of closure is SpiTupleTable<'2> @@ -11,7 +11,7 @@ error: lifetime may not live long enough | |_____________________^ returning this value requires that `'1` must outlive `'2` error[E0515]: cannot return value referencing temporary value - --> tests/ui/escaping-spiclient-1209-cursor.rs:8:9 + --> tests/compile-fail/escaping-spiclient-1209-cursor.rs:8:9 | 8 | c.open_cursor("select 'hello world' from generate_series(1, 1000)", None) | ^------------------------------------------------------------------------ @@ -25,7 +25,7 @@ error[E0515]: cannot return value referencing temporary value = help: use `.collect()` to allocate the iterator error: lifetime may not live long enough - --> tests/ui/escaping-spiclient-1209-cursor.rs:15:26 + --> tests/compile-fail/escaping-spiclient-1209-cursor.rs:15:26 | 15 | Spi::connect(|c| c.open_cursor("select 1", None).fetch(1).unwrap()); | -- ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ returning this value requires that `'1` must outlive `'2` @@ -34,7 +34,7 @@ error: lifetime may not live long enough | has type `SpiClient<'1>` error[E0515]: cannot return value referencing temporary value - --> tests/ui/escaping-spiclient-1209-cursor.rs:15:26 + --> tests/compile-fail/escaping-spiclient-1209-cursor.rs:15:26 | 15 | Spi::connect(|c| c.open_cursor("select 1", None).fetch(1).unwrap()); | -------------------------------^^^^^^^^^^^^^^^^^^ diff --git a/pgrx-tests/tests/ui/escaping-spiclient-1209-prep-stmt.rs b/pgrx-tests/tests/compile-fail/escaping-spiclient-1209-prep-stmt.rs similarity index 100% rename from pgrx-tests/tests/ui/escaping-spiclient-1209-prep-stmt.rs rename to pgrx-tests/tests/compile-fail/escaping-spiclient-1209-prep-stmt.rs diff --git a/pgrx-tests/tests/ui/escaping-spiclient-1209-prep-stmt.stderr b/pgrx-tests/tests/compile-fail/escaping-spiclient-1209-prep-stmt.stderr similarity index 87% rename from pgrx-tests/tests/ui/escaping-spiclient-1209-prep-stmt.stderr rename to pgrx-tests/tests/compile-fail/escaping-spiclient-1209-prep-stmt.stderr index f77459ff7..483312788 100644 --- a/pgrx-tests/tests/ui/escaping-spiclient-1209-prep-stmt.stderr +++ b/pgrx-tests/tests/compile-fail/escaping-spiclient-1209-prep-stmt.stderr @@ -1,5 +1,5 @@ error: lifetime may not live long enough - --> tests/ui/escaping-spiclient-1209-prep-stmt.rs:8:39 + --> tests/compile-fail/escaping-spiclient-1209-prep-stmt.rs:8:39 | 8 | let prepared = { Spi::connect(|c| c.prepare(q, None))? }; | -- ^^^^^^^^^^^^^^^^^^ returning this value requires that `'1` must outlive `'2` diff --git a/pgrx-tests/tests/compile-fail/heap-tuples-dont-leak.rs b/pgrx-tests/tests/compile-fail/heap-tuples-dont-leak.rs new file mode 100644 index 000000000..6107cefdf --- /dev/null +++ b/pgrx-tests/tests/compile-fail/heap-tuples-dont-leak.rs @@ -0,0 +1,29 @@ +use ::pgrx::prelude::*; + +fn main() {} + +#[pg_extern] +fn gets_name_field(dog: Option) -> Option<&str> { + // Gets resolved to: + let dog: Option> = dog; + + dog?.get_by_name("name").ok()? +} + +#[pg_extern] +fn gets_name_field_default( + dog: default!(pgrx::composite_type!("Dog"), "ROW('Nami', 0)::Dog"), +) -> &str { + // Gets resolved to: + let dog: PgHeapTuple = dog; + + dog.get_by_name("name").unwrap().unwrap() +} + +#[pg_extern] +fn gets_name_field_strict(dog: pgrx::composite_type!("Dog")) -> &str { + // Gets resolved to: + let dog: PgHeapTuple = dog; + + dog.get_by_name("name").unwrap().unwrap() +} diff --git a/pgrx-tests/tests/compile-fail/heap-tuples-dont-leak.stderr b/pgrx-tests/tests/compile-fail/heap-tuples-dont-leak.stderr new file mode 100644 index 000000000..b69603025 --- /dev/null +++ b/pgrx-tests/tests/compile-fail/heap-tuples-dont-leak.stderr @@ -0,0 +1,26 @@ +error[E0515]: cannot return value referencing temporary value + --> tests/compile-fail/heap-tuples-dont-leak.rs:10:5 + | +10 | dog?.get_by_name("name").ok()? + | ----^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | returns a value referencing data owned by the current function + | temporary value created here + +error[E0515]: cannot return value referencing local variable `dog` + --> tests/compile-fail/heap-tuples-dont-leak.rs:20:5 + | +20 | dog.get_by_name("name").unwrap().unwrap() + | ---^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | returns a value referencing data owned by the current function + | `dog` is borrowed here + +error[E0515]: cannot return value referencing local variable `dog` + --> tests/compile-fail/heap-tuples-dont-leak.rs:28:5 + | +28 | dog.get_by_name("name").unwrap().unwrap() + | ---^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | returns a value referencing data owned by the current function + | `dog` is borrowed here diff --git a/pgrx-tests/tests/ui/total_eq_for_postgres_eq.rs b/pgrx-tests/tests/compile-fail/total-eq-for-postgres_eq.rs similarity index 100% rename from pgrx-tests/tests/ui/total_eq_for_postgres_eq.rs rename to pgrx-tests/tests/compile-fail/total-eq-for-postgres_eq.rs diff --git a/pgrx-tests/tests/ui/total_eq_for_postgres_eq.stderr b/pgrx-tests/tests/compile-fail/total-eq-for-postgres_eq.stderr similarity index 90% rename from pgrx-tests/tests/ui/total_eq_for_postgres_eq.stderr rename to pgrx-tests/tests/compile-fail/total-eq-for-postgres_eq.stderr index 5f4923756..bf72b7ab2 100644 --- a/pgrx-tests/tests/ui/total_eq_for_postgres_eq.stderr +++ b/pgrx-tests/tests/compile-fail/total-eq-for-postgres_eq.stderr @@ -1,5 +1,5 @@ error[E0277]: the trait bound `BrokenType: std::cmp::Eq` is not satisfied - --> tests/ui/total_eq_for_postgres_eq.rs:4:59 + --> tests/compile-fail/total-eq-for-postgres_eq.rs:4:59 | 4 | #[derive(Serialize, Deserialize, PartialEq, PostgresType, PostgresEq)] | ^^^^^^^^^^ the trait `std::cmp::Eq` is not implemented for `BrokenType` diff --git a/pgrx-tests/tests/ui/total_ord_for_postgres_ord.rs b/pgrx-tests/tests/compile-fail/total-ord-for-postgres_ord.rs similarity index 100% rename from pgrx-tests/tests/ui/total_ord_for_postgres_ord.rs rename to pgrx-tests/tests/compile-fail/total-ord-for-postgres_ord.rs diff --git a/pgrx-tests/tests/ui/total_ord_for_postgres_ord.stderr b/pgrx-tests/tests/compile-fail/total-ord-for-postgres_ord.stderr similarity index 90% rename from pgrx-tests/tests/ui/total_ord_for_postgres_ord.stderr rename to pgrx-tests/tests/compile-fail/total-ord-for-postgres_ord.stderr index ca51b84ce..498005356 100644 --- a/pgrx-tests/tests/ui/total_ord_for_postgres_ord.stderr +++ b/pgrx-tests/tests/compile-fail/total-ord-for-postgres_ord.stderr @@ -1,5 +1,5 @@ error[E0277]: the trait bound `BrokenType: std::cmp::Ord` is not satisfied - --> tests/ui/total_ord_for_postgres_ord.rs:4:71 + --> tests/compile-fail/total-ord-for-postgres_ord.rs:4:71 | 4 | #[derive(Serialize, Deserialize, PartialEq, PartialOrd, PostgresType, PostgresOrd)] | ^^^^^^^^^^^ the trait `std::cmp::Ord` is not implemented for `BrokenType` diff --git a/pgrx-tests/tests/todo/array-serialize-json.rs b/pgrx-tests/tests/todo/array-serialize-json.rs new file mode 100644 index 000000000..1ab1fe220 --- /dev/null +++ b/pgrx-tests/tests/todo/array-serialize-json.rs @@ -0,0 +1,32 @@ +use pgrx::prelude::*; +use pgrx::datum::Json; +use serde_json::json; + +fn main() {} + +// TODO: fix this test by fixing serde impls for `Array<'a, &'a str> -> Json` +#[pg_extern] +fn serde_serialize_array<'dat>(values: Array<'dat, &'dat str>) -> Json { + Json(json! { { "values": values } }) +} + +#[pgrx::pg_schema] +mod tests { + #[allow(unused_imports)] + use crate as pgrx_tests; + + use pgrx::prelude::*; + use pgrx::Json; + use serde_json::json; + + // TODO: fix this test by redesigning SPI. + #[pg_test] + fn test_serde_serialize_array() -> Result<(), pgrx::spi::Error> { + let json = Spi::get_one::( + "SELECT serde_serialize_array(ARRAY['one', null, 'two', 'three'])", + )? + .expect("returned json was null"); + assert_eq!(json.0, json! {{"values": ["one", null, "two", "three"]}}); + Ok(()) + } +} diff --git a/pgrx-tests/tests/todo/array-serialize-json.stderr b/pgrx-tests/tests/todo/array-serialize-json.stderr new file mode 100644 index 000000000..48ffcda8f --- /dev/null +++ b/pgrx-tests/tests/todo/array-serialize-json.stderr @@ -0,0 +1,22 @@ +error[E0521]: borrowed data escapes outside of function + --> tests/todo/array-serialize-json.rs:10:10 + | +9 | fn serde_serialize_array<'dat>(values: Array<'dat, &'dat str>) -> Json { + | ---- ------ `values` is a reference that is only valid in the function body + | | + | lifetime `'dat` defined here +10 | Json(json! { { "values": values } }) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | `values` escapes the function body here + | argument requires that `'dat` must outlive `'static` + | + = note: requirement occurs because of the type `pgrx::Array<'_, &str>`, which makes the generic argument `&str` invariant + = note: the struct `pgrx::Array<'mcx, T>` is invariant over the parameter `T` + = help: see for more information about variance +note: due to current limitations in the borrow checker, this implies a `'static` lifetime + --> $CARGO/serde_json-1.0.108/src/value/mod.rs + | + | T: Serialize, + | ^^^^^^^^^ + = note: this error originates in the macro `json_internal` which comes from the expansion of the macro `json` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/pgrx-tests/tests/todo/busted-exotic-signature.rs b/pgrx-tests/tests/todo/busted-exotic-signature.rs new file mode 100644 index 000000000..94b8debb0 --- /dev/null +++ b/pgrx-tests/tests/todo/busted-exotic-signature.rs @@ -0,0 +1,38 @@ +use pgrx::prelude::*; + +fn main() {} + +// Just a compile test... +// We don't run these, but we ensure we can build SQL for them +mod sql_generator_tests { + use super::*; + + #[pg_extern] + fn exotic_signature( + _cats: pgrx::default!( + Option>>, + "ARRAY[ROW('Sally', 0)]::Cat[]" + ), + _a_single_fish: pgrx::default!( + Option<::pgrx::composite_type!('static, "Fish")>, + "ROW('Bob', 0)::Fish" + ), + _dogs: pgrx::default!( + Option<::pgrx::VariadicArray<::pgrx::composite_type!('static, "Dog")>>, + "ARRAY[ROW('Nami', 0), ROW('Brandy', 0)]::Dog[]" + ), + ) -> TableIterator< + 'static, + ( + name!(dog, Option<::pgrx::composite_type!('static, "Dog")>), + name!(cat, Option<::pgrx::composite_type!('static, "Cat")>), + name!(fish, Option<::pgrx::composite_type!('static, "Fish")>), + name!( + related_edges, + Option> + ), + ), + > { + TableIterator::new(Vec::new().into_iter()) + } +} diff --git a/pgrx-tests/tests/todo/busted-exotic-signature.stderr b/pgrx-tests/tests/todo/busted-exotic-signature.stderr new file mode 100644 index 000000000..c75c484f0 --- /dev/null +++ b/pgrx-tests/tests/todo/busted-exotic-signature.stderr @@ -0,0 +1,15 @@ +error[E0277]: the trait bound `Vec>>: FromDatum` is not satisfied + --> tests/todo/busted-exotic-signature.rs:10:5 + | +10 | #[pg_extern] + | ^^^^^^^^^^^^ the trait `FromDatum` is not implemented for `Vec>>` + | + = help: the following other types implement trait `FromDatum`: + Vec + Vec> + Vec +note: required by a bound in `pg_getarg` + --> $WORKSPACE/pgrx/src/fcinfo.rs + | + | pub unsafe fn pg_getarg(fcinfo: pg_sys::FunctionCallInfo, num: usize) -> Option { + | ^^^^^^^^^ required by this bound in `pg_getarg` diff --git a/pgrx-tests/tests/todo/composite-types-broken-on-spi.rs b/pgrx-tests/tests/todo/composite-types-broken-on-spi.rs new file mode 100644 index 000000000..450bc267d --- /dev/null +++ b/pgrx-tests/tests/todo/composite-types-broken-on-spi.rs @@ -0,0 +1,252 @@ +use pgrx::prelude::*; + +fn main() {} + +const DOG_COMPOSITE_TYPE: &str = "Dog"; + +mod vec { + // ! TODO: This probably requires the FunctionArguments<'curr, 'fn> type + use super::*; + + #[pg_extern] + fn sum_scritches_for_names( + dogs: Option>, + ) -> i32 { + // Gets resolved to: + let dogs: Option>> = dogs; + + let dogs = dogs.unwrap(); + let mut sum_scritches = 0; + for dog in dogs { + let scritches: i32 = + dog.get_by_name("scritches").ok().unwrap_or_default().unwrap_or_default(); + sum_scritches += scritches; + } + sum_scritches + } + + #[pg_extern] + fn sum_scritches_for_names_default( + dogs: pgrx::default!(Vec, "ARRAY[ROW('Nami', 0)]::Dog[]"), + ) -> i32 { + // Gets resolved to: + let dogs: Vec> = dogs; + + let mut sum_scritches = 0; + for dog in dogs { + let scritches: i32 = + dog.get_by_name("scritches").ok().unwrap_or_default().unwrap_or_default(); + sum_scritches += scritches; + } + sum_scritches + } + + #[pg_extern] + fn sum_scritches_for_names_strict(dogs: Vec) -> i32 { + // Gets resolved to: + let dogs: Vec> = dogs; + + let mut sum_scritches = 0; + for dog in dogs { + let scritches: i32 = + dog.get_by_name("scritches").ok().unwrap_or_default().unwrap_or_default(); + sum_scritches += scritches; + } + sum_scritches + } + + #[pg_extern] + fn sum_scritches_for_names_strict_optional_items( + dogs: Vec>, + ) -> i32 { + // Gets resolved to: + let dogs: Vec>> = dogs; + + let mut sum_scritches = 0; + for dog in dogs { + let dog = dog.unwrap(); + let scritches: i32 = + dog.get_by_name("scritches").ok().unwrap_or_default().unwrap_or_default(); + sum_scritches += scritches; + } + sum_scritches + } + + #[pg_extern] + fn sum_scritches_for_names_default_optional_items( + dogs: pgrx::default!( + Vec>, + "ARRAY[ROW('Nami', 0)]::Dog[]" + ), + ) -> i32 { + // Gets resolved to: + let dogs: Vec>> = dogs; + + let mut sum_scritches = 0; + for dog in dogs { + let dog = dog.unwrap(); + let scritches: i32 = + dog.get_by_name("scritches").ok().unwrap_or_default().unwrap_or_default(); + sum_scritches += scritches; + } + sum_scritches + } + + #[pg_extern] + fn sum_scritches_for_names_optional_items( + dogs: Option>>, + ) -> i32 { + // Gets resolved to: + let dogs: Option>>> = dogs; + + let dogs = dogs.unwrap(); + let mut sum_scritches = 0; + for dog in dogs { + let dog = dog.unwrap(); + let scritches: i32 = dog.get_by_name("scritches").unwrap().unwrap(); + sum_scritches += scritches; + } + sum_scritches + } +} + +mod array { + use super::*; + + #[pg_extern] + fn sum_scritches_for_names_array( + dogs: Option>, + ) -> i32 { + // Gets resolved to: + let dogs: Option>> = dogs; + + let dogs = dogs.unwrap(); + let mut sum_scritches = 0; + for dog in dogs { + let dog = dog.unwrap(); + let scritches: i32 = + dog.get_by_name("scritches").ok().unwrap_or_default().unwrap_or_default(); + sum_scritches += scritches; + } + sum_scritches + } + + #[pg_extern] + fn sum_scritches_for_names_array_default( + dogs: pgrx::default!( + pgrx::Array, + "ARRAY[ROW('Nami', 0)]::Dog[]" + ), + ) -> i32 { + // Gets resolved to: + let dogs: pgrx::Array> = dogs; + + let mut sum_scritches = 0; + for dog in dogs { + let dog = dog.unwrap(); + let scritches: i32 = + dog.get_by_name("scritches").ok().unwrap_or_default().unwrap_or_default(); + sum_scritches += scritches; + } + sum_scritches + } + + #[pg_extern] + fn sum_scritches_for_names_array_strict( + dogs: pgrx::Array, + ) -> i32 { + // Gets resolved to: + let dogs: pgrx::Array> = dogs; + + let mut sum_scritches = 0; + for dog in dogs { + let dog = dog.unwrap(); + let scritches: i32 = + dog.get_by_name("scritches").ok().unwrap_or_default().unwrap_or_default(); + sum_scritches += scritches; + } + sum_scritches + } +} + +#[pgrx::pg_schema] +mod tests { + use pgrx::prelude::*; + + fn test_sum_scritches_for_names() { + let retval = Spi::get_one::( + " + SELECT sum_scritches_for_names(ARRAY[ROW('Nami', 1), ROW('Brandy', 42)]::Dog[]) + ", + ); + assert_eq!(retval, Ok(Some(43))); + } + + fn test_sum_scritches_for_names_default() { + let retval = Spi::get_one::( + " + SELECT sum_scritches_for_names_default() + ", + ); + assert_eq!(retval, Ok(Some(0))); + } + + fn test_sum_scritches_for_names_strict() { + let retval = Spi::get_one::( + " + SELECT sum_scritches_for_names_strict(ARRAY[ROW('Nami', 1), ROW('Brandy', 42)]::Dog[]) + ", + ); + assert_eq!(retval, Ok(Some(43))); + } + + fn test_sum_scritches_for_names_strict_optional_items() { + let retval = Spi::get_one::( + " + SELECT sum_scritches_for_names_strict(ARRAY[ROW('Nami', 1), ROW('Brandy', 42)]::Dog[]) + ", + ); + assert_eq!(retval, Ok(Some(43))); + } + + fn test_sum_scritches_for_names_default_optional_items() { + let retval = Spi::get_one::( + " + SELECT sum_scritches_for_names_default_optional_items() + ", + ); + assert_eq!(retval, Ok(Some(0))); + } + + fn test_sum_scritches_for_names_optional_items() { + let retval = Spi::get_one::(" + SELECT sum_scritches_for_names_optional_items(ARRAY[ROW('Nami', 1), ROW('Brandy', 42)]::Dog[]) + "); + assert_eq!(retval, Ok(Some(43))); + } + + fn test_sum_scritches_for_names_array() { + let retval = Spi::get_one::( + " + SELECT sum_scritches_for_names_array(ARRAY[ROW('Nami', 1), ROW('Brandy', 42)]::Dog[]) + ", + ); + assert_eq!(retval, Ok(Some(43))); + } + + fn test_sum_scritches_for_names_array_default() { + let retval = Spi::get_one::( + " + SELECT sum_scritches_for_names_array_default() + ", + ); + assert_eq!(retval, Ok(Some(0))); + } + + fn test_sum_scritches_for_names_array_strict() { + let retval = Spi::get_one::(" + SELECT sum_scritches_for_names_array_strict(ARRAY[ROW('Nami', 1), ROW('Brandy', 42)]::Dog[]) + "); + assert_eq!(retval, Ok(Some(43))); + } +} diff --git a/pgrx-tests/tests/todo/composite-types-broken-on-spi.stderr b/pgrx-tests/tests/todo/composite-types-broken-on-spi.stderr new file mode 100644 index 000000000..8deb1fdf4 --- /dev/null +++ b/pgrx-tests/tests/todo/composite-types-broken-on-spi.stderr @@ -0,0 +1,71 @@ +error[E0277]: the trait bound `Vec>>: FromDatum` is not satisfied + --> tests/todo/composite-types-broken-on-spi.rs:58:5 + | +58 | #[pg_extern] + | ^^^^^^^^^^^^ the trait `FromDatum` is not implemented for `Vec>>` + | + = help: the following other types implement trait `FromDatum`: + Vec + Vec> + Vec +note: required by a bound in `pg_getarg` + --> $WORKSPACE/pgrx/src/fcinfo.rs + | + | pub unsafe fn pg_getarg(fcinfo: pg_sys::FunctionCallInfo, num: usize) -> Option { + | ^^^^^^^^^ required by this bound in `pg_getarg` + +error[E0277]: the trait bound `Vec>>: FromDatum` is not satisfied + --> tests/todo/composite-types-broken-on-spi.rs:75:5 + | +75 | #[pg_extern] + | ^^^^^^^^^^^^ the trait `FromDatum` is not implemented for `Vec>>` + | + = help: the following other types implement trait `FromDatum`: + Vec + Vec> + Vec +note: required by a bound in `pg_getarg` + --> $WORKSPACE/pgrx/src/fcinfo.rs + | + | pub unsafe fn pg_getarg(fcinfo: pg_sys::FunctionCallInfo, num: usize) -> Option { + | ^^^^^^^^^ required by this bound in `pg_getarg` + +error[E0277]: the trait bound `Vec>>: FromDatum` is not satisfied + --> tests/todo/composite-types-broken-on-spi.rs:95:5 + | +95 | #[pg_extern] + | ^^^^^^^^^^^^ the trait `FromDatum` is not implemented for `Vec>>` + | + = help: the following other types implement trait `FromDatum`: + Vec + Vec> + Vec +note: required by a bound in `pg_getarg` + --> $WORKSPACE/pgrx/src/fcinfo.rs + | + | pub unsafe fn pg_getarg(fcinfo: pg_sys::FunctionCallInfo, num: usize) -> Option { + | ^^^^^^^^^ required by this bound in `pg_getarg` + +error[E0277]: the trait bound `Array<'_, pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>: IntoIterator` is not satisfied + --> tests/todo/composite-types-broken-on-spi.rs:125:20 + | +125 | for dog in dogs { + | ^^^^ the trait `IntoIterator` is not implemented for `Array<'_, pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>` + | + = help: the trait `IntoIterator` is implemented for `Array<'_, pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>` + +error[E0277]: the trait bound `Array<'_, pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>: IntoIterator` is not satisfied + --> tests/todo/composite-types-broken-on-spi.rs:145:20 + | +145 | for dog in dogs { + | ^^^^ the trait `IntoIterator` is not implemented for `Array<'_, pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>` + | + = help: the trait `IntoIterator` is implemented for `Array<'_, pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>` + +error[E0277]: the trait bound `Array<'_, pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>: IntoIterator` is not satisfied + --> tests/todo/composite-types-broken-on-spi.rs:162:20 + | +162 | for dog in dogs { + | ^^^^ the trait `IntoIterator` is not implemented for `Array<'_, pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>` + | + = help: the trait `IntoIterator` is implemented for `Array<'_, pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>` diff --git a/pgrx-tests/tests/todo/for-dog-in-dogs.rs b/pgrx-tests/tests/todo/for-dog-in-dogs.rs new file mode 100644 index 000000000..4092de64a --- /dev/null +++ b/pgrx-tests/tests/todo/for-dog-in-dogs.rs @@ -0,0 +1,109 @@ +use pgrx::prelude::*; + +fn main() {} + +const DOG_COMPOSITE_TYPE: &str = "Dog"; + +#[pg_extern] +fn gets_name_field_variadic( + dogs: VariadicArray, +) -> Vec { + // Gets resolved to: + let dogs: pgrx::VariadicArray> = dogs; + + let mut names = Vec::with_capacity(dogs.len()); + for dog in dogs { + let dog = dog.unwrap(); + let name = dog.get_by_name("name").unwrap().unwrap(); + names.push(name); + } + names +} + +/// TODO: add test which actually calls this maybe??? +#[pg_extern] +fn gets_name_field_default_variadic( + dogs: default!(VariadicArray, "ARRAY[ROW('Nami', 0)]::Dog[]"), +) -> Vec { + // Gets resolved to: + let dogs: pgrx::VariadicArray> = dogs; + + let mut names = Vec::with_capacity(dogs.len()); + for dog in dogs { + let dog = dog.unwrap(); + let name = dog.get_by_name("name").unwrap().unwrap(); + names.push(name); + } + names +} + +#[pg_extern] +fn gets_name_field_strict_variadic( + dogs: pgrx::VariadicArray, +) -> Vec { + // Gets resolved to: + let dogs: pgrx::VariadicArray> = dogs; + + let mut names = Vec::with_capacity(dogs.len()); + for dog in dogs { + let dog = dog.unwrap(); + let name = dog.get_by_name("name").unwrap().unwrap(); + names.push(name); + } + names +} + +#[pg_extern] +fn sum_scritches_for_names_array( + dogs: Option>, +) -> i32 { + // Gets resolved to: + let dogs: Option>> = dogs; + + let dogs = dogs.unwrap(); + let mut sum_scritches = 0; + for dog in dogs { + let dog = dog.unwrap(); + let scritches: i32 = + dog.get_by_name("scritches").ok().unwrap_or_default().unwrap_or_default(); + sum_scritches += scritches; + } + sum_scritches +} + +#[pg_extern] +fn sum_scritches_for_names_array_default( + dogs: pgrx::default!( + pgrx::Array, + "ARRAY[ROW('Nami', 0)]::Dog[]" + ), +) -> i32 { + // Gets resolved to: + let dogs: pgrx::Array> = dogs; + + let mut sum_scritches = 0; + for dog in dogs { + let dog = dog.unwrap(); + let scritches: i32 = + dog.get_by_name("scritches").ok().unwrap_or_default().unwrap_or_default(); + sum_scritches += scritches; + } + sum_scritches +} + +#[pg_extern] +fn sum_scritches_for_names_array_strict( + dogs: pgrx::Array, +) -> i32 { + // Gets resolved to: + let dogs: pgrx::Array> = dogs; + + let mut sum_scritches = 0; + for dog in dogs { + let dog = dog.unwrap(); + let scritches: i32 = + dog.get_by_name("scritches").ok().unwrap_or_default().unwrap_or_default(); + sum_scritches += scritches; + } + sum_scritches +} diff --git a/pgrx-tests/tests/todo/for-dog-in-dogs.stderr b/pgrx-tests/tests/todo/for-dog-in-dogs.stderr new file mode 100644 index 000000000..cb2c32568 --- /dev/null +++ b/pgrx-tests/tests/todo/for-dog-in-dogs.stderr @@ -0,0 +1,47 @@ +error[E0277]: the trait bound `pgrx::VariadicArray<'_, pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>: IntoIterator` is not satisfied + --> tests/todo/for-dog-in-dogs.rs:15:16 + | +15 | for dog in dogs { + | ^^^^ the trait `IntoIterator` is not implemented for `pgrx::VariadicArray<'_, pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>` + | + = help: the trait `IntoIterator` is implemented for `pgrx::VariadicArray<'_, pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>` + +error[E0277]: the trait bound `pgrx::VariadicArray<'_, pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>: IntoIterator` is not satisfied + --> tests/todo/for-dog-in-dogs.rs:32:16 + | +32 | for dog in dogs { + | ^^^^ the trait `IntoIterator` is not implemented for `pgrx::VariadicArray<'_, pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>` + | + = help: the trait `IntoIterator` is implemented for `pgrx::VariadicArray<'_, pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>` + +error[E0277]: the trait bound `pgrx::VariadicArray<'_, pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>: IntoIterator` is not satisfied + --> tests/todo/for-dog-in-dogs.rs:48:16 + | +48 | for dog in dogs { + | ^^^^ the trait `IntoIterator` is not implemented for `pgrx::VariadicArray<'_, pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>` + | + = help: the trait `IntoIterator` is implemented for `pgrx::VariadicArray<'_, pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>` + +error[E0277]: the trait bound `Array<'_, pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>: IntoIterator` is not satisfied + --> tests/todo/for-dog-in-dogs.rs:65:16 + | +65 | for dog in dogs { + | ^^^^ the trait `IntoIterator` is not implemented for `Array<'_, pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>` + | + = help: the trait `IntoIterator` is implemented for `Array<'_, pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>` + +error[E0277]: the trait bound `Array<'_, pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>: IntoIterator` is not satisfied + --> tests/todo/for-dog-in-dogs.rs:85:16 + | +85 | for dog in dogs { + | ^^^^ the trait `IntoIterator` is not implemented for `Array<'_, pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>` + | + = help: the trait `IntoIterator` is implemented for `Array<'_, pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>` + +error[E0277]: the trait bound `Array<'_, pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>: IntoIterator` is not satisfied + --> tests/todo/for-dog-in-dogs.rs:102:16 + | +102 | for dog in dogs { + | ^^^^ the trait `IntoIterator` is not implemented for `Array<'_, pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>` + | + = help: the trait `IntoIterator` is implemented for `Array<'_, pgrx::prelude::PgHeapTuple<'_, pgrx::AllocatedByRust>>` diff --git a/pgrx-tests/tests/todo/random-vec-strs-arent-okay.rs b/pgrx-tests/tests/todo/random-vec-strs-arent-okay.rs new file mode 100644 index 000000000..5f5029498 --- /dev/null +++ b/pgrx-tests/tests/todo/random-vec-strs-arent-okay.rs @@ -0,0 +1,28 @@ +use pgrx::prelude::*; + +fn main() {} + +#[pg_extern] +fn exec<'a>( + command: &'a str, + args: default!(Vec>, "ARRAY[]::text[]"), +) -> TableIterator<'static, (name!(status, Option), name!(stdout, String))> { + let mut command = &mut std::process::Command::new(command); + + for arg in args { + if let Some(arg) = arg { + command = command.arg(arg); + } + } + + let output = command.output().expect("command failed"); + + if !output.stderr.is_empty() { + panic!("{}", String::from_utf8(output.stderr).expect("stderr is not valid utf8")) + } + + TableIterator::once(( + output.status.code(), + String::from_utf8(output.stdout).expect("stdout is not valid utf8"), + )) +} diff --git a/pgrx-tests/tests/todo/random-vec-strs-arent-okay.stderr b/pgrx-tests/tests/todo/random-vec-strs-arent-okay.stderr new file mode 100644 index 000000000..4328b1dc4 --- /dev/null +++ b/pgrx-tests/tests/todo/random-vec-strs-arent-okay.stderr @@ -0,0 +1,15 @@ +error[E0277]: the trait bound `Vec>: FromDatum` is not satisfied + --> tests/todo/random-vec-strs-arent-okay.rs:5:1 + | +5 | #[pg_extern] + | ^^^^^^^^^^^^ the trait `FromDatum` is not implemented for `Vec>` + | + = help: the following other types implement trait `FromDatum`: + Vec + Vec> + Vec +note: required by a bound in `pg_getarg` + --> $WORKSPACE/pgrx/src/fcinfo.rs + | + | pub unsafe fn pg_getarg(fcinfo: pg_sys::FunctionCallInfo, num: usize) -> Option { + | ^^^^^^^^^ required by this bound in `pg_getarg` diff --git a/pgrx-tests/tests/todo/roundtrip-tests.rs b/pgrx-tests/tests/todo/roundtrip-tests.rs new file mode 100644 index 000000000..0379ba5fd --- /dev/null +++ b/pgrx-tests/tests/todo/roundtrip-tests.rs @@ -0,0 +1,86 @@ +fn main() {} + +mod tests { + use std::error::Error; + use std::ffi::CStr; + use pgrx::prelude::*; + + #[allow(unused_imports)] + use crate as pgrx_tests; + + macro_rules! roundtrip { + ($fname:ident, $tname:ident, &$lt:lifetime $rtype:ty, $expected:expr) => { + #[pg_extern] + fn $fname<$lt>(i: &$lt $rtype) -> &$lt $rtype { + i + } + + roundtrip_test!($fname, $tname, &'_ $rtype, $expected); + }; + ($fname:ident, $tname:ident, $rtype:ty, $expected:expr) => { + #[pg_extern] + fn $fname(i: $rtype) -> $rtype { + i + } + + roundtrip_test!($fname, $tname, $rtype, $expected); + }; + } + + macro_rules! roundtrip_test { + ($fname:ident, $tname:ident, $rtype:ty, $expected:expr) => { + #[pg_test] + fn $tname() -> Result<(), Box> { + let value: $rtype = $expected; + let expected: $rtype = Clone::clone(&value); + let result: $rtype = Spi::get_one_with_args( + &format!("SELECT {}($1)", stringify!(tests.$fname)), + vec![(PgOid::from(<$rtype>::type_oid()), value.into_datum())], + )? + .unwrap(); + + assert_eq!(result, expected); + Ok(()) + } + }; + } + + // TODO: Need to fix these array-of-bytea, array-of-text, and array-of-cstr tests, + // because `impl FromDatum for >` is broken enough pg_getarg does not work. + // This likely requires changing the glue code that fetches arguments. + + roundtrip!( + rt_array_bytea, + test_rt_array_bytea, + Vec>, + vec![ + None, + Some([b'a', b'b', b'c'].as_slice()), + Some([b'd', b'e', b'f'].as_slice()), + None, + Some([b'g', b'h', b'i'].as_slice()), + None + ] + ); + + roundtrip!( + rt_array_refstr, + test_rt_array_refstr, + Vec>, + vec![None, Some("foo"), Some("bar"), None, Some("baz"), None] + ); + + roundtrip!( + rt_array_cstr, + test_rt_array_cstr, + Vec>, + vec![ + None, + Some(unsafe { CStr::from_bytes_with_nul_unchecked(b"&one\0") }), + Some(unsafe { CStr::from_bytes_with_nul_unchecked(b"&two\0") }), + None, + Some(unsafe { CStr::from_bytes_with_nul_unchecked(b"&three\0") }), + None, + ] + ); +} diff --git a/pgrx-tests/tests/todo/roundtrip-tests.stderr b/pgrx-tests/tests/todo/roundtrip-tests.stderr new file mode 100644 index 000000000..bd82a5ccf --- /dev/null +++ b/pgrx-tests/tests/todo/roundtrip-tests.stderr @@ -0,0 +1,161 @@ +error[E0261]: use of undeclared lifetime name `'a` + --> tests/todo/roundtrip-tests.rs:69:21 + | +22 | fn $fname(i: $rtype) -> $rtype { + | - - help: consider introducing lifetime `'a` here: `<'a>` + | | + | lifetime `'a` is missing in item created through this procedural macro +... +69 | Vec>, + | ^^ undeclared lifetime + +error[E0261]: use of undeclared lifetime name `'a` + --> tests/todo/roundtrip-tests.rs:69:21 + | +21 | #[pg_extern] + | - lifetime `'a` is missing in item created through this procedural macro +... +69 | Vec>, + | ^^ undeclared lifetime + +error[E0261]: use of undeclared lifetime name `'a` + --> tests/todo/roundtrip-tests.rs:69:21 + | +67 | rt_array_refstr, + | - help: consider introducing lifetime `'a` here: `<'a>` +68 | test_rt_array_refstr, +69 | Vec>, + | ^^ undeclared lifetime + +error[E0261]: use of undeclared lifetime name `'a` + --> tests/todo/roundtrip-tests.rs:69:21 + | +68 | test_rt_array_refstr, + | - help: consider introducing lifetime `'a` here: `<'a>` +69 | Vec>, + | ^^ undeclared lifetime + +error[E0277]: the trait bound `Vec>: FromDatum` is not satisfied + --> tests/todo/roundtrip-tests.rs:55:9 + | +55 | Vec>, + | ^^^^^^^^^^^^^^^^^^ the trait `FromDatum` is not implemented for `Vec>` + | + = help: the following other types implement trait `FromDatum`: + Vec + Vec> + Vec +note: required by a bound in `pg_getarg` + --> $WORKSPACE/pgrx/src/fcinfo.rs + | + | pub unsafe fn pg_getarg(fcinfo: pg_sys::FunctionCallInfo, num: usize) -> Option { + | ^^^^^^^^^ required by this bound in `pg_getarg` + +error[E0277]: the trait bound `Vec>: FromDatum` is not satisfied + --> tests/todo/roundtrip-tests.rs:36:38 + | +36 | let result: $rtype = Spi::get_one_with_args( + | ^^^^^^^^^^^^^^^^^^^^^^ the trait `FromDatum` is not implemented for `Vec>` +... +52 | / roundtrip!( +53 | | rt_array_bytea, +54 | | test_rt_array_bytea, +55 | | Vec>, +... | +63 | | ] +64 | | ); + | |_____- in this macro invocation + | + = help: the following other types implement trait `FromDatum`: + Vec + Vec> + Vec +note: required by a bound in `pgrx::Spi::get_one_with_args` + --> $WORKSPACE/pgrx/src/spi.rs + | + | pub fn get_one_with_args( + | ^^^^^^^^^ required by this bound in `Spi::get_one_with_args` + = note: this error originates in the macro `roundtrip_test` which comes from the expansion of the macro `roundtrip` (in Nightly builds, run with -Z macro-backtrace for more info) + +error[E0277]: the trait bound `Vec>: FromDatum` is not satisfied + --> tests/todo/roundtrip-tests.rs:69:9 + | +69 | Vec>, + | ^^^^^^^^^^^^^^^^^^^^ the trait `FromDatum` is not implemented for `Vec>` + | + = help: the following other types implement trait `FromDatum`: + Vec + Vec> + Vec +note: required by a bound in `pg_getarg` + --> $WORKSPACE/pgrx/src/fcinfo.rs + | + | pub unsafe fn pg_getarg(fcinfo: pg_sys::FunctionCallInfo, num: usize) -> Option { + | ^^^^^^^^^ required by this bound in `pg_getarg` + +error[E0277]: the trait bound `Vec>: FromDatum` is not satisfied + --> tests/todo/roundtrip-tests.rs:36:38 + | +36 | let result: $rtype = Spi::get_one_with_args( + | ^^^^^^^^^^^^^^^^^^^^^^ the trait `FromDatum` is not implemented for `Vec>` +... +66 | / roundtrip!( +67 | | rt_array_refstr, +68 | | test_rt_array_refstr, +69 | | Vec>, +70 | | vec![None, Some("foo"), Some("bar"), None, Some("baz"), None] +71 | | ); + | |_____- in this macro invocation + | + = help: the following other types implement trait `FromDatum`: + Vec + Vec> + Vec +note: required by a bound in `pgrx::Spi::get_one_with_args` + --> $WORKSPACE/pgrx/src/spi.rs + | + | pub fn get_one_with_args( + | ^^^^^^^^^ required by this bound in `Spi::get_one_with_args` + = note: this error originates in the macro `roundtrip_test` which comes from the expansion of the macro `roundtrip` (in Nightly builds, run with -Z macro-backtrace for more info) + +error[E0277]: the trait bound `Vec>: FromDatum` is not satisfied + --> tests/todo/roundtrip-tests.rs:76:9 + | +76 | Vec>, + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `FromDatum` is not implemented for `Vec>` + | + = help: the following other types implement trait `FromDatum`: + Vec + Vec> + Vec +note: required by a bound in `pg_getarg` + --> $WORKSPACE/pgrx/src/fcinfo.rs + | + | pub unsafe fn pg_getarg(fcinfo: pg_sys::FunctionCallInfo, num: usize) -> Option { + | ^^^^^^^^^ required by this bound in `pg_getarg` + +error[E0277]: the trait bound `Vec>: FromDatum` is not satisfied + --> tests/todo/roundtrip-tests.rs:36:38 + | +36 | let result: $rtype = Spi::get_one_with_args( + | ^^^^^^^^^^^^^^^^^^^^^^ the trait `FromDatum` is not implemented for `Vec>` +... +73 | / roundtrip!( +74 | | rt_array_cstr, +75 | | test_rt_array_cstr, +76 | | Vec>, +... | +84 | | ] +85 | | ); + | |_____- in this macro invocation + | + = help: the following other types implement trait `FromDatum`: + Vec + Vec> + Vec +note: required by a bound in `pgrx::Spi::get_one_with_args` + --> $WORKSPACE/pgrx/src/spi.rs + | + | pub fn get_one_with_args( + | ^^^^^^^^^ required by this bound in `Spi::get_one_with_args` + = note: this error originates in the macro `roundtrip_test` which comes from the expansion of the macro `roundtrip` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/pgrx-tests/tests/todo/vec-option-composite_type-nesting-problems.rs b/pgrx-tests/tests/todo/vec-option-composite_type-nesting-problems.rs new file mode 100644 index 000000000..ebe209089 --- /dev/null +++ b/pgrx-tests/tests/todo/vec-option-composite_type-nesting-problems.rs @@ -0,0 +1,102 @@ +//! TODO: This probably requires the FunctionArguments<'curr, 'fn> type +use pgrx::prelude::*; + +fn main() {} + +const DOG_COMPOSITE_TYPE: &str = "Dog"; + +#[pg_extern] +fn sum_scritches_for_names(dogs: Option>) -> i32 { + // Gets resolved to: + let dogs: Option>> = dogs; + + let dogs = dogs.unwrap(); + let mut sum_scritches = 0; + for dog in dogs { + let scritches: i32 = + dog.get_by_name("scritches").ok().unwrap_or_default().unwrap_or_default(); + sum_scritches += scritches; + } + sum_scritches +} + +#[pg_extern] +fn sum_scritches_for_names_default( + dogs: pgrx::default!(Vec, "ARRAY[ROW('Nami', 0)]::Dog[]"), +) -> i32 { + // Gets resolved to: + let dogs: Vec> = dogs; + + let mut sum_scritches = 0; + for dog in dogs { + let scritches: i32 = + dog.get_by_name("scritches").ok().unwrap_or_default().unwrap_or_default(); + sum_scritches += scritches; + } + sum_scritches +} + +#[pg_extern] +fn sum_scritches_for_names_strict(dogs: Vec) -> i32 { + // Gets resolved to: + let dogs: Vec> = dogs; + + let mut sum_scritches = 0; + for dog in dogs { + let scritches: i32 = + dog.get_by_name("scritches").ok().unwrap_or_default().unwrap_or_default(); + sum_scritches += scritches; + } + sum_scritches +} + +#[pg_extern] +fn sum_scritches_for_names_strict_optional_items( + dogs: Vec>, +) -> i32 { + // Gets resolved to: + let dogs: Vec>> = dogs; + + let mut sum_scritches = 0; + for dog in dogs { + let dog = dog.unwrap(); + let scritches: i32 = + dog.get_by_name("scritches").ok().unwrap_or_default().unwrap_or_default(); + sum_scritches += scritches; + } + sum_scritches +} + +#[pg_extern] +fn sum_scritches_for_names_default_optional_items( + dogs: pgrx::default!(Vec>, "ARRAY[ROW('Nami', 0)]::Dog[]"), +) -> i32 { + // Gets resolved to: + let dogs: Vec>> = dogs; + + let mut sum_scritches = 0; + for dog in dogs { + let dog = dog.unwrap(); + let scritches: i32 = + dog.get_by_name("scritches").ok().unwrap_or_default().unwrap_or_default(); + sum_scritches += scritches; + } + sum_scritches +} + +#[pg_extern] +fn sum_scritches_for_names_optional_items( + dogs: Option>>, +) -> i32 { + // Gets resolved to: + let dogs: Option>>> = dogs; + + let dogs = dogs.unwrap(); + let mut sum_scritches = 0; + for dog in dogs { + let dog = dog.unwrap(); + let scritches: i32 = dog.get_by_name("scritches").unwrap().unwrap(); + sum_scritches += scritches; + } + sum_scritches +} diff --git a/pgrx-tests/tests/todo/vec-option-composite_type-nesting-problems.stderr b/pgrx-tests/tests/todo/vec-option-composite_type-nesting-problems.stderr new file mode 100644 index 000000000..7636f4530 --- /dev/null +++ b/pgrx-tests/tests/todo/vec-option-composite_type-nesting-problems.stderr @@ -0,0 +1,47 @@ +error[E0277]: the trait bound `Vec>>: FromDatum` is not satisfied + --> tests/todo/vec-option-composite_type-nesting-problems.rs:53:1 + | +53 | #[pg_extern] + | ^^^^^^^^^^^^ the trait `FromDatum` is not implemented for `Vec>>` + | + = help: the following other types implement trait `FromDatum`: + Vec + Vec> + Vec +note: required by a bound in `pg_getarg` + --> $WORKSPACE/pgrx/src/fcinfo.rs + | + | pub unsafe fn pg_getarg(fcinfo: pg_sys::FunctionCallInfo, num: usize) -> Option { + | ^^^^^^^^^ required by this bound in `pg_getarg` + +error[E0277]: the trait bound `Vec>>: FromDatum` is not satisfied + --> tests/todo/vec-option-composite_type-nesting-problems.rs:70:1 + | +70 | #[pg_extern] + | ^^^^^^^^^^^^ the trait `FromDatum` is not implemented for `Vec>>` + | + = help: the following other types implement trait `FromDatum`: + Vec + Vec> + Vec +note: required by a bound in `pg_getarg` + --> $WORKSPACE/pgrx/src/fcinfo.rs + | + | pub unsafe fn pg_getarg(fcinfo: pg_sys::FunctionCallInfo, num: usize) -> Option { + | ^^^^^^^^^ required by this bound in `pg_getarg` + +error[E0277]: the trait bound `Vec>>: FromDatum` is not satisfied + --> tests/todo/vec-option-composite_type-nesting-problems.rs:87:1 + | +87 | #[pg_extern] + | ^^^^^^^^^^^^ the trait `FromDatum` is not implemented for `Vec>>` + | + = help: the following other types implement trait `FromDatum`: + Vec + Vec> + Vec +note: required by a bound in `pg_getarg` + --> $WORKSPACE/pgrx/src/fcinfo.rs + | + | pub unsafe fn pg_getarg(fcinfo: pg_sys::FunctionCallInfo, num: usize) -> Option { + | ^^^^^^^^^ required by this bound in `pg_getarg` diff --git a/pgrx-tests/tests/ui.rs b/pgrx-tests/tests/ui.rs index 4e60056b3..c22812de5 100644 --- a/pgrx-tests/tests/ui.rs +++ b/pgrx-tests/tests/ui.rs @@ -2,8 +2,16 @@ use trybuild::TestCases; +/// These are tests which are intended to always fail. #[test] -fn ui() { +fn compile_fail() { let t = TestCases::new(); - t.compile_fail("tests/ui/*.rs"); + t.compile_fail("tests/compile-fail/*.rs"); +} + +/// These are tests which currently fail, but should be fixed later. +#[test] +fn todo() { + let t = TestCases::new(); + t.compile_fail("tests/todo/*.rs"); } diff --git a/pgrx/src/datum/array.rs b/pgrx/src/datum/array.rs index 958b0eaa4..053db7b96 100644 --- a/pgrx/src/datum/array.rs +++ b/pgrx/src/datum/array.rs @@ -8,19 +8,20 @@ //LICENSE //LICENSE Use of this source code is governed by the MIT license that can be found in the LICENSE file. #![allow(clippy::question_mark)] +use super::UnboxDatum; use crate::array::RawArray; use crate::layout::*; use crate::toast::Toast; use crate::{pg_sys, FromDatum, IntoDatum, PgMemoryContexts}; use bitvec::slice::BitSlice; use core::fmt::{Debug, Formatter}; -use core::iter::FusedIterator; use core::ops::DerefMut; use core::ptr::NonNull; use pgrx_sql_entity_graph::metadata::{ ArgumentError, Returns, ReturnsError, SqlMapping, SqlTranslatable, }; use serde::{Serialize, Serializer}; +use std::iter::FusedIterator; /** An array of some type (eg. `TEXT[]`, `int[]`) @@ -55,14 +56,19 @@ fn with_vec(elems: Array) { } ``` */ -pub struct Array<'a, T> { - null_slice: NullKind<'a>, +// An array is a detoasted varlena type, so we reason about the lifetime of +// the memory context that the varlena is actually detoasted into. +pub struct Array<'mcx, T> { + null_slice: NullKind<'mcx>, slide_impl: ChaChaSlideImpl, // Rust drops in FIFO order, drop this last raw: Toast, } -impl<'a, T: FromDatum + Debug> Debug for Array<'a, T> { +impl Debug for Array<'_, T> +where + for<'arr> ::As<'arr>: Debug, +{ fn fmt(&self, f: &mut Formatter<'_>) -> core::fmt::Result { f.debug_list().entries(self.iter()).finish() } @@ -95,22 +101,27 @@ impl NullKind<'_> { } } -impl Serialize for Array<'_, T> { +impl<'mcx, T: UnboxDatum> serde::Serialize for Array<'mcx, T> +where + for<'arr> ::As<'arr>: Serialize, +{ fn serialize(&self, serializer: S) -> Result<::Ok, ::Error> where S: Serializer, { - serializer.collect_seq(self.iter()) + let iter = self.iter(); + let result = serializer.collect_seq(iter); + result } } #[deny(unsafe_op_in_unsafe_fn)] -impl<'a, T: FromDatum> Array<'a, T> { +impl<'mcx, T: UnboxDatum> Array<'mcx, T> { /// # Safety /// /// This function requires that the RawArray was obtained in a properly-constructed form /// (probably from Postgres). - unsafe fn deconstruct_from(mut raw: Toast) -> Array<'a, T> { + unsafe fn deconstruct_from(mut raw: Toast) -> Array<'mcx, T> { let oid = raw.oid(); let elem_layout = Layout::lookup_oid(oid); let nelems = raw.len(); @@ -177,7 +188,7 @@ impl<'a, T: FromDatum> Array<'a, T> { #[allow(clippy::option_option)] #[inline] - pub fn get(&self, index: usize) -> Option> { + pub fn get<'arr>(&'arr self, index: usize) -> Option>> { let Some(is_null) = self.null_slice.get(index) else { return None }; if is_null { return Some(None); @@ -213,7 +224,11 @@ impl<'a, T: FromDatum> Array<'a, T> { /// # Safety /// This assumes the pointer is to a valid element of that type. #[inline] - unsafe fn bring_it_back_now(&self, ptr: *const u8, is_null: bool) -> Option { + unsafe fn bring_it_back_now<'arr>( + &'arr self, + ptr: *const u8, + is_null: bool, + ) -> Option> { match is_null { true => None, false => unsafe { self.slide_impl.bring_it_back_now(self, ptr) }, @@ -376,20 +391,25 @@ fn as_slice<'a, T: Sized>(array: &'a Array<'_, T>) -> Result<&'a [T], ArraySlice } mod casper { + use super::UnboxDatum; use crate::layout::Align; - use crate::{pg_sys, varlena, Array, FromDatum}; + use crate::{pg_sys, varlena, Array}; // it's a pop-culture reference (https://en.wikipedia.org/wiki/Cha_Cha_Slide) not some fancy crypto thing you nerd /// Describes how to instantiate a value `T` from an [`Array`] and its backing byte array pointer. /// It also knows how to determine the size of an [`Array`] element value. - pub(super) trait ChaChaSlide { + pub(super) trait ChaChaSlide { /// Instantiate a `T` from the head of `ptr` /// /// # Safety /// /// This function is unsafe as it cannot guarantee that `ptr` points to the proper bytes /// that represent a `T`, or even that it belongs to `array`. Both of which must be true - unsafe fn bring_it_back_now(&self, array: &Array, ptr: *const u8) -> Option; + unsafe fn bring_it_back_now<'arr, 'mcx>( + &self, + array: &'arr Array<'mcx, T>, + ptr: *const u8, + ) -> Option>; /// Determine how many bytes are used to represent `T`. This could be fixed size or /// even determined at runtime by whatever `ptr` is known to be pointing at. @@ -419,9 +439,13 @@ mod casper { /// Fixed-size byval array elements. N should be 1, 2, 4, or 8. Note that /// `T` (the rust type) may have a different size than `N`. pub(super) struct FixedSizeByVal; - impl ChaChaSlide for FixedSizeByVal { + impl ChaChaSlide for FixedSizeByVal { #[inline(always)] - unsafe fn bring_it_back_now(&self, array: &Array, ptr: *const u8) -> Option { + unsafe fn bring_it_back_now<'arr, 'mcx>( + &self, + _array: &'arr Array<'mcx, T>, + ptr: *const u8, + ) -> Option> { // This branch is optimized away (because `N` is constant). let datum = match N { // for match with `Datum`, read through that directly to @@ -432,7 +456,7 @@ mod casper { 8 => pg_sys::Datum::from(byval_read::(ptr)), _ => unreachable!("`N` must be 1, 2, 4, or 8 (got {N})"), }; - T::from_polymorphic_datum(datum, false, array.raw.oid()) + Some(T::unbox(core::mem::transmute(datum))) } #[inline(always)] @@ -445,11 +469,16 @@ mod casper { pub(super) struct PassByVarlena { pub(super) align: Align, } - impl ChaChaSlide for PassByVarlena { + impl ChaChaSlide for PassByVarlena { #[inline] - unsafe fn bring_it_back_now(&self, array: &Array, ptr: *const u8) -> Option { + unsafe fn bring_it_back_now<'arr, 'mcx>( + &self, + // May need this array param for MemCx reasons? + _array: &'arr Array<'mcx, T>, + ptr: *const u8, + ) -> Option> { let datum = pg_sys::Datum::from(ptr); - unsafe { T::from_polymorphic_datum(datum, false, array.raw.oid()) } + Some(T::unbox(core::mem::transmute(datum))) } #[inline] @@ -465,11 +494,15 @@ mod casper { /// Array elements are standard C strings (`char *`), which are pass-by-reference pub(super) struct PassByCStr; - impl ChaChaSlide for PassByCStr { + impl ChaChaSlide for PassByCStr { #[inline] - unsafe fn bring_it_back_now(&self, array: &Array, ptr: *const u8) -> Option { + unsafe fn bring_it_back_now<'arr, 'mcx>( + &self, + _array: &'arr Array<'mcx, T>, + ptr: *const u8, + ) -> Option> { let datum = pg_sys::Datum::from(ptr); - unsafe { T::from_polymorphic_datum(datum, false, array.raw.oid()) } + Some(T::unbox(core::mem::transmute(datum))) } #[inline] @@ -486,11 +519,15 @@ mod casper { pub(super) padded_size: usize, } - impl ChaChaSlide for PassByFixed { + impl ChaChaSlide for PassByFixed { #[inline] - unsafe fn bring_it_back_now(&self, array: &Array, ptr: *const u8) -> Option { + unsafe fn bring_it_back_now<'arr, 'mcx>( + &self, + _array: &'arr Array<'mcx, T>, + ptr: *const u8, + ) -> Option> { let datum = pg_sys::Datum::from(ptr); - unsafe { T::from_polymorphic_datum(datum, false, array.raw.oid()) } + Some(T::unbox(core::mem::transmute(datum))) } #[inline] @@ -500,9 +537,12 @@ mod casper { } } -pub struct VariadicArray<'a, T>(Array<'a, T>); +pub struct VariadicArray<'mcx, T>(Array<'mcx, T>); -impl Serialize for VariadicArray<'_, T> { +impl<'mcx, T: UnboxDatum> Serialize for VariadicArray<'mcx, T> +where + for<'arr> ::As<'arr>: Serialize, +{ fn serialize(&self, serializer: S) -> Result<::Ok, ::Error> where S: Serializer, @@ -511,7 +551,7 @@ impl Serialize for VariadicArray<'_, T> { } } -impl VariadicArray<'_, T> { +impl<'mcx, T: UnboxDatum> VariadicArray<'mcx, T> { /// Return an Iterator of `Option` over the contained Datums. #[inline] pub fn iter(&self) -> ArrayIterator<'_, T> { @@ -529,7 +569,7 @@ impl VariadicArray<'_, T> { #[allow(clippy::option_option)] #[inline] - pub fn get(&self, i: usize) -> Option> { + pub fn get<'arr>(&'arr self, i: usize) -> Option::As<'arr>>> { self.0.get(i) } } @@ -551,14 +591,14 @@ impl VariadicArray<'_, T> { } } -pub struct ArrayTypedIterator<'a, T> { - array: &'a Array<'a, T>, +pub struct ArrayTypedIterator<'arr, T> { + array: &'arr Array<'arr, T>, curr: usize, ptr: *const u8, } -impl<'a, T: FromDatum> Iterator for ArrayTypedIterator<'a, T> { - type Item = T; +impl<'arr, T: UnboxDatum> Iterator for ArrayTypedIterator<'arr, T> { + type Item = T::As<'arr>; #[inline] fn next(&mut self) -> Option { @@ -582,10 +622,13 @@ impl<'a, T: FromDatum> Iterator for ArrayTypedIterator<'a, T> { } } -impl<'a, T: FromDatum> ExactSizeIterator for ArrayTypedIterator<'a, T> {} -impl<'a, T: FromDatum> FusedIterator for ArrayTypedIterator<'a, T> {} +impl<'a, T: UnboxDatum> ExactSizeIterator for ArrayTypedIterator<'a, T> {} +impl<'a, T: UnboxDatum> core::iter::FusedIterator for ArrayTypedIterator<'a, T> {} -impl<'a, T: FromDatum + Serialize> Serialize for ArrayTypedIterator<'a, T> { +impl<'arr, T: UnboxDatum + serde::Serialize> serde::Serialize for ArrayTypedIterator<'arr, T> +where + ::As<'arr>: serde::Serialize, +{ fn serialize(&self, serializer: S) -> Result<::Ok, ::Error> where S: Serializer, @@ -594,14 +637,14 @@ impl<'a, T: FromDatum + Serialize> Serialize for ArrayTypedIterator<'a, T> { } } -pub struct ArrayIterator<'a, T> { - array: &'a Array<'a, T>, +pub struct ArrayIterator<'arr, T> { + array: &'arr Array<'arr, T>, curr: usize, ptr: *const u8, } -impl<'a, T: FromDatum> Iterator for ArrayIterator<'a, T> { - type Item = Option; +impl<'arr, T: UnboxDatum> Iterator for ArrayIterator<'arr, T> { + type Item = Option>; #[inline] fn next(&mut self) -> Option { @@ -626,8 +669,8 @@ impl<'a, T: FromDatum> Iterator for ArrayIterator<'a, T> { } } -impl<'a, T: FromDatum> ExactSizeIterator for ArrayIterator<'a, T> {} -impl<'a, T: FromDatum> FusedIterator for ArrayIterator<'a, T> {} +impl<'arr, T: UnboxDatum> ExactSizeIterator for ArrayIterator<'arr, T> {} +impl<'arr, T: UnboxDatum> FusedIterator for ArrayIterator<'arr, T> {} pub struct ArrayIntoIterator<'a, T> { array: Array<'a, T>, @@ -635,9 +678,13 @@ pub struct ArrayIntoIterator<'a, T> { ptr: *const u8, } -impl<'a, T: FromDatum> IntoIterator for Array<'a, T> { - type Item = Option; - type IntoIter = ArrayIntoIterator<'a, T>; +// There's nowhere to name the lifetime contraction +impl<'mcx, T> IntoIterator for Array<'mcx, T> +where + for<'arr> T: UnboxDatum = T> + 'static, +{ + type Item = Option>; + type IntoIter = ArrayIntoIterator<'mcx, T>; #[inline] fn into_iter(self) -> Self::IntoIter { @@ -646,9 +693,12 @@ impl<'a, T: FromDatum> IntoIterator for Array<'a, T> { } } -impl<'a, T: FromDatum> IntoIterator for VariadicArray<'a, T> { - type Item = Option; - type IntoIter = ArrayIntoIterator<'a, T>; +impl<'mcx, T> IntoIterator for VariadicArray<'mcx, T> +where + for<'arr> T: UnboxDatum = T> + 'static, +{ + type Item = Option>; + type IntoIter = ArrayIntoIterator<'mcx, T>; #[inline] fn into_iter(self) -> Self::IntoIter { @@ -657,15 +707,17 @@ impl<'a, T: FromDatum> IntoIterator for VariadicArray<'a, T> { } } -impl<'a, T: FromDatum> Iterator for ArrayIntoIterator<'a, T> { - type Item = Option; +impl<'mcx, T> Iterator for ArrayIntoIterator<'mcx, T> +where + for<'arr> T: UnboxDatum = T> + 'static, +{ + type Item = Option>; #[inline] fn next(&mut self) -> Option { let Self { array, curr, ptr } = self; let Some(is_null) = array.null_slice.get(*curr) else { return None }; *curr += 1; - let element = unsafe { array.bring_it_back_now(*ptr, is_null) }; if !is_null { // SAFETY: This has to not move for nulls, as they occupy 0 data bytes, @@ -683,10 +735,16 @@ impl<'a, T: FromDatum> Iterator for ArrayIntoIterator<'a, T> { } } -impl<'a, T: FromDatum> ExactSizeIterator for ArrayIntoIterator<'a, T> {} -impl<'a, T: FromDatum> FusedIterator for ArrayIntoIterator<'a, T> {} +impl<'mcx, T> ExactSizeIterator for ArrayIntoIterator<'mcx, T> where + for<'arr> T: UnboxDatum = T> + 'static +{ +} +impl<'mcx, T: UnboxDatum> FusedIterator for ArrayIntoIterator<'mcx, T> where + for<'arr> T: UnboxDatum = T> + 'static +{ +} -impl<'a, T: FromDatum> FromDatum for VariadicArray<'a, T> { +impl<'a, T: FromDatum + UnboxDatum> FromDatum for VariadicArray<'a, T> { #[inline] unsafe fn from_polymorphic_datum( datum: pg_sys::Datum, @@ -697,7 +755,7 @@ impl<'a, T: FromDatum> FromDatum for VariadicArray<'a, T> { } } -impl<'a, T: FromDatum> FromDatum for Array<'a, T> { +impl<'a, T: UnboxDatum> FromDatum for Array<'a, T> { #[inline] unsafe fn from_polymorphic_datum( datum: pg_sys::Datum, @@ -752,7 +810,10 @@ impl IntoDatum for Array<'_, T> { } } -impl FromDatum for Vec { +impl FromDatum for Vec +where + for<'arr> T: UnboxDatum = T> + 'arr, +{ #[inline] unsafe fn from_polymorphic_datum( datum: pg_sys::Datum, @@ -781,7 +842,10 @@ impl FromDatum for Vec { } } -impl FromDatum for Vec> { +impl FromDatum for Vec> +where + for<'arr> T: UnboxDatum = T> + 'arr, +{ #[inline] unsafe fn from_polymorphic_datum( datum: pg_sys::Datum, diff --git a/pgrx/src/datum/mod.rs b/pgrx/src/datum/mod.rs index c0cecff5d..e58fb4ac0 100644 --- a/pgrx/src/datum/mod.rs +++ b/pgrx/src/datum/mod.rs @@ -32,6 +32,7 @@ mod time_stamp; mod time_stamp_with_timezone; mod time_with_timezone; mod tuples; +mod unbox; mod uuid; mod varlena; @@ -55,15 +56,117 @@ use std::any::TypeId; pub use time_stamp::*; pub use time_stamp_with_timezone::*; pub use time_with_timezone::*; +pub use unbox::*; pub use varlena::*; +use crate::memcx::MemCx; +use crate::pg_sys; use crate::PgBox; +use core::marker::PhantomData; use pgrx_sql_entity_graph::RustSqlMapping; +/// How Postgres represents datatypes +/// +/// The "no-frills" version is [`pg_sys::Datum`], which is abstractly a union of "pointer to void" +/// with other scalar types that can be packed within a pointer's bytes. In practical use, a "raw" +/// Datum can prove to have the same risks as a pointer: code may try to use it without knowing +/// whether its pointee has been deallocated. To lift a Datum into a Rust type requires making +/// implicit lifetimes into explicit bounds. +/// +/// Merely having a lifetime does not make `Datum<'src>` "safe" to use. To abstractly represent a +/// full PostgreSQL value needs at least the tuple (Datum, bool, [`pg_sys::Oid`]): a tagged union. +/// `Datum<'src>` itself is effectively a dynamically-typed union *without a type tag*. It exists +/// not to make code manipulating it safe, but to make it possible to write unsafe code correctly, +/// passing Datums to and from Postgres without having to wonder if the implied `&'src T` would +/// actually refer to deallocated data. +/// +/// # Designing safe abstractions +/// A function must only be declared safe if *all* inputs **cannot** cause [undefined behavior]. +/// Transmuting a raw `pg_sys::Datum` into [`&'a T`] grants a potentially-unbounded lifetime, +/// breaking the rule borrows must not outlive the borrowed. Avoiding such transmutations infects +/// even simple generic functions with soundness obligations. Using only `&'a pg_sys::Datum` lasts +/// only until one must pass by-value, which is the entire point of the original type as Postgres +/// defined it, but can still be preferable. +/// +/// `Datum<'src>` makes it theoretically possible to write functions with a signature like +/// ``` +/// use pgrx::datum::Datum; +/// # use core::marker::PhantomData; +/// # use pgrx::memcx::MemCx; +/// # struct InCx<'mcx, T>(T, PhantomData<&'mcx MemCx<'mcx>>); +/// fn construct_type_from_datum<'src, T>( +/// datum: Datum<'src>, +/// func: impl FnOnce(Datum<'src>) -> InCx<'src, T> +/// ) -> InCx<'src, T> { +/// func(datum) +/// } +/// ``` +/// However, it is possible for `T<'src>` to be insufficient to represent the real lifetime of the +/// abstract Postgres type's allocations. Often a Datum must be "detoasted", which may reallocate. +/// This may demand two constraints on the return type to represent both possible lifetimes, like: +/// ``` +/// use pgrx::datum::Datum; +/// use pgrx::memcx::MemCx; +/// # use core::marker::PhantomData; +/// # struct Detoasted<'mcx, T>(T, PhantomData<&'mcx MemCx<'mcx>>); +/// # struct InCx<'mcx, T>(T, PhantomData<&'mcx MemCx<'mcx>>); +/// fn detoast_type_from_datum<'old, 'new, T>( +/// datum: Datum<'old>, +/// memcx: MemCx<'new>, +/// ) -> Detoasted<'new, InCx<'old, T>> { +/// todo!() +/// } +/// ``` +/// In actual practice, these can be unified into a single lifetime: the lower bound of both. +/// This is both good and bad: types can use fewer lifetime annotations, even after detoasting. +/// However, in general, because lifetime unification can be done implicitly by the compiler, +/// it is often important to name each and every single lifetime involved in functions that +/// perform these tasks. +/// +/// [`&'a T`]: reference +/// [undefined behavior]: https://doc.rust-lang.org/reference/behavior-considered-undefined.html +pub struct Datum<'src>( + pg_sys::Datum, + /// if a Datum borrows anything, it's "from" a [`pg_sys::MemoryContext`] + /// as a memory context, like an arena, is deallocated "together". + /// FIXME: a more-correct inner type later + PhantomData<&'src MemCx<'src>>, +); + +impl<'src> Datum<'src> { + /// The Datum without its lifetime. + pub fn sans_lifetime(self) -> pg_sys::Datum { + self.0 + } +} + /// A tagging trait to indicate a user type is also meant to be used by Postgres /// Implemented automatically by `#[derive(PostgresType)]` pub trait PostgresType {} +/// Obtain a TypeId for T without `T: 'static` +#[inline] +#[doc(hidden)] +pub fn nonstatic_typeid() -> core::any::TypeId { + trait NonStaticAny { + fn type_id(&self) -> core::any::TypeId + where + Self: 'static; + } + impl NonStaticAny for core::marker::PhantomData { + #[inline] + fn type_id(&self) -> core::any::TypeId + where + Self: 'static, + { + core::any::TypeId::of::() + } + } + let it = core::marker::PhantomData::; + // There is no excuse for the crimes we have done here, but what jury would convict us? + unsafe { core::mem::transmute::<&dyn NonStaticAny, &'static dyn NonStaticAny>(&it).type_id() } +} + /// A type which can have it's [`core::any::TypeId`]s registered for Rust to SQL mapping. /// /// An example use of this trait: @@ -77,9 +180,9 @@ pub trait PostgresType {} /// /// let mut mappings = Default::default(); /// let treat_string = stringify!(Treat).to_string(); -/// as pgrx::datum::WithTypeIds>::register_with_refs(&mut mappings, treat_string.clone()); +/// as pgrx::datum::WithTypeIds>::register_with_refs(&mut mappings, treat_string.clone()); /// -/// assert!(mappings.iter().any(|x| x.id == core::any::TypeId::of::>())); +/// assert!(mappings.iter().any(|x| x.id == ::pgrx::datum::nonstatic_typeid::>())); /// ``` /// /// This trait uses the fact that inherent implementations are a higher priority than trait @@ -98,10 +201,7 @@ pub trait WithTypeIds { const VARLENA_ID: Lazy>; const OPTION_VARLENA_ID: Lazy>; - fn register_with_refs(map: &mut std::collections::HashSet, single_sql: String) - where - Self: 'static, - { + fn register_with_refs(map: &mut std::collections::HashSet, single_sql: String) { Self::register(map, single_sql.clone()); <&Self as WithTypeIds>::register(map, single_sql.clone()); <&mut Self as WithTypeIds>::register(map, single_sql); @@ -110,55 +210,37 @@ pub trait WithTypeIds { fn register_sized_with_refs( _map: &mut std::collections::HashSet, _single_sql: String, - ) where - Self: 'static, - { + ) { () } - fn register_sized(_map: &mut std::collections::HashSet, _single_sql: String) - where - Self: 'static, - { + fn register_sized(_map: &mut std::collections::HashSet, _single_sql: String) { () } fn register_varlena_with_refs( _map: &mut std::collections::HashSet, _single_sql: String, - ) where - Self: 'static, - { + ) { () } - fn register_varlena(_map: &mut std::collections::HashSet, _single_sql: String) - where - Self: 'static, - { + fn register_varlena(_map: &mut std::collections::HashSet, _single_sql: String) { () } fn register_array_with_refs( _map: &mut std::collections::HashSet, _single_sql: String, - ) where - Self: 'static, - { + ) { () } - fn register_array(_map: &mut std::collections::HashSet, _single_sql: String) - where - Self: 'static, - { + fn register_array(_map: &mut std::collections::HashSet, _single_sql: String) { () } - fn register(set: &mut std::collections::HashSet, single_sql: String) - where - Self: 'static, - { + fn register(set: &mut std::collections::HashSet, single_sql: String) { let rust = core::any::type_name::(); assert!( set.insert(RustSqlMapping { @@ -173,8 +255,8 @@ pub trait WithTypeIds { } } -impl WithTypeIds for T { - const ITEM_ID: Lazy = Lazy::new(|| TypeId::of::()); +impl WithTypeIds for T { + const ITEM_ID: Lazy = Lazy::new(|| nonstatic_typeid::()); const OPTION_ID: Lazy> = Lazy::new(|| None); const VEC_ID: Lazy> = Lazy::new(|| None); const VEC_OPTION_ID: Lazy> = Lazy::new(|| None); @@ -214,20 +296,20 @@ impl WithTypeIds for T { /// implementations. pub struct WithSizedTypeIds(pub core::marker::PhantomData); -impl WithSizedTypeIds { - pub const PG_BOX_ID: Lazy> = Lazy::new(|| Some(TypeId::of::>())); +impl WithSizedTypeIds { + pub const PG_BOX_ID: Lazy> = Lazy::new(|| Some(nonstatic_typeid::>())); pub const PG_BOX_OPTION_ID: Lazy> = - Lazy::new(|| Some(TypeId::of::>>())); + Lazy::new(|| Some(nonstatic_typeid::>>())); pub const PG_BOX_VEC_ID: Lazy> = - Lazy::new(|| Some(TypeId::of::>>())); - pub const OPTION_ID: Lazy> = Lazy::new(|| Some(TypeId::of::>())); - pub const VEC_ID: Lazy> = Lazy::new(|| Some(TypeId::of::>())); + Lazy::new(|| Some(nonstatic_typeid::>>())); + pub const OPTION_ID: Lazy> = Lazy::new(|| Some(nonstatic_typeid::>())); + pub const VEC_ID: Lazy> = Lazy::new(|| Some(nonstatic_typeid::>())); pub const VEC_OPTION_ID: Lazy> = - Lazy::new(|| Some(TypeId::of::>>())); + Lazy::new(|| Some(nonstatic_typeid::>>())); pub const OPTION_VEC_ID: Lazy> = - Lazy::new(|| Some(TypeId::of::>>())); + Lazy::new(|| Some(nonstatic_typeid::>>())); pub const OPTION_VEC_OPTION_ID: Lazy> = - Lazy::new(|| Some(TypeId::of::>>>())); + Lazy::new(|| Some(nonstatic_typeid::>>>())); pub fn register_sized_with_refs( map: &mut std::collections::HashSet, @@ -333,7 +415,7 @@ impl WithSizedTypeIds { /// treat_string.clone() /// ); /// -/// assert!(mappings.iter().any(|x| x.id == core::any::TypeId::of::>())); +/// assert!(mappings.iter().any(|x| x.id == ::pgrx::datum::nonstatic_typeid::>())); /// ``` /// /// This trait uses the fact that inherent implementations are a higher priority than trait @@ -341,13 +423,13 @@ impl WithSizedTypeIds { pub struct WithArrayTypeIds(pub core::marker::PhantomData); impl WithArrayTypeIds { - pub const ARRAY_ID: Lazy> = Lazy::new(|| Some(TypeId::of::>())); + pub const ARRAY_ID: Lazy> = Lazy::new(|| Some(nonstatic_typeid::>())); pub const OPTION_ARRAY_ID: Lazy> = - Lazy::new(|| Some(TypeId::of::>>())); + Lazy::new(|| Some(nonstatic_typeid::>>())); pub const VARIADICARRAY_ID: Lazy> = - Lazy::new(|| Some(TypeId::of::>())); + Lazy::new(|| Some(nonstatic_typeid::>())); pub const OPTION_VARIADICARRAY_ID: Lazy> = - Lazy::new(|| Some(TypeId::of::>>())); + Lazy::new(|| Some(nonstatic_typeid::>>())); pub fn register_array_with_refs( map: &mut std::collections::HashSet, @@ -418,7 +500,7 @@ impl WithArrayTypeIds { /// treat_string.clone() /// ); /// -/// assert!(mappings.iter().any(|x| x.id == core::any::TypeId::of::>>())); +/// assert!(mappings.iter().any(|x| x.id == ::pgrx::datum::nonstatic_typeid::>>())); /// ``` /// /// This trait uses the fact that inherent implementations are a higher priority than trait @@ -426,11 +508,12 @@ impl WithArrayTypeIds { pub struct WithVarlenaTypeIds(pub core::marker::PhantomData); impl WithVarlenaTypeIds { - pub const VARLENA_ID: Lazy> = Lazy::new(|| Some(TypeId::of::>())); + pub const VARLENA_ID: Lazy> = + Lazy::new(|| Some(nonstatic_typeid::>())); pub const PG_BOX_VARLENA_ID: Lazy> = - Lazy::new(|| Some(TypeId::of::>>())); + Lazy::new(|| Some(nonstatic_typeid::>>())); pub const OPTION_VARLENA_ID: Lazy> = - Lazy::new(|| Some(TypeId::of::>>())); + Lazy::new(|| Some(nonstatic_typeid::>>())); pub fn register_varlena_with_refs( map: &mut std::collections::HashSet, diff --git a/pgrx/src/datum/unbox.rs b/pgrx/src/datum/unbox.rs new file mode 100644 index 000000000..169023f8c --- /dev/null +++ b/pgrx/src/datum/unbox.rs @@ -0,0 +1,345 @@ +use super::uuid::Uuid; +use super::Datum; +use crate::prelude::*; +use crate::varlena::{text_to_rust_str_unchecked, varlena_to_byte_slice}; +use alloc::ffi::CString; +use core::ffi::CStr; + +/// Directly convert a Datum into this type +/// +/// Previously, pgrx used FromDatum exclusively, which captures conversion into T, but +/// leaves ambiguous a number of possibilities and allows large swathes of behavior to +/// be folded under a single trait. This provided certain beneficial ergonomics at first, +/// but eventually behavior was incorrectly folded under FromDatum, which provided the +/// illusion of soundness. +/// +/// UnboxDatum should only be implemented for a type that CAN be directly converted from a Datum, +/// and it doesn't say whether it can be used directly or if it should be detoasted via MemCx. +/// It's currently just a possibly-temporary shim to make pgrx work. +/// +/// # Safety +/// This trait is used to bound the lifetime of certain types: thus the associated type must be +/// this type but "infected" by the Datum's lifetime. By implementing this, you verify that you +/// are implementing this in the way that satisfies that lifetime constraint. There isn't really +/// a good way to constrain lifetimes correctly without forcing from-Datum types to go through a +/// wrapper type bound by the lifetime of the Datum. And what would you use as the bound, hmm? +pub unsafe trait UnboxDatum { + // TODO: Currently, this doesn't actually get used to identify all the cases where the Datum + // is actually a pointer type. However, it has been noted that Postgres does yield nullptr + // on occasion, even when they say something is not supposed to be nullptr. As it is common + // for Postgres to init [Datum<'_>] with palloc0, it is reasonable to assume nullptr is a risk, + // even if `is_null == false`. + // + // Wait, what are you about, Jubilee? In some cases, the chance of nullness doesn't exist! + // This is because we are materializing the datums from e.g. pointers to an Array, which + // requires you to have had a valid base pointer into an ArrayType to start! + // That's why you started using this goofy GAT scheme in the first place! + /// Self with the lifetime `'src` + /// + /// The lifetime `'src` represents the lifetime of the "root" source of this Datum, which + /// may be a memory context or a shorter-lived type inside that context. + /// + type As<'src> + where + Self: 'src; + /// Convert from `Datum<'src>` to `T::As<'src>` + /// + /// This should be the direct conversion in each case. It is very unsafe to use directly. + /// + /// # Safety + /// Due to the absence of an `is_null` parameter, this does not validate "SQL nullness" of + /// the Datum in question. The intention is that this core fn eschews an additional branch. + /// Just... don't use it if it might be null? + /// + /// This also should not be used as the primary conversion mechanism if it requires a MemCx, + /// as this is intended primarily to be used in cases where the datum is guaranteed to be + /// detoasted already. + unsafe fn unbox<'src>(datum: Datum<'src>) -> Self::As<'src> + where + Self: 'src; +} + +macro_rules! unbox_int { + ($($int_ty:ty),*) => { + $( + unsafe impl UnboxDatum for $int_ty { + type As<'src> = $int_ty; + unsafe fn unbox<'src>(datum: Datum<'src>) -> Self::As<'src> where Self: 'src { + datum.0.value() as $int_ty + } + } + )* + } +} + +unbox_int! { + i8, i16, i32, i64 +} + +unsafe impl UnboxDatum for bool { + type As<'src> = bool; + #[inline] + unsafe fn unbox<'src>(datum: Datum<'src>) -> Self::As<'src> + where + Self: 'src, + { + datum.0.value() != 0 + } +} + +unsafe impl UnboxDatum for f32 { + type As<'src> = f32; + #[inline] + unsafe fn unbox<'src>(datum: Datum<'src>) -> Self::As<'src> + where + Self: 'src, + { + f32::from_bits(datum.0.value() as u32) + } +} + +unsafe impl UnboxDatum for f64 { + type As<'src> = f64; + #[inline] + unsafe fn unbox<'src>(datum: Datum<'src>) -> Self::As<'src> + where + Self: 'src, + { + f64::from_bits(datum.0.value() as u64) + } +} + +unsafe impl UnboxDatum for str { + type As<'src> = &'src str; + #[inline] + unsafe fn unbox<'src>(datum: Datum<'src>) -> Self::As<'src> + where + Self: 'src, + { + unsafe { text_to_rust_str_unchecked(datum.0.cast_mut_ptr()) } + } +} + +unsafe impl UnboxDatum for &str { + type As<'src> = &'src str where Self: 'src; + #[inline] + unsafe fn unbox<'src>(datum: Datum<'src>) -> Self::As<'src> + where + Self: 'src, + { + unsafe { text_to_rust_str_unchecked(datum.0.cast_mut_ptr()) } + } +} + +unsafe impl UnboxDatum for CStr { + type As<'src> = &'src CStr; + #[inline] + unsafe fn unbox<'src>(datum: Datum<'src>) -> Self::As<'src> + where + Self: 'src, + { + unsafe { CStr::from_ptr(datum.0.cast_mut_ptr()) } + } +} + +unsafe impl UnboxDatum for &CStr { + type As<'src> = &'src CStr where Self: 'src; + #[inline] + unsafe fn unbox<'src>(datum: Datum<'src>) -> Self::As<'src> + where + Self: 'src, + { + unsafe { CStr::from_ptr(datum.0.cast_mut_ptr()) } + } +} + +unsafe impl UnboxDatum for [u8] { + type As<'src> = &'src [u8]; + #[inline] + unsafe fn unbox<'src>(datum: Datum<'src>) -> Self::As<'src> + where + Self: 'src, + { + unsafe { varlena_to_byte_slice(datum.0.cast_mut_ptr()) } + } +} + +unsafe impl UnboxDatum for &[u8] { + type As<'src> = &'src [u8] where Self: 'src; + #[inline] + unsafe fn unbox<'src>(datum: Datum<'src>) -> Self::As<'src> + where + Self: 'src, + { + unsafe { varlena_to_byte_slice(datum.0.cast_mut_ptr()) } + } +} + +unsafe impl UnboxDatum for pg_sys::Oid { + type As<'src> = pg_sys::Oid; + #[inline] + unsafe fn unbox<'src>(datum: Datum<'src>) -> Self::As<'src> + where + Self: 'src, + { + pg_sys::Oid::from(datum.0.value() as u32) + } +} + +unsafe impl UnboxDatum for String { + type As<'src> = String; + #[inline] + unsafe fn unbox<'src>(datum: Datum<'src>) -> Self::As<'src> + where + Self: 'src, + { + unsafe { str::unbox(datum) }.to_owned() + } +} + +unsafe impl UnboxDatum for CString { + type As<'src> = CString; + #[inline] + unsafe fn unbox<'src>(datum: Datum<'src>) -> Self::As<'src> + where + Self: 'src, + { + unsafe { CStr::unbox(datum) }.to_owned() + } +} + +unsafe impl UnboxDatum for pg_sys::Datum { + type As<'src> = pg_sys::Datum; + #[inline] + unsafe fn unbox<'src>(datum: Datum<'src>) -> Self::As<'src> + where + Self: 'src, + { + datum.0 + } +} + +unsafe impl UnboxDatum for Uuid { + type As<'src> = Uuid; + #[inline] + unsafe fn unbox<'src>(datum: Datum<'src>) -> Self::As<'src> + where + Self: 'src, + { + Uuid::from_bytes(datum.0.cast_mut_ptr::<[u8; 16]>().read()) + } +} + +unsafe impl UnboxDatum for Date { + type As<'src> = Date; + #[inline] + unsafe fn unbox<'src>(datum: Datum<'src>) -> Self::As<'src> + where + Self: 'src, + { + Date::try_from(i32::unbox(datum)).unwrap_unchecked() + } +} + +unsafe impl UnboxDatum for Time { + type As<'src> = Time; + #[inline] + unsafe fn unbox<'src>(datum: Datum<'src>) -> Self::As<'src> + where + Self: 'src, + { + Time::try_from(i64::unbox(datum)).unwrap_unchecked() + } +} + +unsafe impl UnboxDatum for Timestamp { + type As<'src> = Timestamp; + #[inline] + unsafe fn unbox<'src>(datum: Datum<'src>) -> Self::As<'src> + where + Self: 'src, + { + Timestamp::try_from(i64::unbox(datum)).unwrap_unchecked() + } +} + +unsafe impl UnboxDatum for TimestampWithTimeZone { + type As<'src> = TimestampWithTimeZone; + #[inline] + unsafe fn unbox<'src>(datum: Datum<'src>) -> Self::As<'src> + where + Self: 'src, + { + TimestampWithTimeZone::try_from(i64::unbox(datum)).unwrap_unchecked() + } +} + +macro_rules! unbox_with_fromdatum { + ($($from_ty:ty,)*) => { + $( + unsafe impl UnboxDatum for $from_ty { + type As<'src> = $from_ty; + unsafe fn unbox<'src>(datum: Datum<'src>) -> Self::As<'src> where Self: 'src { + Self::from_datum(datum.0, false).unwrap() + } + } + )* + } +} + +unbox_with_fromdatum! { + TimeWithTimeZone, AnyNumeric, char, pg_sys::Point, Interval, pg_sys::BOX, +} + +unsafe impl UnboxDatum for PgHeapTuple<'_, crate::AllocatedByRust> { + type As<'src> = PgHeapTuple<'src, AllocatedByRust> where Self: 'src; + #[inline] + unsafe fn unbox<'src>(d: Datum<'src>) -> Self::As<'src> + where + Self: 'src, + { + PgHeapTuple::from_datum(d.0, false).unwrap() + } +} + +unsafe impl UnboxDatum for Array<'_, T> { + type As<'src> = Array<'src, T> where Self: 'src; + unsafe fn unbox<'src>(d: Datum<'src>) -> Array<'src, T> + where + Self: 'src, + { + Array::from_datum(d.0, false).unwrap() + } +} + +unsafe impl UnboxDatum for VariadicArray<'_, T> { + type As<'src> = VariadicArray<'src, T> where Self: 'src; + unsafe fn unbox<'src>(d: Datum<'src>) -> VariadicArray<'src, T> + where + Self: 'src, + { + VariadicArray::from_datum(d.0, false).unwrap() + } +} + +unsafe impl UnboxDatum for Numeric { + type As<'src> = Numeric; + #[inline] + unsafe fn unbox<'src>(d: Datum<'src>) -> Self::As<'src> + where + Self: 'src, + { + Numeric::from_datum(d.0, false).unwrap() + } +} + +unsafe impl UnboxDatum for PgBox { + type As<'src> = PgBox where Self: 'src; + #[inline] + unsafe fn unbox<'src>(d: Datum<'src>) -> Self::As<'src> + where + Self: 'src, + { + PgBox::from_datum(d.0, false).unwrap() + } +} diff --git a/pgrx/src/fcinfo.rs b/pgrx/src/fcinfo.rs index 0b1c41a61..01fa8e027 100644 --- a/pgrx/src/fcinfo.rs +++ b/pgrx/src/fcinfo.rs @@ -95,6 +95,7 @@ macro_rules! variadic { /// /// We also cannot ensure that the specified Rust type `T` is compatible with whatever the /// underlying datum is at the argument `num` position. This too, is your responsibility +// TODO: should relate back to lifetimes and use a bound like T: UnboxDatum<'dat>? #[inline] pub unsafe fn pg_getarg(fcinfo: pg_sys::FunctionCallInfo, num: usize) -> Option { let datum = pg_get_nullable_datum(fcinfo, num); diff --git a/pgrx/src/heap_tuple.rs b/pgrx/src/heap_tuple.rs index 7d13e6a88..96324578a 100644 --- a/pgrx/src/heap_tuple.rs +++ b/pgrx/src/heap_tuple.rs @@ -10,7 +10,7 @@ //! Provides a safe interface to Postgres `HeapTuple` objects. //! //! [`PgHeapTuple`]s also describe composite types as defined by [`pgrx::composite_type!()`][crate::composite_type]. -use crate::datum::lookup_type_name; +use crate::datum::{lookup_type_name, UnboxDatum}; use crate::{ heap_getattr_raw, pg_sys, trigger_fired_by_delete, trigger_fired_by_insert, trigger_fired_by_update, trigger_fired_for_statement, AllocatedByPostgres, AllocatedByRust, @@ -52,12 +52,12 @@ pub enum PgHeapTupleError { /// allocated by Postgres, it is not mutable until [`PgHeapTuple::into_owned`] is called. /// /// [`PgHeapTuple`]s also describe composite types as defined by [`pgrx::composite_type!()`][crate::composite_type]. -pub struct PgHeapTuple<'a, AllocatedBy: WhoAllocated> { +pub struct PgHeapTuple<'mcx, AllocatedBy: WhoAllocated> { tuple: PgBox, - tupdesc: PgTupleDesc<'a>, + tupdesc: PgTupleDesc<'mcx>, } -impl<'a> FromDatum for PgHeapTuple<'a, AllocatedByRust> { +impl FromDatum for PgHeapTuple<'_, AllocatedByRust> { unsafe fn from_polymorphic_datum( composite: pg_sys::Datum, is_null: bool, @@ -89,7 +89,7 @@ impl<'a> FromDatum for PgHeapTuple<'a, AllocatedByRust> { } } -impl<'a> PgHeapTuple<'a, AllocatedByPostgres> { +impl<'mcx> PgHeapTuple<'mcx, AllocatedByPostgres> { /// Creates a new [PgHeapTuple] from a [PgTupleDesc] and a [pg_sys::HeapTuple] pointer. The /// returned [PgHeapTuple] will be considered by have been allocated by Postgres and is not mutable /// until [PgHeapTuple::into_owned] is called. @@ -99,7 +99,10 @@ impl<'a> PgHeapTuple<'a, AllocatedByPostgres> { /// This function is unsafe as we cannot guarantee that the [pg_sys::HeapTuple] pointer is valid, /// nor can we guaratee that the provided [PgTupleDesc] properly describes the structure of /// the heap tuple. - pub unsafe fn from_heap_tuple(tupdesc: PgTupleDesc<'a>, heap_tuple: pg_sys::HeapTuple) -> Self { + pub unsafe fn from_heap_tuple( + tupdesc: PgTupleDesc<'mcx>, + heap_tuple: pg_sys::HeapTuple, + ) -> Self { Self { tuple: PgBox::from_pg(heap_tuple), tupdesc } } @@ -117,9 +120,9 @@ impl<'a> PgHeapTuple<'a, AllocatedByPostgres> { /// argument are valid or that it's being used in the context of a firing trigger, which necessitates /// Postgres internal state be correct for executing a trigger. pub unsafe fn from_trigger_data( - trigger_data: &'a pg_sys::TriggerData, + trigger_data: &'mcx pg_sys::TriggerData, which_tuple: TriggerTuple, - ) -> Option> { + ) -> Option> { if trigger_fired_for_statement(trigger_data.tg_event) { // there is no HeapTuple for a statement-level trigger as such triggers aren't run // per-row @@ -171,7 +174,7 @@ impl<'a> PgHeapTuple<'a, AllocatedByPostgres> { /// Consumes a [`PgHeapTuple`] considered to be allocated by Postgres and transforms it into one /// that is considered allocated by Rust. This is accomplished by copying the underlying [pg_sys::HeapTupleData]. - pub fn into_owned(self) -> PgHeapTuple<'a, AllocatedByRust> { + pub fn into_owned(self) -> PgHeapTuple<'mcx, AllocatedByRust> { let copy = unsafe { pg_sys::heap_copytuple(self.tuple.into_pg()) }; PgHeapTuple { tuple: unsafe { PgBox::::from_rust(copy) }, @@ -180,7 +183,7 @@ impl<'a> PgHeapTuple<'a, AllocatedByPostgres> { } } -impl<'a> PgHeapTuple<'a, AllocatedByRust> { +impl<'mcx> PgHeapTuple<'mcx, AllocatedByRust> { /** Create a new heap tuple in the shape of a defined composite type ```rust,no_run @@ -206,7 +209,7 @@ impl<'a> PgHeapTuple<'a, AllocatedByRust> { */ pub fn new_composite_type( type_name: &str, - ) -> Result, PgHeapTupleError> { + ) -> Result, PgHeapTupleError> { let tuple_desc = PgTupleDesc::for_composite_type(type_name) .ok_or_else(|| PgHeapTupleError::NoSuchType(type_name.to_string()))?; @@ -215,7 +218,7 @@ impl<'a> PgHeapTuple<'a, AllocatedByRust> { pub fn new_composite_type_by_oid( typoid: pg_sys::Oid, - ) -> Result, PgHeapTupleError> { + ) -> Result, PgHeapTupleError> { PgTryBuilder::new(|| { let tuple_desc = PgTupleDesc::for_composite_type_by_oid(typoid) .ok_or(PgHeapTupleError::NotACompositeType(typoid))?; @@ -255,9 +258,9 @@ impl<'a> PgHeapTuple<'a, AllocatedByRust> { /// This function is unsafe as we cannot guarantee the provided [`pg_sys::Datum`]s are valid /// as the specified [`PgTupleDesc`] might expect pub unsafe fn from_datums>>( - tupdesc: PgTupleDesc<'a>, + tupdesc: PgTupleDesc<'mcx>, datums: I, - ) -> Result, PgHeapTupleError> { + ) -> Result, PgHeapTupleError> { let iter = datums.into_iter(); let mut datums = Vec::::with_capacity(iter.size_hint().1.unwrap_or(1)); let mut nulls = Vec::::with_capacity(iter.size_hint().1.unwrap_or(1)); @@ -404,7 +407,7 @@ impl<'a> PgHeapTuple<'a, AllocatedByRust> { } } -impl<'a, AllocatedBy: WhoAllocated> IntoDatum for PgHeapTuple<'a, AllocatedBy> { +impl<'mcx, AllocatedBy: WhoAllocated> IntoDatum for PgHeapTuple<'mcx, AllocatedBy> { // Delegate to `into_composite_datum()` as this will normally be used with composite types. // See `into_trigger_datum()` if using as a trigger. fn into_datum(self) -> Option { @@ -430,7 +433,7 @@ impl<'a, AllocatedBy: WhoAllocated> IntoDatum for PgHeapTuple<'a, AllocatedBy> { } } -impl<'a, AllocatedBy: WhoAllocated> PgHeapTuple<'a, AllocatedBy> { +impl<'mcx, AllocatedBy: WhoAllocated> PgHeapTuple<'mcx, AllocatedBy> { /// Consume this [`PgHeapTuple`] and return a composite Datum representation, containing the tuple /// data and the corresponding tuple descriptor information. pub fn into_composite_datum(self) -> Option { @@ -463,8 +466,8 @@ impl<'a, AllocatedBy: WhoAllocated> PgHeapTuple<'a, AllocatedBy> { /// /// The return value is `(attribute_number: NonZeroUsize, attribute_info: &pg_sys::FormData_pg_attribute)`. pub fn attributes( - &'a self, - ) -> impl std::iter::Iterator { + &self, + ) -> impl std::iter::Iterator { self.tupdesc.iter().enumerate().map(|(i, att)| (NonZeroUsize::new(i + 1).unwrap(), att)) } @@ -473,9 +476,9 @@ impl<'a, AllocatedBy: WhoAllocated> PgHeapTuple<'a, AllocatedBy> { /// Returns `None` if the attribute number is out of bounds. #[inline] pub fn get_attribute_by_index( - &'a self, + &self, index: NonZeroUsize, - ) -> Option<&'a pg_sys::FormData_pg_attribute> { + ) -> Option<&pg_sys::FormData_pg_attribute> { self.tupdesc.get(index.get() - 1) } @@ -483,9 +486,9 @@ impl<'a, AllocatedBy: WhoAllocated> PgHeapTuple<'a, AllocatedBy> { /// /// Returns `None` if the attribute name is not found. pub fn get_attribute_by_name( - &'a self, + &self, name: &str, - ) -> Option<(NonZeroUsize, &'a pg_sys::FormData_pg_attribute)> { + ) -> Option<(NonZeroUsize, &pg_sys::FormData_pg_attribute)> { for i in 0..self.len() { let i = NonZeroUsize::new(i + 1).unwrap(); let att = self.get_attribute_by_index(i).unwrap(); @@ -505,10 +508,10 @@ impl<'a, AllocatedBy: WhoAllocated> PgHeapTuple<'a, AllocatedBy> { /// - return [`TryFromDatumError::NoSuchAttributeName`] if the attribute does not exist /// - return [`TryFromDatumError::IncompatibleTypes`] if the Rust type of the `value` is not /// compatible with the attribute's Postgres type - pub fn get_by_name( - &self, - attname: &str, - ) -> Result, TryFromDatumError> { + pub fn get_by_name<'tup, T>(&'tup self, attname: &str) -> Result, TryFromDatumError> + where + T: FromDatum + IntoDatum + UnboxDatum = T> + 'tup, + { // find the attribute number by name for att in self.tupdesc.iter() { if att.name() == attname { @@ -529,10 +532,13 @@ impl<'a, AllocatedBy: WhoAllocated> PgHeapTuple<'a, AllocatedBy> { /// - return [`TryFromDatumError::NoSuchAttributeNumber`] if the attribute does not exist /// - return [`TryFromDatumError::IncompatibleTypes`] if the Rust type of the `value` is not /// compatible with the attribute's Postgres type - pub fn get_by_index( - &self, + pub fn get_by_index<'tup, T>( + &'tup self, attno: NonZeroUsize, - ) -> Result, TryFromDatumError> { + ) -> Result, TryFromDatumError> + where + T: FromDatum + IntoDatum + UnboxDatum = T> + 'tup, + { unsafe { // tuple descriptor attribute numbers are zero-based match self.tupdesc.get(attno.get() - 1) { @@ -639,7 +645,7 @@ use pgrx::{prelude::*, AllocatedByRust}; #[pg_extern] fn this_dog_name_or_your_favorite_dog_name( dog: pgrx::default!(pgrx::composite_type!("Dog"), "ROW('Nami', 0)::Dog"), -) -> &str { +) -> String { // Gets resolved to: let dog: PgHeapTuple = dog; diff --git a/pgrx/src/pgbox.rs b/pgrx/src/pgbox.rs index aa720d041..917ec4c42 100644 --- a/pgrx/src/pgbox.rs +++ b/pgrx/src/pgbox.rs @@ -92,7 +92,7 @@ use std::ptr::NonNull; /// ``` #[repr(transparent)] pub struct PgBox { - ptr: Option>, + ptr: Option>, // TODO: add this memcx's lifetime __marker: PhantomData, } diff --git a/pgrx/src/spi.rs b/pgrx/src/spi.rs index 5171f620b..6fb157294 100644 --- a/pgrx/src/spi.rs +++ b/pgrx/src/spi.rs @@ -366,7 +366,13 @@ impl Spi { /// This function will panic if for some reason it's unable to "connect" to Postgres' SPI /// system. At the time of this writing, that's actually impossible as the underlying function /// ([`pg_sys::SPI_connect()`]) **always** returns a successful response. - pub fn connect) -> R>(f: F) -> R { + pub fn connect(f: F) -> R + where + F: FnOnce(SpiClient<'_>) -> R, /* TODO: redesign this with 2 lifetimes: + - 'conn ~= CurrentMemoryContext after connection + - 'ret ~= SPI_palloc's context + */ + { // connect to SPI // // Postgres documents (https://www.postgresql.org/docs/current/spi-spi-connect.html) that