From e7a9248b005d6139ca6187787ab431ddd8e6c8a7 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Fri, 23 Feb 2024 13:48:56 -0800 Subject: [PATCH 01/54] Introduce Text type for BorrowDatum This is a bit MVP-ish at the moment. --- pgrx/src/lib.rs | 1 + pgrx/src/text.rs | 63 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+) create mode 100644 pgrx/src/text.rs diff --git a/pgrx/src/lib.rs b/pgrx/src/lib.rs index 3359a4dc48..fd22eeebd7 100644 --- a/pgrx/src/lib.rs +++ b/pgrx/src/lib.rs @@ -73,6 +73,7 @@ pub mod spi; #[cfg(feature = "cshim")] pub mod spinlock; pub mod stringinfo; +pub mod text; pub mod trigger_support; pub mod tupdesc; pub mod varlena; diff --git a/pgrx/src/text.rs b/pgrx/src/text.rs new file mode 100644 index 0000000000..69da300562 --- /dev/null +++ b/pgrx/src/text.rs @@ -0,0 +1,63 @@ +use crate::datum::{BorrowDatum, Datum}; +use crate::pgrx_sql_entity_graph::metadata::{ + ArgumentError, Returns, ReturnsError, SqlMapping, SqlTranslatable, +}; +use crate::{pg_sys, varlena}; +use core::ops::{Deref, DerefMut}; +use core::ptr; + +/// A more strongly-typed representation of a Postgres string, AKA `TEXT`. +/// A pointer to this points to a byte array, which includes a variable-length header +/// and an unsized data field which is... often but not always UTF-8. +#[repr(transparent)] +pub struct Text([u8]); + +// TODO(0.12.0): strip this and make Text forward its impl to BStr fn instead +impl Deref for Text { + type Target = str; + fn deref(&self) -> &str { + let self_ptr = self as *const Text as *const pg_sys::varlena; + unsafe { varlena::text_to_rust_str(self_ptr).unwrap() } + } +} + +// TODO(0.12.0): strip this and make Text forward its impl to BStr fn instead +impl DerefMut for Text { + fn deref_mut(&mut self) -> &mut str { + let self_ptr = self as *mut Text as *mut pg_sys::varlena; + unsafe { + let len = varlena::varsize_any_exhdr(self_ptr); + let data = varlena::vardata_any(self_ptr); + + &mut *(ptr::slice_from_raw_parts_mut(data as *mut u8, len) as *mut str) + } + } +} + +unsafe impl BorrowDatum for Text { + unsafe fn borrow_from<'dat>(datum: &'dat Datum<'_>) -> &'dat Self { + let ptr = datum as *const Datum<'_> as *const *const pg_sys::varlena; + unsafe { + let ptr = *ptr; + let len = varlena::varsize_any(ptr); + &*(ptr::slice_from_raw_parts(ptr as *const u8, len) as *const Text) + } + } + unsafe fn borrow_mut_from<'dat>(datum: &'dat mut Datum<'_>) -> &'dat mut Self { + let ptr = datum as *mut Datum<'_> as *mut *mut pg_sys::varlena; + unsafe { + let ptr = *ptr; + let len = varlena::varsize_any(ptr); + &mut *(ptr::slice_from_raw_parts(ptr as *mut u8, len) as *mut Text) + } + } +} + +unsafe impl<'dat> SqlTranslatable for &'dat Text { + fn argument_sql() -> Result { + Ok(SqlMapping::literal("TEXT")) + } + fn return_sql() -> Result { + Ok(Returns::One(SqlMapping::literal("TEXT"))) + } +} From 1ed08e064e4beceb840c82a4638e9b2bbd887c24 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Fri, 23 Feb 2024 14:22:02 -0800 Subject: [PATCH 02/54] Template out FlatArray Right now, it only needs to typecheck. --- pgrx/src/array.rs | 44 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/pgrx/src/array.rs b/pgrx/src/array.rs index ee31128737..5f1c53bf2a 100644 --- a/pgrx/src/array.rs +++ b/pgrx/src/array.rs @@ -8,16 +8,58 @@ //LICENSE //LICENSE Use of this source code is governed by the MIT license that can be found in the LICENSE file. #![allow(clippy::precedence)] -use crate::datum::Array; +#![allow(unused)] +use crate::datum::{Array, BorrowDatum, Datum}; +use crate::pgrx_sql_entity_graph::metadata::{ + ArgumentError, Returns, ReturnsError, SqlMapping, SqlTranslatable, +}; use crate::toast::{Toast, Toasty}; use crate::{layout, pg_sys, varlena}; use bitvec::ptr::{self as bitptr, BitPtr, BitPtrError, Mut}; use bitvec::slice::BitSlice; use core::ptr::{self, NonNull}; use core::slice; +use core::marker::PhantomData; mod port; +/// &FlatArray is akin to &ArrayType +#[repr(C)] +pub struct FlatArray<'mcx, T: ?Sized> { + scalar: PhantomData<&'mcx T>, + head: pg_sys::ArrayType, + tail: [u8], +} + +impl<'mcx, T> FlatArray<'mcx, T> +where + T: ?Sized + BorrowDatum, +{ + pub fn get(&self, index: usize) -> Option<&T> { + // FIXME: consider nullability + self.datum_at(index).map(|datum| unsafe { T::borrow_from(datum) }) + } + + pub fn get_mut(&mut self, index: usize) -> Option<&mut T> { + // FIXME: consider nullability + self.datum_mut_at(index).map(|datum| unsafe { T::borrow_mut_from(datum) }) + } + + fn datum_at(&self, index: usize) -> Option<&Datum<'_>> { + let data_ptr = unsafe { ARR_DATA_PTR(ptr::addr_of!(self.head).cast_mut()) }; + todo!(); + // FIXME: replace with actual impl instead of something that merely typechecks + Some(unsafe { &*(data_ptr as *const Datum<'_>) }) + } + + fn datum_mut_at(&mut self, index: usize) -> Option<&mut Datum<'_>> { + let data_ptr = unsafe { ARR_DATA_PTR(ptr::addr_of_mut!(self.head)) }; + todo!(); + // FIXME: replace with actual impl instead of something that merely typechecks + Some(unsafe { &mut *(data_ptr as *mut Datum<'_>) }) + } +} + /** An aligned, dereferenceable `NonNull` with low-level accessors. From 40bb692810af415aeff33fbb50df023154a8d5b0 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Fri, 23 Feb 2024 18:06:34 -0800 Subject: [PATCH 03/54] impl BorrowDatum for FlatArray --- pgrx/src/array.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/pgrx/src/array.rs b/pgrx/src/array.rs index 5f1c53bf2a..af67eada6c 100644 --- a/pgrx/src/array.rs +++ b/pgrx/src/array.rs @@ -60,6 +60,25 @@ where } } +unsafe impl BorrowDatum for FlatArray<'_, T> { + unsafe fn borrow_from<'dat>(datum: &'dat Datum<'_>) -> &'dat Self { + let ptr = datum as *const Datum<'_> as *const *const pg_sys::varlena; + unsafe { + let ptr = *ptr; + let len = varlena::varsize_any(ptr) - mem::size_of::(); + &*(ptr::slice_from_raw_parts(ptr as *const u8, len) as *const Self) + } + } + unsafe fn borrow_mut_from<'dat>(datum: &'dat mut Datum<'_>) -> &'dat mut Self { + let ptr = datum as *mut Datum<'_> as *mut *mut pg_sys::varlena; + unsafe { + let ptr = *ptr; + let len = varlena::varsize_any(ptr) - mem::size_of::(); + &mut *(ptr::slice_from_raw_parts(ptr as *mut u8, len) as *mut Self) + } + } +} + /** An aligned, dereferenceable `NonNull` with low-level accessors. From 68815dbbb2509a3c29efdf37a602aff5f6d65492 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Sat, 24 Feb 2024 14:03:16 -0800 Subject: [PATCH 04/54] Sketch out some more fn for FlatArray --- pgrx/src/array.rs | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/pgrx/src/array.rs b/pgrx/src/array.rs index af67eada6c..89ac48770e 100644 --- a/pgrx/src/array.rs +++ b/pgrx/src/array.rs @@ -45,18 +45,34 @@ where self.datum_mut_at(index).map(|datum| unsafe { T::borrow_mut_from(datum) }) } - fn datum_at(&self, index: usize) -> Option<&Datum<'_>> { + fn datum_at(&self, index: usize) -> Option<&Datum<'mcx>> { let data_ptr = unsafe { ARR_DATA_PTR(ptr::addr_of!(self.head).cast_mut()) }; - todo!(); + // todo!(); // FIXME: replace with actual impl instead of something that merely typechecks - Some(unsafe { &*(data_ptr as *const Datum<'_>) }) + Some(unsafe { &*(data_ptr as *const Datum<'mcx>) }) } - fn datum_mut_at(&mut self, index: usize) -> Option<&mut Datum<'_>> { + fn datum_mut_at(&mut self, index: usize) -> Option<&mut Datum<'mcx>> { let data_ptr = unsafe { ARR_DATA_PTR(ptr::addr_of_mut!(self.head)) }; - todo!(); + // todo!(); // FIXME: replace with actual impl instead of something that merely typechecks - Some(unsafe { &mut *(data_ptr as *mut Datum<'_>) }) + Some(unsafe { &mut *(data_ptr as *mut Datum<'mcx>) }) + } + + fn nullbitmap(&self) -> &[u8] { + todo!() + } + + /// Obtain a mutable reference to the null bitmap. + /// # Safety + /// The items set to 1 in the null bitmap are treated as valid. + /// If the null bitmap covers any extent, then trailing bits must be set to 0 and + /// all elements that have a 1 marking them must be initialized. The null bitmap + /// is linear but the layout of elements may be nonlinear, so for some arrays + /// the positions of the null bits that must be set or unset cannot be directly + /// calculated from the positions of the elements in the array, and vice versa. + unsafe fn nullbitmap_mut(&mut self) -> &mut [u8] { + todo!() } } From 4e1004ac5ac9ef3cba89d64e2f5fd17281c5ace5 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Sat, 24 Feb 2024 18:16:00 -0800 Subject: [PATCH 05/54] Copy nulls_mut into FlatArray --- pgrx/src/array.rs | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/pgrx/src/array.rs b/pgrx/src/array.rs index 89ac48770e..2e05a46c37 100644 --- a/pgrx/src/array.rs +++ b/pgrx/src/array.rs @@ -59,6 +59,12 @@ where Some(unsafe { &mut *(data_ptr as *mut Datum<'mcx>) }) } + fn len(&self) -> usize { + let ndims = self.head.ndim as usize; + let dims_ptr = unsafe { ARR_DIMS(ptr::addr_of!(self.head).cast_mut()) }; + unsafe { slice::from_raw_parts(dims_ptr, ndims).into_iter().sum::() as usize } + } + fn nullbitmap(&self) -> &[u8] { todo!() } @@ -74,6 +80,28 @@ where unsafe fn nullbitmap_mut(&mut self) -> &mut [u8] { todo!() } + + /** + Oxidized form of [ARR_NULLBITMAP(ArrayType*)][ARR_NULLBITMAP] + + If this returns None, the array cannot have nulls. + If this returns Some, it points to the bitslice that marks nulls in this array. + + Note that unlike the `is_null: bool` that appears elsewhere, here a 0 bit is null, + or possibly out of bounds for the final byte of the bitslice. + + [ARR_NULLBITMAP]: + */ + pub unsafe fn nulls_mut(&mut self) -> Option<&mut [u8]> { + let len = self.len() + 7 >> 3; // Obtains 0 if len was 0. + + // SAFETY: This obtains the nulls pointer from a function that must either + // return a null pointer or a pointer to a valid null bitmap. + unsafe { + let nulls_ptr = ARR_NULLBITMAP(ptr::addr_of_mut!(self.head)); + ptr::slice_from_raw_parts_mut(nulls_ptr, len).as_mut() + } + } } unsafe impl BorrowDatum for FlatArray<'_, T> { From aac87d7fd8a3cf2ba7986116111e9a05f0d14e21 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Sat, 24 Feb 2024 18:40:06 -0800 Subject: [PATCH 06/54] Add FlatArray::nulls --- pgrx/src/array.rs | 33 ++++++++++++++------------------- 1 file changed, 14 insertions(+), 19 deletions(-) diff --git a/pgrx/src/array.rs b/pgrx/src/array.rs index 2e05a46c37..57bc15895c 100644 --- a/pgrx/src/array.rs +++ b/pgrx/src/array.rs @@ -65,31 +65,26 @@ where unsafe { slice::from_raw_parts(dims_ptr, ndims).into_iter().sum::() as usize } } - fn nullbitmap(&self) -> &[u8] { - todo!() - } + pub fn nulls(&self) -> Option<&[u8]> { + let len = self.len() + 7 >> 3; // Obtains 0 if len was 0. - /// Obtain a mutable reference to the null bitmap. - /// # Safety - /// The items set to 1 in the null bitmap are treated as valid. - /// If the null bitmap covers any extent, then trailing bits must be set to 0 and - /// all elements that have a 1 marking them must be initialized. The null bitmap - /// is linear but the layout of elements may be nonlinear, so for some arrays - /// the positions of the null bits that must be set or unset cannot be directly - /// calculated from the positions of the elements in the array, and vice versa. - unsafe fn nullbitmap_mut(&mut self) -> &mut [u8] { - todo!() + // SAFETY: This obtains the nulls pointer from a function that must either + // return a null pointer or a pointer to a valid null bitmap. + unsafe { + let nulls_ptr = ARR_NULLBITMAP(ptr::addr_of!(self.head).cast_mut()); + ptr::slice_from_raw_parts(nulls_ptr, len).as_ref() + } } /** Oxidized form of [ARR_NULLBITMAP(ArrayType*)][ARR_NULLBITMAP] + If this returns None, the array *cannot* have nulls. + Note that unlike the `is_null: bool` that appears elsewhere, 1 is valid and 0 is null. - If this returns None, the array cannot have nulls. - If this returns Some, it points to the bitslice that marks nulls in this array. - - Note that unlike the `is_null: bool` that appears elsewhere, here a 0 bit is null, - or possibly out of bounds for the final byte of the bitslice. - + # Safety + Trailing bits must be set to 0, and all elements marked as valid (1) must be initialized. + The null bitmap is linear but the layout of elements may be nonlinear, so for some arrays + these cannot be calculated directly from each other. [ARR_NULLBITMAP]: */ pub unsafe fn nulls_mut(&mut self) -> Option<&mut [u8]> { From 4934fd42fbbd651c8e0fbd43ff7dbe5fba11593c Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Mon, 26 Feb 2024 08:22:52 -0800 Subject: [PATCH 07/54] Document FlatArray pub fns --- pgrx/src/array.rs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/pgrx/src/array.rs b/pgrx/src/array.rs index 57bc15895c..4c1e71ae75 100644 --- a/pgrx/src/array.rs +++ b/pgrx/src/array.rs @@ -35,13 +35,21 @@ impl<'mcx, T> FlatArray<'mcx, T> where T: ?Sized + BorrowDatum, { - pub fn get(&self, index: usize) -> Option<&T> { + /// Borrow the nth element. + /// + /// `FlatArray::nth` may have to iterate the array, thus it is named for `Iterator::nth`. + pub fn nth(&self, index: usize) -> Option<&T> { // FIXME: consider nullability + // FIXME: Become a dispatch to Iterator::nth self.datum_at(index).map(|datum| unsafe { T::borrow_from(datum) }) } - pub fn get_mut(&mut self, index: usize) -> Option<&mut T> { + /// Mutably borrow the nth element. + /// + /// `FlatArray::nth_mut` may have to iterate the array, thus it is named for `Iterator::nth`. + pub fn nth_mut(&mut self, index: usize) -> Option<&mut T> { // FIXME: consider nullability + // FIXME: Become a dispatch to Iterator::nth self.datum_mut_at(index).map(|datum| unsafe { T::borrow_mut_from(datum) }) } @@ -59,6 +67,9 @@ where Some(unsafe { &mut *(data_ptr as *mut Datum<'mcx>) }) } + /// Number of elements in the Array + /// + /// Note that for many Arrays, this doesn't have a linear relationship with array byte-len. fn len(&self) -> usize { let ndims = self.head.ndim as usize; let dims_ptr = unsafe { ARR_DIMS(ptr::addr_of!(self.head).cast_mut()) }; @@ -79,10 +90,10 @@ where /** Oxidized form of [ARR_NULLBITMAP(ArrayType*)][ARR_NULLBITMAP] If this returns None, the array *cannot* have nulls. - Note that unlike the `is_null: bool` that appears elsewhere, 1 is valid and 0 is null. + Note that unlike the `is_null: bool` that appears elsewhere, 1 is "valid" and 0 is "null". # Safety - Trailing bits must be set to 0, and all elements marked as valid (1) must be initialized. + Trailing bits must be set to 0, and all elements marked with 1 must be initialized. The null bitmap is linear but the layout of elements may be nonlinear, so for some arrays these cannot be calculated directly from each other. [ARR_NULLBITMAP]: From 742bcf5da0787ddb06a4685848ee26f93c81e93d Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Mon, 26 Feb 2024 08:46:26 -0800 Subject: [PATCH 08/54] Oh that was wildly unsound --- pgrx/src/array.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pgrx/src/array.rs b/pgrx/src/array.rs index 4c1e71ae75..5936f9e027 100644 --- a/pgrx/src/array.rs +++ b/pgrx/src/array.rs @@ -60,7 +60,12 @@ where Some(unsafe { &*(data_ptr as *const Datum<'mcx>) }) } - fn datum_mut_at(&mut self, index: usize) -> Option<&mut Datum<'mcx>> { + /// # Safety + /// This is an incredibly naughty function for arrays where *the scalar type is + /// smaller than Datum*. This is because when we return `&mut Datum`, that could overlap + /// with the next `&mut Datum`, so it must essentially always be converted. + /// Consider replacing this with raw pointers. + unsafe fn datum_mut_at(&mut self, index: usize) -> Option<&mut Datum<'mcx>> { let data_ptr = unsafe { ARR_DATA_PTR(ptr::addr_of_mut!(self.head)) }; // todo!(); // FIXME: replace with actual impl instead of something that merely typechecks From 536c7292d79dc58a82ef1b77cba3f7ada81a3287 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Mon, 26 Feb 2024 09:09:32 -0800 Subject: [PATCH 09/54] Fix imports --- pgrx/src/array.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/pgrx/src/array.rs b/pgrx/src/array.rs index 5936f9e027..ee8f855f36 100644 --- a/pgrx/src/array.rs +++ b/pgrx/src/array.rs @@ -18,7 +18,7 @@ use crate::{layout, pg_sys, varlena}; use bitvec::ptr::{self as bitptr, BitPtr, BitPtrError, Mut}; use bitvec::slice::BitSlice; use core::ptr::{self, NonNull}; -use core::slice; +use core::{slice, mem}; use core::marker::PhantomData; mod port; @@ -54,7 +54,7 @@ where } fn datum_at(&self, index: usize) -> Option<&Datum<'mcx>> { - let data_ptr = unsafe { ARR_DATA_PTR(ptr::addr_of!(self.head).cast_mut()) }; + let data_ptr = unsafe { port::ARR_DATA_PTR(ptr::addr_of!(self.head).cast_mut()) }; // todo!(); // FIXME: replace with actual impl instead of something that merely typechecks Some(unsafe { &*(data_ptr as *const Datum<'mcx>) }) @@ -66,7 +66,7 @@ where /// with the next `&mut Datum`, so it must essentially always be converted. /// Consider replacing this with raw pointers. unsafe fn datum_mut_at(&mut self, index: usize) -> Option<&mut Datum<'mcx>> { - let data_ptr = unsafe { ARR_DATA_PTR(ptr::addr_of_mut!(self.head)) }; + let data_ptr = unsafe { port::ARR_DATA_PTR(ptr::addr_of_mut!(self.head)) }; // todo!(); // FIXME: replace with actual impl instead of something that merely typechecks Some(unsafe { &mut *(data_ptr as *mut Datum<'mcx>) }) @@ -77,7 +77,7 @@ where /// Note that for many Arrays, this doesn't have a linear relationship with array byte-len. fn len(&self) -> usize { let ndims = self.head.ndim as usize; - let dims_ptr = unsafe { ARR_DIMS(ptr::addr_of!(self.head).cast_mut()) }; + let dims_ptr = unsafe { port::ARR_DIMS(ptr::addr_of!(self.head).cast_mut()) }; unsafe { slice::from_raw_parts(dims_ptr, ndims).into_iter().sum::() as usize } } @@ -87,7 +87,7 @@ where // SAFETY: This obtains the nulls pointer from a function that must either // return a null pointer or a pointer to a valid null bitmap. unsafe { - let nulls_ptr = ARR_NULLBITMAP(ptr::addr_of!(self.head).cast_mut()); + let nulls_ptr = port::ARR_NULLBITMAP(ptr::addr_of!(self.head).cast_mut()); ptr::slice_from_raw_parts(nulls_ptr, len).as_ref() } } @@ -109,7 +109,7 @@ where // SAFETY: This obtains the nulls pointer from a function that must either // return a null pointer or a pointer to a valid null bitmap. unsafe { - let nulls_ptr = ARR_NULLBITMAP(ptr::addr_of_mut!(self.head)); + let nulls_ptr = port::ARR_NULLBITMAP(ptr::addr_of_mut!(self.head)); ptr::slice_from_raw_parts_mut(nulls_ptr, len).as_mut() } } From 48b493304693ec76da22582bf9dbd138bb6fb219 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Mon, 26 Feb 2024 09:10:10 -0800 Subject: [PATCH 10/54] Add unsafe {} for unsafe fn --- pgrx/src/array.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pgrx/src/array.rs b/pgrx/src/array.rs index ee8f855f36..31271fa3de 100644 --- a/pgrx/src/array.rs +++ b/pgrx/src/array.rs @@ -50,7 +50,7 @@ where pub fn nth_mut(&mut self, index: usize) -> Option<&mut T> { // FIXME: consider nullability // FIXME: Become a dispatch to Iterator::nth - self.datum_mut_at(index).map(|datum| unsafe { T::borrow_mut_from(datum) }) + unsafe { self.datum_mut_at(index).map(|datum| unsafe { T::borrow_mut_from(datum) }) } } fn datum_at(&self, index: usize) -> Option<&Datum<'mcx>> { @@ -63,7 +63,7 @@ where /// # Safety /// This is an incredibly naughty function for arrays where *the scalar type is /// smaller than Datum*. This is because when we return `&mut Datum`, that could overlap - /// with the next `&mut Datum`, so it must essentially always be converted. + /// with the next `&mut Datum`, so it must essentially always be converted before exposure. /// Consider replacing this with raw pointers. unsafe fn datum_mut_at(&mut self, index: usize) -> Option<&mut Datum<'mcx>> { let data_ptr = unsafe { port::ARR_DATA_PTR(ptr::addr_of_mut!(self.head)) }; From b9a33c38939c99184950bbd97929191aaefa7a19 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Tue, 27 Feb 2024 15:37:14 -0800 Subject: [PATCH 11/54] Completely redesign BorrowDatum --- pgrx/src/array.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pgrx/src/array.rs b/pgrx/src/array.rs index 31271fa3de..4161ae6d16 100644 --- a/pgrx/src/array.rs +++ b/pgrx/src/array.rs @@ -17,9 +17,9 @@ use crate::toast::{Toast, Toasty}; use crate::{layout, pg_sys, varlena}; use bitvec::ptr::{self as bitptr, BitPtr, BitPtrError, Mut}; use bitvec::slice::BitSlice; -use core::ptr::{self, NonNull}; -use core::{slice, mem}; use core::marker::PhantomData; +use core::ptr::{self, NonNull}; +use core::{mem, slice}; mod port; From d8452045a6e4c564a2caa397025687147809b613 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Tue, 27 Feb 2024 15:46:04 -0800 Subject: [PATCH 12/54] Fix BorrowDatum for FlatArray && Text Also comment out a shitton of code that doesn't compile anymore. --- pgrx/src/array.rs | 29 ++++++++++------------------- pgrx/src/text.rs | 18 +++++------------- 2 files changed, 15 insertions(+), 32 deletions(-) diff --git a/pgrx/src/array.rs b/pgrx/src/array.rs index 4161ae6d16..c8c69c142b 100644 --- a/pgrx/src/array.rs +++ b/pgrx/src/array.rs @@ -41,7 +41,8 @@ where pub fn nth(&self, index: usize) -> Option<&T> { // FIXME: consider nullability // FIXME: Become a dispatch to Iterator::nth - self.datum_at(index).map(|datum| unsafe { T::borrow_from(datum) }) + todo!() + // self.datum_at(index).map(|datum| unsafe { T::borrow_from(datum) }) } /// Mutably borrow the nth element. @@ -50,14 +51,14 @@ where pub fn nth_mut(&mut self, index: usize) -> Option<&mut T> { // FIXME: consider nullability // FIXME: Become a dispatch to Iterator::nth - unsafe { self.datum_mut_at(index).map(|datum| unsafe { T::borrow_mut_from(datum) }) } + todo!(); + // unsafe { self.datum_mut_at(index).map(|datum| unsafe { T::borrow_mut_from(datum) }) } } fn datum_at(&self, index: usize) -> Option<&Datum<'mcx>> { let data_ptr = unsafe { port::ARR_DATA_PTR(ptr::addr_of!(self.head).cast_mut()) }; - // todo!(); + todo!(); // FIXME: replace with actual impl instead of something that merely typechecks - Some(unsafe { &*(data_ptr as *const Datum<'mcx>) }) } /// # Safety @@ -67,9 +68,8 @@ where /// Consider replacing this with raw pointers. unsafe fn datum_mut_at(&mut self, index: usize) -> Option<&mut Datum<'mcx>> { let data_ptr = unsafe { port::ARR_DATA_PTR(ptr::addr_of_mut!(self.head)) }; - // todo!(); + todo!(); // FIXME: replace with actual impl instead of something that merely typechecks - Some(unsafe { &mut *(data_ptr as *mut Datum<'mcx>) }) } /// Number of elements in the Array @@ -116,20 +116,11 @@ where } unsafe impl BorrowDatum for FlatArray<'_, T> { - unsafe fn borrow_from<'dat>(datum: &'dat Datum<'_>) -> &'dat Self { - let ptr = datum as *const Datum<'_> as *const *const pg_sys::varlena; + const PASS: Option = Some(layout::PassBy::Ref); + unsafe fn point_from(ptr: *mut u8) -> *mut Self { unsafe { - let ptr = *ptr; - let len = varlena::varsize_any(ptr) - mem::size_of::(); - &*(ptr::slice_from_raw_parts(ptr as *const u8, len) as *const Self) - } - } - unsafe fn borrow_mut_from<'dat>(datum: &'dat mut Datum<'_>) -> &'dat mut Self { - let ptr = datum as *mut Datum<'_> as *mut *mut pg_sys::varlena; - unsafe { - let ptr = *ptr; - let len = varlena::varsize_any(ptr) - mem::size_of::(); - &mut *(ptr::slice_from_raw_parts(ptr as *mut u8, len) as *mut Self) + let len = varlena::varsize_any(ptr.cast()) - mem::size_of::(); + ptr::slice_from_raw_parts_mut(ptr, len) as *mut Self } } } diff --git a/pgrx/src/text.rs b/pgrx/src/text.rs index 69da300562..38b6b6a212 100644 --- a/pgrx/src/text.rs +++ b/pgrx/src/text.rs @@ -1,4 +1,5 @@ use crate::datum::{BorrowDatum, Datum}; +use crate::layout::PassBy; use crate::pgrx_sql_entity_graph::metadata::{ ArgumentError, Returns, ReturnsError, SqlMapping, SqlTranslatable, }; @@ -35,20 +36,11 @@ impl DerefMut for Text { } unsafe impl BorrowDatum for Text { - unsafe fn borrow_from<'dat>(datum: &'dat Datum<'_>) -> &'dat Self { - let ptr = datum as *const Datum<'_> as *const *const pg_sys::varlena; + const PASS: Option = Some(PassBy::Ref); + unsafe fn point_from(ptr: *mut u8) -> *mut Self { unsafe { - let ptr = *ptr; - let len = varlena::varsize_any(ptr); - &*(ptr::slice_from_raw_parts(ptr as *const u8, len) as *const Text) - } - } - unsafe fn borrow_mut_from<'dat>(datum: &'dat mut Datum<'_>) -> &'dat mut Self { - let ptr = datum as *mut Datum<'_> as *mut *mut pg_sys::varlena; - unsafe { - let ptr = *ptr; - let len = varlena::varsize_any(ptr); - &mut *(ptr::slice_from_raw_parts(ptr as *mut u8, len) as *mut Text) + let len = varlena::varsize_any(ptr.cast()); + ptr::slice_from_raw_parts_mut(ptr, len) as *mut Text } } } From ba5e73f048e017d807bf08bf1c21786439a029c0 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Wed, 28 Feb 2024 13:16:42 -0800 Subject: [PATCH 13/54] Make FlatArray typecheck --- pgrx/src/array.rs | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/pgrx/src/array.rs b/pgrx/src/array.rs index c8c69c142b..8039108960 100644 --- a/pgrx/src/array.rs +++ b/pgrx/src/array.rs @@ -24,6 +24,8 @@ use core::{mem, slice}; mod port; /// &FlatArray is akin to &ArrayType +/// +/// `pgrx::datum::Array` is essentially `&FlatArray` #[repr(C)] pub struct FlatArray<'mcx, T: ?Sized> { scalar: PhantomData<&'mcx T>, @@ -41,8 +43,7 @@ where pub fn nth(&self, index: usize) -> Option<&T> { // FIXME: consider nullability // FIXME: Become a dispatch to Iterator::nth - todo!() - // self.datum_at(index).map(|datum| unsafe { T::borrow_from(datum) }) + self.ptr_to(index).map(|ptr| unsafe { &*T::point_from(ptr.cast_mut()) }) } /// Mutably borrow the nth element. @@ -51,24 +52,22 @@ where pub fn nth_mut(&mut self, index: usize) -> Option<&mut T> { // FIXME: consider nullability // FIXME: Become a dispatch to Iterator::nth - todo!(); - // unsafe { self.datum_mut_at(index).map(|datum| unsafe { T::borrow_mut_from(datum) }) } + self.ptr_mut_to(index).map(|ptr| unsafe { &mut *T::point_from(ptr) }) } - fn datum_at(&self, index: usize) -> Option<&Datum<'mcx>> { + // Obtain a pointer with read-only permissions to the type at this index + #[inline] + fn ptr_to(&self, index: usize) -> Option<*const u8> { let data_ptr = unsafe { port::ARR_DATA_PTR(ptr::addr_of!(self.head).cast_mut()) }; - todo!(); + todo!() // FIXME: replace with actual impl instead of something that merely typechecks } - /// # Safety - /// This is an incredibly naughty function for arrays where *the scalar type is - /// smaller than Datum*. This is because when we return `&mut Datum`, that could overlap - /// with the next `&mut Datum`, so it must essentially always be converted before exposure. - /// Consider replacing this with raw pointers. - unsafe fn datum_mut_at(&mut self, index: usize) -> Option<&mut Datum<'mcx>> { + // Obtain a pointer with read-write permissions to the type at this index + #[inline] + fn ptr_mut_to(&mut self, index: usize) -> Option<*mut u8> { let data_ptr = unsafe { port::ARR_DATA_PTR(ptr::addr_of_mut!(self.head)) }; - todo!(); + todo!() // FIXME: replace with actual impl instead of something that merely typechecks } From 1b91a9ec9e337fc2764077652356652ceab1bf4c Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Tue, 19 Mar 2024 21:09:25 -0700 Subject: [PATCH 14/54] Make FlatArray::len public as FlatArray::count --- pgrx/src/array.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pgrx/src/array.rs b/pgrx/src/array.rs index 8039108960..d0573fd55e 100644 --- a/pgrx/src/array.rs +++ b/pgrx/src/array.rs @@ -74,14 +74,14 @@ where /// Number of elements in the Array /// /// Note that for many Arrays, this doesn't have a linear relationship with array byte-len. - fn len(&self) -> usize { + pub fn count(&self) -> usize { let ndims = self.head.ndim as usize; let dims_ptr = unsafe { port::ARR_DIMS(ptr::addr_of!(self.head).cast_mut()) }; unsafe { slice::from_raw_parts(dims_ptr, ndims).into_iter().sum::() as usize } } pub fn nulls(&self) -> Option<&[u8]> { - let len = self.len() + 7 >> 3; // Obtains 0 if len was 0. + let len = self.count() + 7 >> 3; // Obtains 0 if len was 0. // SAFETY: This obtains the nulls pointer from a function that must either // return a null pointer or a pointer to a valid null bitmap. @@ -103,7 +103,7 @@ where [ARR_NULLBITMAP]: */ pub unsafe fn nulls_mut(&mut self) -> Option<&mut [u8]> { - let len = self.len() + 7 >> 3; // Obtains 0 if len was 0. + let len = self.count() + 7 >> 3; // Obtains 0 if len was 0. // SAFETY: This obtains the nulls pointer from a function that must either // return a null pointer or a pointer to a valid null bitmap. From e84d3d215aac198ee9d16174e43f7f42a7e65da3 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Wed, 20 Mar 2024 12:47:57 -0700 Subject: [PATCH 15/54] Draft an ArrayIter --- pgrx/src/array.rs | 53 ++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 48 insertions(+), 5 deletions(-) diff --git a/pgrx/src/array.rs b/pgrx/src/array.rs index d0573fd55e..353c57a30a 100644 --- a/pgrx/src/array.rs +++ b/pgrx/src/array.rs @@ -10,6 +10,7 @@ #![allow(clippy::precedence)] #![allow(unused)] use crate::datum::{Array, BorrowDatum, Datum}; +use crate::nullable::Nullable; use crate::pgrx_sql_entity_graph::metadata::{ ArgumentError, Returns, ReturnsError, SqlMapping, SqlTranslatable, }; @@ -37,22 +38,31 @@ impl<'mcx, T> FlatArray<'mcx, T> where T: ?Sized + BorrowDatum, { + /// Iterate the array + pub fn iter(&self) -> ArrayIter<'mcx, T> { + /* ArrayIter { + + } */ + todo!() + } + /// Borrow the nth element. /// - /// `FlatArray::nth` may have to iterate the array, thus it is named for `Iterator::nth`. - pub fn nth(&self, index: usize) -> Option<&T> { + /// `FlatArray::nth` may have to iterate the array, thus it is named for `Iterator::nth`, + /// as opposed to a constant-time `get`. + pub fn nth(&self, index: usize) -> Option> { // FIXME: consider nullability // FIXME: Become a dispatch to Iterator::nth - self.ptr_to(index).map(|ptr| unsafe { &*T::point_from(ptr.cast_mut()) }) + todo!() } /// Mutably borrow the nth element. /// /// `FlatArray::nth_mut` may have to iterate the array, thus it is named for `Iterator::nth`. - pub fn nth_mut(&mut self, index: usize) -> Option<&mut T> { + pub fn nth_mut(&mut self, index: usize) -> Option> { // FIXME: consider nullability // FIXME: Become a dispatch to Iterator::nth - self.ptr_mut_to(index).map(|ptr| unsafe { &mut *T::point_from(ptr) }) + todo!() } // Obtain a pointer with read-only permissions to the type at this index @@ -124,6 +134,39 @@ unsafe impl BorrowDatum for FlatArray<'_, T> { } } +/// Iterator for arrays +#[derive(Clone)] +struct ArrayIter<'arr, T> +where + T: ?Sized + BorrowDatum, +{ + arr: &'arr FlatArray<'arr, T>, + data: *const u8, + nulls: *const u8, +} + +fn is_null(p: *const u8) -> bool { + todo!() +} + +impl<'arr, T> Iterator for ArrayIter<'arr, T> +where + T: BorrowDatum, +{ + type Item = Nullable<&'arr T>; + + fn next(&mut self) -> Option> { + if is_null(self.nulls) { + Some(Nullable::Null) + } else { + unsafe { + let ptr = ::point_from(self.data.cast_mut()); + Some(Nullable::Valid(&*ptr)) + } + } + } +} + /** An aligned, dereferenceable `NonNull` with low-level accessors. From e2ddff76f83400151faaa0ae99f9acff58f13d1d Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Fri, 12 Apr 2024 01:15:39 -0700 Subject: [PATCH 16/54] Pipe through RawArray in places --- pgrx/src/array.rs | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/pgrx/src/array.rs b/pgrx/src/array.rs index 353c57a30a..207bcc0a42 100644 --- a/pgrx/src/array.rs +++ b/pgrx/src/array.rs @@ -38,8 +38,21 @@ impl<'mcx, T> FlatArray<'mcx, T> where T: ?Sized + BorrowDatum, { + fn as_raw(&self) -> RawArray { + unsafe { + let ptr = NonNull::new_unchecked(ptr::from_ref(self).cast_mut()); + RawArray::from_ptr(ptr.cast()) + } + } + /// Iterate the array pub fn iter(&self) -> ArrayIter<'mcx, T> { + let nulls = self.nulls(); + let nelems = self.count(); + let raw = self.as_raw(); + + let data_ptr = raw.data_ptr(); + /* ArrayIter { } */ @@ -85,9 +98,7 @@ where /// /// Note that for many Arrays, this doesn't have a linear relationship with array byte-len. pub fn count(&self) -> usize { - let ndims = self.head.ndim as usize; - let dims_ptr = unsafe { port::ARR_DIMS(ptr::addr_of!(self.head).cast_mut()) }; - unsafe { slice::from_raw_parts(dims_ptr, ndims).into_iter().sum::() as usize } + self.as_raw().len() } pub fn nulls(&self) -> Option<&[u8]> { From 85ea44cc4325070d7b61d0a3fe22b7ad9aca8657 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Fri, 12 Apr 2024 01:46:08 -0700 Subject: [PATCH 17/54] Finish unpacking the parts of the array --- pgrx/src/array.rs | 35 +++++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/pgrx/src/array.rs b/pgrx/src/array.rs index 207bcc0a42..62f1ed7641 100644 --- a/pgrx/src/array.rs +++ b/pgrx/src/array.rs @@ -46,19 +46,27 @@ where } /// Iterate the array - pub fn iter(&self) -> ArrayIter<'mcx, T> { - let nulls = self.nulls(); + // this lifetime seems wrong + pub fn iter(&'mcx self) -> ArrayIter<'mcx, T> { + let nulls = self + .nulls() + .map(|p| unsafe { NonNull::new_unchecked(ptr::from_ref(p).cast_mut().cast()) }); let nelems = self.count(); let raw = self.as_raw(); - let data_ptr = raw.data_ptr(); + let data = raw.data_ptr(); + let arr = self; + let index = 0; + let offset = 0; - /* ArrayIter { - - } */ - todo!() + ArrayIter { data, nulls, nelems, arr, index, offset } } + /* + pub fn iter_mut(&mut self) -> ArrayIterMut<'mcx, T> { + ??? + } + */ /// Borrow the nth element. /// /// `FlatArray::nth` may have to iterate the array, thus it is named for `Iterator::nth`, @@ -153,11 +161,18 @@ where { arr: &'arr FlatArray<'arr, T>, data: *const u8, - nulls: *const u8, + nulls: Option>, + nelems: usize, + index: usize, + offset: usize, } -fn is_null(p: *const u8) -> bool { - todo!() +fn is_null(p: Option>) -> bool { + if let Some(p) = p { + todo!() + } else { + false + } } impl<'arr, T> Iterator for ArrayIter<'arr, T> From 684005ce3b0608c37bfdbd0672cc15af5c885c41 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Fri, 12 Apr 2024 07:46:21 -0700 Subject: [PATCH 18/54] So now that just works --- pgrx/src/array.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/pgrx/src/array.rs b/pgrx/src/array.rs index 62f1ed7641..e49da60192 100644 --- a/pgrx/src/array.rs +++ b/pgrx/src/array.rs @@ -72,9 +72,7 @@ where /// `FlatArray::nth` may have to iterate the array, thus it is named for `Iterator::nth`, /// as opposed to a constant-time `get`. pub fn nth(&self, index: usize) -> Option> { - // FIXME: consider nullability - // FIXME: Become a dispatch to Iterator::nth - todo!() + self.iter().nth(index) } /// Mutably borrow the nth element. @@ -177,7 +175,7 @@ fn is_null(p: Option>) -> bool { impl<'arr, T> Iterator for ArrayIter<'arr, T> where - T: BorrowDatum, + T: ?Sized + BorrowDatum, { type Item = Nullable<&'arr T>; From 6b3ec5886550103e5eb57e52294bfcf309f43411 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Fri, 12 Apr 2024 08:03:53 -0700 Subject: [PATCH 19/54] Remove now-irrelevant code --- pgrx/src/array.rs | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/pgrx/src/array.rs b/pgrx/src/array.rs index e49da60192..f982fc1d39 100644 --- a/pgrx/src/array.rs +++ b/pgrx/src/array.rs @@ -84,22 +84,6 @@ where todo!() } - // Obtain a pointer with read-only permissions to the type at this index - #[inline] - fn ptr_to(&self, index: usize) -> Option<*const u8> { - let data_ptr = unsafe { port::ARR_DATA_PTR(ptr::addr_of!(self.head).cast_mut()) }; - todo!() - // FIXME: replace with actual impl instead of something that merely typechecks - } - - // Obtain a pointer with read-write permissions to the type at this index - #[inline] - fn ptr_mut_to(&mut self, index: usize) -> Option<*mut u8> { - let data_ptr = unsafe { port::ARR_DATA_PTR(ptr::addr_of_mut!(self.head)) }; - todo!() - // FIXME: replace with actual impl instead of something that merely typechecks - } - /// Number of elements in the Array /// /// Note that for many Arrays, this doesn't have a linear relationship with array byte-len. From c0f0238511873d65b62a81b5615b7928573510eb Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Fri, 12 Apr 2024 08:07:17 -0700 Subject: [PATCH 20/54] mark incomplete code as incomplete --- pgrx/src/array.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pgrx/src/array.rs b/pgrx/src/array.rs index f982fc1d39..913f630970 100644 --- a/pgrx/src/array.rs +++ b/pgrx/src/array.rs @@ -75,6 +75,7 @@ where self.iter().nth(index) } + /* /// Mutably borrow the nth element. /// /// `FlatArray::nth_mut` may have to iterate the array, thus it is named for `Iterator::nth`. @@ -83,6 +84,7 @@ where // FIXME: Become a dispatch to Iterator::nth todo!() } + */ /// Number of elements in the Array /// @@ -164,7 +166,7 @@ where type Item = Nullable<&'arr T>; fn next(&mut self) -> Option> { - if is_null(self.nulls) { + if todo!() { Some(Nullable::Null) } else { unsafe { From 55b7543467cca13a2fa1b639e630507b6e46e867 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Fri, 12 Apr 2024 10:59:33 -0700 Subject: [PATCH 21/54] use bitvec for MVP again --- pgrx/src/array.rs | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/pgrx/src/array.rs b/pgrx/src/array.rs index 913f630970..3f6997b0da 100644 --- a/pgrx/src/array.rs +++ b/pgrx/src/array.rs @@ -16,8 +16,8 @@ use crate::pgrx_sql_entity_graph::metadata::{ }; use crate::toast::{Toast, Toasty}; use crate::{layout, pg_sys, varlena}; -use bitvec::ptr::{self as bitptr, BitPtr, BitPtrError, Mut}; -use bitvec::slice::BitSlice; +use bitvec::ptr::{self as bitptr, BitPtr, BitPtrError, Const, Mut}; +use bitvec::slice::{self as bitslice, BitSlice}; use core::marker::PhantomData; use core::ptr::{self, NonNull}; use core::{mem, slice}; @@ -48,11 +48,10 @@ where /// Iterate the array // this lifetime seems wrong pub fn iter(&'mcx self) -> ArrayIter<'mcx, T> { - let nulls = self - .nulls() - .map(|p| unsafe { NonNull::new_unchecked(ptr::from_ref(p).cast_mut().cast()) }); let nelems = self.count(); let raw = self.as_raw(); + let nulls = + raw.nulls_bitptr().map(|p| unsafe { bitslice::from_raw_parts(p, nelems).unwrap() }); let data = raw.data_ptr(); let arr = self; @@ -145,7 +144,7 @@ where { arr: &'arr FlatArray<'arr, T>, data: *const u8, - nulls: Option>, + nulls: Option<&'arr BitSlice>, nelems: usize, index: usize, offset: usize, @@ -406,11 +405,22 @@ impl RawArray { } #[inline] - fn nulls_bitptr(&mut self) -> Option> { + fn nulls_bitptr(&self) -> Option> { + let nulls_ptr = unsafe { port::ARR_NULLBITMAP(self.ptr.as_ptr()) }.cast_const(); + match BitPtr::try_from(nulls_ptr) { + Ok(ptr) => Some(ptr), + Err(BitPtrError::Null(_)) => None, + Err(BitPtrError::Misaligned(_)) => unreachable!(), + } + } + + #[inline] + fn nulls_mut_bitptr(&mut self) -> Option> { + let nulls_ptr = unsafe { port::ARR_NULLBITMAP(self.ptr.as_ptr()) }; match BitPtr::try_from(self.nulls_mut_ptr()) { Ok(ptr) => Some(ptr), Err(BitPtrError::Null(_)) => None, - Err(BitPtrError::Misaligned(_)) => unreachable!("impossible to misalign *mut u8"), + Err(BitPtrError::Misaligned(_)) => unreachable!(), } } @@ -447,7 +457,7 @@ impl RawArray { [ARR_NULLBITMAP]: */ pub fn nulls_bitslice(&mut self) -> Option>> { - NonNull::new(bitptr::bitslice_from_raw_parts_mut(self.nulls_bitptr()?, self.len())) + NonNull::new(bitptr::bitslice_from_raw_parts_mut(self.nulls_mut_bitptr()?, self.len())) } /** From 24c764848bcc9d3f583b914482ad69320a6fc2f5 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Fri, 12 Apr 2024 14:46:18 -0700 Subject: [PATCH 22/54] comment on iter_mut design problems --- pgrx/src/array.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pgrx/src/array.rs b/pgrx/src/array.rs index 3f6997b0da..9ea44ab784 100644 --- a/pgrx/src/array.rs +++ b/pgrx/src/array.rs @@ -62,6 +62,13 @@ where } /* + /** + Some problems with the design of an iter_mut for FlatArray: + In order to traverse the array, we need to assume well-formedness of e.g. cstring/varlena elements, + but &mut would allow safely updating varlenas within their span, e.g. injecting \0 into cstrings. + making it so that nothing allows making an ill-formed varlena via &mut seems untenable, also? + probably only viable to expose &mut for fixed-size types, then + */ pub fn iter_mut(&mut self) -> ArrayIterMut<'mcx, T> { ??? } From f98cc2c353e8703813a1bdc9cd1ca0184ffb85e9 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Fri, 12 Apr 2024 15:04:40 -0700 Subject: [PATCH 23/54] Split FlatArray into unconditional and conditional impls --- pgrx/src/array.rs | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/pgrx/src/array.rs b/pgrx/src/array.rs index 9ea44ab784..28fb6de6f4 100644 --- a/pgrx/src/array.rs +++ b/pgrx/src/array.rs @@ -36,7 +36,7 @@ pub struct FlatArray<'mcx, T: ?Sized> { impl<'mcx, T> FlatArray<'mcx, T> where - T: ?Sized + BorrowDatum, + T: ?Sized, { fn as_raw(&self) -> RawArray { unsafe { @@ -45,6 +45,18 @@ where } } + /// Number of elements in the Array + /// + /// Note that for many Arrays, this doesn't have a linear relationship with array byte-len. + pub fn count(&self) -> usize { + self.as_raw().len() + } +} + +impl<'mcx, T> FlatArray<'mcx, T> +where + T: ?Sized + BorrowDatum, +{ /// Iterate the array // this lifetime seems wrong pub fn iter(&'mcx self) -> ArrayIter<'mcx, T> { @@ -92,13 +104,6 @@ where } */ - /// Number of elements in the Array - /// - /// Note that for many Arrays, this doesn't have a linear relationship with array byte-len. - pub fn count(&self) -> usize { - self.as_raw().len() - } - pub fn nulls(&self) -> Option<&[u8]> { let len = self.count() + 7 >> 3; // Obtains 0 if len was 0. @@ -133,7 +138,7 @@ where } } -unsafe impl BorrowDatum for FlatArray<'_, T> { +unsafe impl BorrowDatum for FlatArray<'_, T> { const PASS: Option = Some(layout::PassBy::Ref); unsafe fn point_from(ptr: *mut u8) -> *mut Self { unsafe { From 848f2bcaea0cd9b5694a6eab3a22e9e6548f8110 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Wed, 11 Sep 2024 08:13:35 -0700 Subject: [PATCH 24/54] fix doc link maybe --- pgrx/src/array.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pgrx/src/array.rs b/pgrx/src/array.rs index 28fb6de6f4..d4e8e383a0 100644 --- a/pgrx/src/array.rs +++ b/pgrx/src/array.rs @@ -116,7 +116,8 @@ where } /** - Oxidized form of [ARR_NULLBITMAP(ArrayType*)][ARR_NULLBITMAP] + Oxidized form of [ARR_NULLBITMAP(ArrayType*)][arr_nullbitmap] + If this returns None, the array *cannot* have nulls. Note that unlike the `is_null: bool` that appears elsewhere, 1 is "valid" and 0 is "null". @@ -124,6 +125,7 @@ where Trailing bits must be set to 0, and all elements marked with 1 must be initialized. The null bitmap is linear but the layout of elements may be nonlinear, so for some arrays these cannot be calculated directly from each other. + [ARR_NULLBITMAP]: */ pub unsafe fn nulls_mut(&mut self) -> Option<&mut [u8]> { From 7cea9b725af1d52095ef5e8e94099d42547c3f2e Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 12 Sep 2024 23:57:08 -0700 Subject: [PATCH 25/54] Make PassBy less optional --- pgrx/src/array.rs | 2 +- pgrx/src/text.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pgrx/src/array.rs b/pgrx/src/array.rs index d4e8e383a0..388a52fc83 100644 --- a/pgrx/src/array.rs +++ b/pgrx/src/array.rs @@ -141,7 +141,7 @@ where } unsafe impl BorrowDatum for FlatArray<'_, T> { - const PASS: Option = Some(layout::PassBy::Ref); + const PASS: layout::PassBy = layout::PassBy::Ref; unsafe fn point_from(ptr: *mut u8) -> *mut Self { unsafe { let len = varlena::varsize_any(ptr.cast()) - mem::size_of::(); diff --git a/pgrx/src/text.rs b/pgrx/src/text.rs index 38b6b6a212..99ae9e3734 100644 --- a/pgrx/src/text.rs +++ b/pgrx/src/text.rs @@ -36,7 +36,7 @@ impl DerefMut for Text { } unsafe impl BorrowDatum for Text { - const PASS: Option = Some(PassBy::Ref); + const PASS: PassBy = PassBy::Ref; unsafe fn point_from(ptr: *mut u8) -> *mut Self { unsafe { let len = varlena::varsize_any(ptr.cast()); From 3fa0675b86cb13c3e0b4dd0a05df7d696f3253eb Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 26 Sep 2024 14:03:22 -0700 Subject: [PATCH 26/54] Fixup text docs --- pgrx/src/text.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pgrx/src/text.rs b/pgrx/src/text.rs index 99ae9e3734..01c168ddf0 100644 --- a/pgrx/src/text.rs +++ b/pgrx/src/text.rs @@ -7,9 +7,10 @@ use crate::{pg_sys, varlena}; use core::ops::{Deref, DerefMut}; use core::ptr; -/// A more strongly-typed representation of a Postgres string, AKA `TEXT`. -/// A pointer to this points to a byte array, which includes a variable-length header -/// and an unsized data field which is... often but not always UTF-8. +/// A Postgres string, AKA `TEXT`. +/// +/// This is a varlena: a reference to a variable-length header followed by a slice of bytes. +/// Usually this will be UTF-8, but this is not always strictly enforced by PostgreSQL. #[repr(transparent)] pub struct Text([u8]); From fac7f1ef2caca8e42d24ea634d4f2d155cea62a4 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 26 Sep 2024 15:18:31 -0700 Subject: [PATCH 27/54] Address the TODO for Text --- Cargo.lock | 12 +++++++++ pgrx/Cargo.toml | 1 + pgrx/src/text.rs | 63 ++++++++++++++++++++++++++++++++++++++---------- 3 files changed, 63 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8828937c85..228fc69a31 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -234,6 +234,17 @@ dependencies = [ "generic-array", ] +[[package]] +name = "bstr" +version = "1.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "40723b8fb387abc38f4f4a37c09073622e41dd12327033091ef8950659e6dc0c" +dependencies = [ + "memchr", + "regex-automata 0.4.5", + "serde", +] + [[package]] name = "bumpalo" version = "3.14.0" @@ -1492,6 +1503,7 @@ dependencies = [ "atomic-traits", "bitflags 2.4.2", "bitvec", + "bstr", "enum-map", "heapless", "libc", diff --git a/pgrx/Cargo.toml b/pgrx/Cargo.toml index bcdf8b67f2..43b5e8f610 100644 --- a/pgrx/Cargo.toml +++ b/pgrx/Cargo.toml @@ -61,6 +61,7 @@ enum-map = "2.6.3" atomic-traits = "0.3.0" # PgAtomic and shmem init bitflags = "2.4.0" # BackgroundWorker bitvec = "1.0" # processing array nullbitmaps +bstr = "1.10" heapless = "0.8" # shmem and PgLwLock libc.workspace = true # FFI type compat seahash = "4.1.0" # derive(PostgresHash) diff --git a/pgrx/src/text.rs b/pgrx/src/text.rs index 01c168ddf0..a8240a1713 100644 --- a/pgrx/src/text.rs +++ b/pgrx/src/text.rs @@ -1,11 +1,18 @@ -use crate::datum::{BorrowDatum, Datum}; +use crate::datum::BorrowDatum; use crate::layout::PassBy; use crate::pgrx_sql_entity_graph::metadata::{ ArgumentError, Returns, ReturnsError, SqlMapping, SqlTranslatable, }; use crate::{pg_sys, varlena}; -use core::ops::{Deref, DerefMut}; -use core::ptr; +use core::borrow::Borrow; +use core::{ptr, slice, str}; + +use bstr::{BStr, ByteSlice}; + +// We reexport these types so people don't have to care whether they're pulled from BStr or std, +// they just use the ones from pgrx::text::* +pub use bstr::{Bytes, Chars}; +pub use core::str::{Utf8Chunks, Utf8Error}; /// A Postgres string, AKA `TEXT`. /// @@ -14,26 +21,56 @@ use core::ptr; #[repr(transparent)] pub struct Text([u8]); -// TODO(0.12.0): strip this and make Text forward its impl to BStr fn instead -impl Deref for Text { - type Target = str; - fn deref(&self) -> &str { +impl Text { + pub fn as_bytes(&self) -> &[u8] { let self_ptr = self as *const Text as *const pg_sys::varlena; - unsafe { varlena::text_to_rust_str(self_ptr).unwrap() } + unsafe { + let len = varlena::varsize_any_exhdr(self_ptr); + let data = varlena::vardata_any(self_ptr); + + slice::from_raw_parts(data.cast::(), len) + } } -} -// TODO(0.12.0): strip this and make Text forward its impl to BStr fn instead -impl DerefMut for Text { - fn deref_mut(&mut self) -> &mut str { + /// Manipulate Text as its byte repr + /// + /// # Safety + /// Like [`str::as_bytes_mut`], this can cause problems if you change Text in a way that + /// your database is not specified to support, so the caller must assure that it remains in + /// a valid encoding for the database. + pub unsafe fn as_bytes_mut(&mut self) -> &mut [u8] { let self_ptr = self as *mut Text as *mut pg_sys::varlena; unsafe { let len = varlena::varsize_any_exhdr(self_ptr); let data = varlena::vardata_any(self_ptr); - &mut *(ptr::slice_from_raw_parts_mut(data as *mut u8, len) as *mut str) + slice::from_raw_parts_mut(data.cast::().cast_mut(), len) } } + + /// Reborrow `&Text as `&BStr` + /// + /// We do not implement Deref to BStr or [u8] because we'd like to expose a more selective API. + /// Several fn that [u8] implements are implemented very differently on str! + fn as_bstr(&self) -> &BStr { + self.as_bytes().borrow() + } + + pub fn chars(&self) -> Chars<'_> { + self.as_bstr().chars() + } + + pub fn bytes(&self) -> Bytes<'_> { + self.as_bstr().bytes() + } + + pub fn to_str(&self) -> Result<&str, Utf8Error> { + str::from_utf8(self.as_bytes()) + } + + pub fn utf8_chunks(&self) -> Utf8Chunks { + self.as_bytes().utf8_chunks() + } } unsafe impl BorrowDatum for Text { From 14c47acd76d183361861883e5dd56b9d86ea7119 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 26 Sep 2024 19:41:00 -0700 Subject: [PATCH 28/54] Elaborate on the Text API --- Cargo.lock | 2 -- pgrx/Cargo.toml | 2 +- pgrx/src/text.rs | 35 +++++++++++++++++++++++++++++++++-- 3 files changed, 34 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 228fc69a31..ae68977930 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -241,8 +241,6 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "40723b8fb387abc38f4f4a37c09073622e41dd12327033091ef8950659e6dc0c" dependencies = [ "memchr", - "regex-automata 0.4.5", - "serde", ] [[package]] diff --git a/pgrx/Cargo.toml b/pgrx/Cargo.toml index 43b5e8f610..9382f2e2c2 100644 --- a/pgrx/Cargo.toml +++ b/pgrx/Cargo.toml @@ -61,7 +61,7 @@ enum-map = "2.6.3" atomic-traits = "0.3.0" # PgAtomic and shmem init bitflags = "2.4.0" # BackgroundWorker bitvec = "1.0" # processing array nullbitmaps -bstr = "1.10" +bstr = { version = "1.10", default-features = false} heapless = "0.8" # shmem and PgLwLock libc.workspace = true # FFI type compat seahash = "4.1.0" # derive(PostgresHash) diff --git a/pgrx/src/text.rs b/pgrx/src/text.rs index a8240a1713..715f6858d9 100644 --- a/pgrx/src/text.rs +++ b/pgrx/src/text.rs @@ -21,7 +21,11 @@ pub use core::str::{Utf8Chunks, Utf8Error}; #[repr(transparent)] pub struct Text([u8]); +// API decision: we could deref to TextData and move some fn to TextData so it can be returned from +// `split_at`, `trim`, etc., and thus preserve conveniences that [u8] doesn't have? + impl Text { + /// Obtain a reference to the Text's data as bytes pub fn as_bytes(&self) -> &[u8] { let self_ptr = self as *const Text as *const pg_sys::varlena; unsafe { @@ -32,7 +36,7 @@ impl Text { } } - /// Manipulate Text as its byte repr + /// Obtain a mutable reference the Text's data as bytes /// /// # Safety /// Like [`str::as_bytes_mut`], this can cause problems if you change Text in a way that @@ -51,23 +55,50 @@ impl Text { /// Reborrow `&Text as `&BStr` /// /// We do not implement Deref to BStr or [u8] because we'd like to expose a more selective API. - /// Several fn that [u8] implements are implemented very differently on str! + /// Several fn that [u8] implements are implemented very differently on str, and we would like + /// the API of Text to "feel like" that of str in most cases. fn as_bstr(&self) -> &BStr { self.as_bytes().borrow() } + /// Iterate over the UTF-8 characters of this Text + /// + /// If the data is not UTF-8, the replacement character � is returned. pub fn chars(&self) -> Chars<'_> { self.as_bstr().chars() } + /// Iterate over the Text's data as bytes pub fn bytes(&self) -> Bytes<'_> { self.as_bstr().bytes() } + /// Is the data ASCII? + pub fn is_ascii(&self) -> bool { + self.as_bytes().is_ascii() + } + + /// Is the varlena larger than its header? + pub fn is_empty(&self) -> bool { + self.as_bytes().is_empty() + } + + /// Length of the data in bytes + pub fn len_data(&self) -> usize { + self.as_bytes().len() + } + + /// Length of the entire varlena in bytes + pub fn len_full(&self) -> usize { + self.0.len() + } + + /// Obtain a reference to the varlena data if it is a UTF-8 str pub fn to_str(&self) -> Result<&str, Utf8Error> { str::from_utf8(self.as_bytes()) } + /// Iterate over the UTF-8 chunks of the Text's data pub fn utf8_chunks(&self) -> Utf8Chunks { self.as_bytes().utf8_chunks() } From df9d60d751cd7ec4f394c58165d1a06c06367258 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 26 Sep 2024 20:04:09 -0700 Subject: [PATCH 29/54] Have a cow --- pgrx/src/text.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pgrx/src/text.rs b/pgrx/src/text.rs index 715f6858d9..7b6c505763 100644 --- a/pgrx/src/text.rs +++ b/pgrx/src/text.rs @@ -4,6 +4,8 @@ use crate::pgrx_sql_entity_graph::metadata::{ ArgumentError, Returns, ReturnsError, SqlMapping, SqlTranslatable, }; use crate::{pg_sys, varlena}; +use alloc::borrow::Cow; +use alloc::string::String; use core::borrow::Borrow; use core::{ptr, slice, str}; @@ -98,6 +100,13 @@ impl Text { str::from_utf8(self.as_bytes()) } + /// You have two cows. Both are UTF-8 data. + /// + /// One is completely UTF-8, but the other is allocated and non-UTF-8 is patched over with �. + pub fn to_str_lossy(&self) -> Cow<'_, str> { + String::from_utf8_lossy(self.as_bytes()) + } + /// Iterate over the UTF-8 chunks of the Text's data pub fn utf8_chunks(&self) -> Utf8Chunks { self.as_bytes().utf8_chunks() From 3606a9a0a600cdbf537b538c1206e75ef091ec0f Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 26 Sep 2024 20:06:51 -0700 Subject: [PATCH 30/54] obtain more confidence --- pgrx/src/array.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pgrx/src/array.rs b/pgrx/src/array.rs index 388a52fc83..0852a0b575 100644 --- a/pgrx/src/array.rs +++ b/pgrx/src/array.rs @@ -58,8 +58,7 @@ where T: ?Sized + BorrowDatum, { /// Iterate the array - // this lifetime seems wrong - pub fn iter(&'mcx self) -> ArrayIter<'mcx, T> { + pub fn iter(&self) -> ArrayIter<'_, T> { let nelems = self.count(); let raw = self.as_raw(); let nulls = From ef7ac23ee626c79d20da59a13adab2b846694ed5 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 26 Sep 2024 20:24:10 -0700 Subject: [PATCH 31/54] Round out FlatArray API --- pgrx/src/array.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pgrx/src/array.rs b/pgrx/src/array.rs index 0852a0b575..1c60e6a908 100644 --- a/pgrx/src/array.rs +++ b/pgrx/src/array.rs @@ -72,6 +72,10 @@ where ArrayIter { data, nulls, nelems, arr, index, offset } } + pub fn iter_non_null(&self) -> impl Iterator { + self.iter().filter_map(|elem| elem.into_option()) + } + /* /** Some problems with the design of an iter_mut for FlatArray: From 151af4222c0492e7d3d33eeb937ad0895af5c3cc Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 26 Sep 2024 20:26:22 -0700 Subject: [PATCH 32/54] SqlTranslatable for &FlatArray --- pgrx/src/array.rs | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/pgrx/src/array.rs b/pgrx/src/array.rs index 1c60e6a908..87da552efe 100644 --- a/pgrx/src/array.rs +++ b/pgrx/src/array.rs @@ -153,6 +153,33 @@ unsafe impl BorrowDatum for FlatArray<'_, T> { } } +unsafe impl SqlTranslatable for &FlatArray<'_, T> +where + T: SqlTranslatable, +{ + fn argument_sql() -> Result { + match T::argument_sql()? { + SqlMapping::As(sql) => Ok(SqlMapping::As(format!("{sql}[]"))), + SqlMapping::Skip => Err(ArgumentError::SkipInArray), + SqlMapping::Composite { .. } => Ok(SqlMapping::Composite { array_brackets: true }), + } + } + + fn return_sql() -> Result { + match T::return_sql()? { + Returns::One(SqlMapping::As(sql)) => { + Ok(Returns::One(SqlMapping::As(format!("{sql}[]")))) + } + Returns::One(SqlMapping::Composite { array_brackets: _ }) => { + Ok(Returns::One(SqlMapping::Composite { array_brackets: true })) + } + Returns::One(SqlMapping::Skip) => Err(ReturnsError::SkipInArray), + Returns::SetOf(_) => Err(ReturnsError::SetOfInArray), + Returns::Table(_) => Err(ReturnsError::TableInArray), + } + } +} + /// Iterator for arrays #[derive(Clone)] struct ArrayIter<'arr, T> From 89a1f2763450d0351854d4a641bffd727561ee07 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 26 Sep 2024 20:53:53 -0700 Subject: [PATCH 33/54] try to add some tests --- pgrx-tests/src/tests/array_borrowed.rs | 523 +++++++++++++++++++++++++ pgrx-tests/src/tests/mod.rs | 1 + 2 files changed, 524 insertions(+) create mode 100644 pgrx-tests/src/tests/array_borrowed.rs diff --git a/pgrx-tests/src/tests/array_borrowed.rs b/pgrx-tests/src/tests/array_borrowed.rs new file mode 100644 index 0000000000..3c70020a36 --- /dev/null +++ b/pgrx-tests/src/tests/array_borrowed.rs @@ -0,0 +1,523 @@ +//LICENSE Portions Copyright 2019-2021 ZomboDB, LLC. +//LICENSE +//LICENSE Portions Copyright 2021-2023 Technology Concepts & Design, Inc. +//LICENSE +//LICENSE Portions Copyright 2023-2023 PgCentral Foundation, Inc. +//LICENSE +//LICENSE All rights reserved. +//LICENSE +//LICENSE Use of this source code is governed by the MIT license that can be found in the LICENSE file. +use pgrx::array::{FlatArray, RawArray}; +use pgrx::prelude::*; +use pgrx::Json; +use pgrx::PostgresEnum; +use serde::Serialize; +use serde_json::json; + +#[pg_extern(name = "sum_array")] +fn borrow_sum_array_i32(values: &FlatArray<'_, i32>) -> i32 { + // we implement it this way so we can trap an overflow (as we have a test for this) and + // catch it correctly in both --debug and --release modes + let mut sum = 0_i32; + for v in values { + let v = v.unwrap_or(0); + let (val, overflow) = sum.overflowing_add(v); + if overflow { + panic!("attempt to add with overflow"); + } else { + sum = val; + } + } + sum +} + +#[pg_extern(name = "sum_array")] +fn borrow_sum_array_i64(values: &FlatArray<'_, i64>) -> i64 { + values.iter().map(|v| v.unwrap_or(0i64)).sum() +} + +#[pg_extern] +fn borrow_count_true(values: &FlatArray<'_, bool>) -> i32 { + values.iter().filter(|b| b.unwrap_or(false)).count() as i32 +} + +#[pg_extern] +fn borrow_count_nulls(values: &FlatArray<'_, i32>) -> i32 { + values.iter().map(|v| v.is_none()).filter(|v| *v).count() as i32 +} + +#[pg_extern] +fn borrow_optional_array_arg(values: Option<&FlatArray<'_, f32>>) -> f32 { + values.unwrap().iter().map(|v| v.unwrap_or(0f32)).sum() +} + +#[pg_extern] +fn borrow_iterate_array_with_deny_null(values: &FlatArray<'_, i32>) { + for _ in values.iter_deny_null() { + // noop + } +} + +#[pg_extern] +fn borrow_optional_array_with_default( + values: default!(Option<&FlatArray<'_, i32>>, "NULL"), +) -> i32 { + values.unwrap().iter().map(|v| v.unwrap_or(0)).sum() +} + +// TODO: fix this test by fixing serde impls for `FlatArray<'a, &'a str> -> Json` +// #[pg_extern] +// fn borrow_serde_serialize_array<'dat>(values: &FlatArray<'dat, &'dat str>) -> Json { +// Json(json! { { "values": values } }) +// } + +#[pg_extern] +fn borrow_serde_serialize_array_i32(values: &FlatArray<'_, i32>) -> Json { + Json(json! { { "values": values } }) +} + +#[pg_extern] +fn borrow_serde_serialize_array_i32_deny_null(values: &FlatArray<'_, i32>) -> Json { + Json(json! { { "values": values.iter_deny_null() } }) +} + +#[pg_extern] +fn borrow_return_text_array() -> Vec<&'static str> { + vec!["a", "b", "c", "d"] +} + +#[pg_extern] +fn borrow_return_zero_length_vec() -> Vec { + Vec::new() +} + +#[pg_extern] +fn borrow_get_arr_nelems(arr: &FlatArray<'_, i32>) -> libc::c_int { + // SAFETY: Eh it's fine, it's just a len check. + unsafe { RawFlatArray::from_array(arr) }.unwrap().len() as _ +} + +#[pg_extern] +fn borrow_get_arr_data_ptr_nth_elem(arr: &FlatArray<'_, i32>, elem: i32) -> Option { + // SAFETY: this is Known to be an FlatArray from FlatArrayType, + // and it's valid-ish to see any bitpattern of an i32 inbounds of a slice. + unsafe { + let raw = RawFlatArray::from_array(arr).unwrap().data::(); + let slice = &(*raw.as_ptr()); + slice.get(elem as usize).copied() + } +} + +#[pg_extern] +fn borrow_display_get_arr_nullbitmap(arr: &FlatArray<'_, i32>) -> String { + let mut raw = unsafe { RawFlatArray::from_array(arr) }.unwrap(); + + if let Some(slice) = raw.nulls() { + // SAFETY: If the test has gotten this far, the ptr is good for 0+ bytes, + // so reborrow NonNull<[u8]> as &[u8] for the hot second we're looking at it. + let slice = unsafe { &*slice.as_ptr() }; + // might panic if the array is len 0 + format!("{:#010b}", slice[0]) + } else { + String::from("") + } +} + +#[pg_extern] +fn borrow_get_arr_ndim(arr: &FlatArray<'_, i32>) -> libc::c_int { + // SAFETY: This is a valid FlatArrayType and it's just a field access. + unsafe { RawFlatArray::from_array(arr) }.unwrap().dims().len() as _ +} + +// This deliberately iterates the FlatArray. +// Because FlatArray::iter currently iterates the FlatArray as Datums, this is guaranteed to be "bug-free" regarding size. +#[pg_extern] +fn borrow_arr_mapped_vec(arr: &FlatArray<'_, i32>) -> Vec { + arr.iter().filter_map(|x| x).collect() +} + +/// Naive conversion. +#[pg_extern] +#[allow(deprecated)] +fn borrow_arr_into_vec(arr: &FlatArray<'_, i32>) -> Vec { + arr.iter_deny_null().collect() +} + +#[pg_extern] +#[allow(deprecated)] +fn borrow_arr_sort_uniq(arr: &FlatArray<'_, i32>) -> Vec { + let mut v: Vec = arr.iter_deny_null().collect(); + v.sort(); + v.dedup(); + v +} + +#[derive(Debug, Eq, PartialEq, PostgresEnum, Serialize)] +pub enum BorrowFlatArrayTestEnum { + One, + Two, + Three, +} + +#[pg_extern] +fn borrow_enum_array_roundtrip( + a: &FlatArray<'_, BorrowFlatArrayTestEnum>, +) -> Vec> { + a.into_iter().collect() +} + +#[pg_extern] +fn borrow_validate_cstring_array<'a>( + a: &FlatArray<'a, &'a core::ffi::CStr>, +) -> std::result::Result> { + assert_eq!( + 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")?), + None, + Some(core::ffi::CStr::from_bytes_with_nul(b"four\0")?), + Some(core::ffi::CStr::from_bytes_with_nul(b"five\0")?), + None, + Some(core::ffi::CStr::from_bytes_with_nul(b"seven\0")?), + None, + None + ] + ); + Ok(true) +} + +#[cfg(any(test, feature = "pg_test"))] +#[pgrx::pg_schema] +mod tests { + #[allow(unused_imports)] + use crate as pgrx_tests; + + use super::*; + use pgrx::prelude::*; + use pgrx::{IntoDatum, Json}; + use serde_json::json; + + #[pg_test] + fn borrow_test_enum_array_roundtrip() -> spi::Result<()> { + let a = Spi::get_one::>>( + "SELECT borrow_enum_array_roundtrip(ARRAY['One', 'Two']::BorrowFlatArrayTestEnum[])", + )? + .expect("SPI result was null"); + assert_eq!(a, vec![Some(BorrowFlatArrayTestEnum::One), Some(BorrowFlatArrayTestEnum::Two)]); + Ok(()) + } + + #[pg_test] + fn borrow_test_sum_array_i32() { + let sum = Spi::get_one::("SELECT borrow_sum_array(ARRAY[1,2,3]::integer[])"); + assert_eq!(sum, Ok(Some(6))); + } + + #[pg_test] + fn borrow_test_sum_array_i64() { + let sum = Spi::get_one::("SELECT borrow_sum_array(ARRAY[1,2,3]::bigint[])"); + assert_eq!(sum, Ok(Some(6))); + } + + #[pg_test(expected = "attempt to add with overflow")] + fn borrow_test_sum_array_i32_overflow() -> Result, pgrx::spi::Error> { + Spi::get_one::( + "SELECT borrow_sum_array(a) FROM (SELECT borrow_array_agg(s) a FROM generate_series(1, 1000000) s) x;", + ) + } + + #[pg_test] + fn borrow_test_count_true() { + let cnt = Spi::get_one::("SELECT borrow_count_true(ARRAY[true, true, false, true])"); + assert_eq!(cnt, Ok(Some(3))); + } + + #[pg_test] + fn borrow_test_count_nulls() { + let cnt = + Spi::get_one::("SELECT borrow_count_nulls(ARRAY[NULL, 1, 2, NULL]::integer[])"); + assert_eq!(cnt, Ok(Some(2))); + } + + #[pg_test] + fn borrow_test_optional_array() { + let sum = Spi::get_one::("SELECT borrow_optional_array_arg(ARRAY[1,2,3]::real[])"); + assert_eq!(sum, Ok(Some(6f32))); + } + + #[pg_test(expected = "array contains NULL")] + fn borrow_test_array_deny_nulls() -> Result<(), spi::Error> { + Spi::run("SELECT borrow_iterate_array_with_deny_null(ARRAY[1,2,3, NULL]::int[])") + } + + // TODO: fix this test by redesigning SPI. + // #[pg_test] + // fn borrow_test_serde_serialize_array() -> Result<(), pgrx::spi::Error> { + // let json = Spi::get_one::( + // "SELECT borrow_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 borrow_test_optional_array_with_default() { + let sum = Spi::get_one::("SELECT borrow_optional_array_with_default(ARRAY[1,2,3])"); + assert_eq!(sum, Ok(Some(6))); + } + + #[pg_test] + fn borrow_test_serde_serialize_array_i32() -> Result<(), pgrx::spi::Error> { + let json = Spi::get_one::( + "SELECT borrow_serde_serialize_array_i32(ARRAY[1, null, 2, 3, null, 4, 5])", + )? + .expect("returned json was null"); + assert_eq!(json.0, json! {{"values": [1,null,2,3,null,4, 5]}}); + Ok(()) + } + + #[pg_test(expected = "array contains NULL")] + fn borrow_test_serde_serialize_array_i32_deny_null() -> Result, pgrx::spi::Error> { + Spi::get_one::( + "SELECT borrow_serde_serialize_array_i32_deny_null(ARRAY[1, 2, 3, null, 4, 5])", + ) + } + + #[pg_test] + fn borrow_test_return_text_array() { + let rc = Spi::get_one::("SELECT ARRAY['a', 'b', 'c', 'd'] = return_text_array();"); + assert_eq!(rc, Ok(Some(true))); + } + + #[pg_test] + fn borrow_test_return_zero_length_vec() { + let rc = Spi::get_one::("SELECT ARRAY[]::integer[] = return_zero_length_vec();"); + assert_eq!(rc, Ok(Some(true))); + } + + #[pg_test] + fn borrow_test_slice_to_array() -> Result<(), pgrx::spi::Error> { + let owned_vec = vec![Some(1), None, Some(2), Some(3), None, Some(4), Some(5)]; + let json = Spi::connect(|client| { + client + .select( + "SELECT borrow_serde_serialize_array_i32($1)", + None, + Some(vec![( + PgBuiltInOids::INT4ARRAYOID.oid(), + owned_vec.as_slice().into_datum(), + )]), + )? + .first() + .get_one::() + })? + .expect("Failed to return json even though it's right there ^^"); + assert_eq!(json.0, json! {{"values": [1, null, 2, 3, null, 4, 5]}}); + Ok(()) + } + + #[pg_test] + fn borrow_test_arr_data_ptr() { + let len = Spi::get_one::("SELECT borrow_get_arr_nelems('{1,2,3,4,5}'::int[])"); + assert_eq!(len, Ok(Some(5))); + } + + #[pg_test] + fn borrow_test_get_arr_data_ptr_nth_elem() { + let nth = + Spi::get_one::("SELECT borrow_get_arr_data_ptr_nth_elem('{1,2,3,4,5}'::int[], 2)"); + assert_eq!(nth, Ok(Some(3))); + } + + #[pg_test] + fn borrow_test_display_get_arr_nullbitmap() -> Result<(), pgrx::spi::Error> { + let bitmap_str = Spi::get_one::( + "SELECT borrow_display_get_arr_nullbitmap(ARRAY[1,NULL,3,NULL,5]::int[])", + )? + .expect("datum was null"); + + assert_eq!(bitmap_str, "0b00010101"); + + let bitmap_str = Spi::get_one::( + "SELECT borrow_display_get_arr_nullbitmap(ARRAY[1,2,3,4,5]::int[])", + )? + .expect("datum was null"); + + assert_eq!(bitmap_str, ""); + Ok(()) + } + + #[pg_test] + fn borrow_test_get_arr_ndim() -> Result<(), pgrx::spi::Error> { + let ndim = Spi::get_one::("SELECT borrow_get_arr_ndim(ARRAY[1,2,3,4,5]::int[])")? + .expect("datum was null"); + + assert_eq!(ndim, 1); + + let ndim = Spi::get_one::("SELECT borrow_get_arr_ndim('{{1,2,3},{4,5,6}}'::int[])")? + .expect("datum was null"); + + assert_eq!(ndim, 2); + Ok(()) + } + + #[pg_test] + fn borrow_test_arr_to_vec() { + let result = + Spi::get_one::>("SELECT borrow_arr_mapped_vec(ARRAY[3,2,2,1]::integer[])"); + let other = + Spi::get_one::>("SELECT borrow_arr_into_vec(ARRAY[3,2,2,1]::integer[])"); + // One should be equivalent to the canonical form. + assert_eq!(result, Ok(Some(vec![3, 2, 2, 1]))); + // And they should be equal to each other. + assert_eq!(result, other); + } + + #[pg_test] + fn borrow_test_arr_sort_uniq() { + let result = + Spi::get_one::>("SELECT borrow_arr_sort_uniq(ARRAY[3,2,2,1]::integer[])"); + assert_eq!(result, Ok(Some(vec![1, 2, 3]))); + } + + #[pg_test] + #[should_panic] + fn borrow_test_arr_sort_uniq_with_null() -> Result<(), pgrx::spi::Error> { + Spi::get_one::>("SELECT borrow_arr_sort_uniq(ARRAY[3,2,NULL,2,1]::integer[])") + .map(|_| ()) + } + + #[pg_test] + fn borrow_test_cstring_array() -> Result<(), pgrx::spi::Error> { + let strings = Spi::get_one::("SELECT borrow_validate_cstring_array(ARRAY['one', 'two', NULL, 'four', 'five', NULL, 'seven', NULL, NULL]::cstring[])")?.expect("datum was NULL"); + assert_eq!(strings, true); + Ok(()) + } + + #[pg_test] + fn borrow_test_f64_slice() -> Result<(), Box> { + let array = Spi::get_one::>("SELECT ARRAY[1.0, 2.0, 3.0]::float8[]")? + .expect("datum was null"); + assert_eq!(array.as_slice()?, &[1.0, 2.0, 3.0]); + Ok(()) + } + + #[pg_test] + fn borrow_test_f32_slice() -> Result<(), Box> { + let array = Spi::get_one::>("SELECT ARRAY[1.0, 2.0, 3.0]::float4[]")? + .expect("datum was null"); + assert_eq!(array.as_slice()?, &[1.0, 2.0, 3.0]); + Ok(()) + } + + #[pg_test] + fn borrow_test_i64_slice() -> Result<(), Box> { + let array = Spi::get_one::>("SELECT ARRAY[1, 2, 3]::bigint[]")? + .expect("datum was null"); + assert_eq!(array.as_slice()?, &[1, 2, 3]); + Ok(()) + } + + #[pg_test] + fn borrow_test_i32_slice() -> Result<(), Box> { + let array = Spi::get_one::>("SELECT ARRAY[1, 2, 3]::integer[]")? + .expect("datum was null"); + assert_eq!(array.as_slice()?, &[1, 2, 3]); + Ok(()) + } + + #[pg_test] + fn borrow_test_i16_slice() -> Result<(), Box> { + let array = Spi::get_one::>("SELECT ARRAY[1, 2, 3]::smallint[]")? + .expect("datum was null"); + assert_eq!(array.as_slice()?, &[1, 2, 3]); + Ok(()) + } + + // #[pg_test] + // fn borrow_test_slice_with_null() -> Result<(), Box> { + // let array = Spi::get_one::>("SELECT ARRAY[1, 2, 3, NULL]::smallint[]")? + // .expect("datum was null"); + // assert_eq!(array.as_slice(), Err(FlatArraySliceError::ContainsNulls)); + // Ok(()) + // } + + #[pg_test] + fn borrow_test_array_of_points() -> Result<(), Box> { + let points: &FlatArray<'_, pg_sys::Point> = Spi::get_one( + "SELECT ARRAY['(1,1)', '(2, 2)', '(3,3)', '(4,4)', NULL, '(5,5)']::point[]", + )? + .unwrap(); + let points = points.into_iter().collect::>(); + let expected = vec![ + Some(pg_sys::Point { x: 1.0, y: 1.0 }), + Some(pg_sys::Point { x: 2.0, y: 2.0 }), + Some(pg_sys::Point { x: 3.0, y: 3.0 }), + Some(pg_sys::Point { x: 4.0, y: 4.0 }), + None, + Some(pg_sys::Point { x: 5.0, y: 5.0 }), + ]; + + for (p, expected) in points.into_iter().zip(expected.into_iter()) { + match (p, expected) { + (Some(l), Some(r)) => { + assert_eq!(l.x, r.x); + assert_eq!(l.y, r.y); + } + (None, None) => (), + _ => panic!("points not equal"), + } + } + Ok(()) + } + + #[pg_test] + fn borrow_test_text_array_as_vec_string() -> Result<(), Box> { + let a = Spi::get_one::>( + "SELECT ARRAY[NULL, NULL, NULL, NULL, 'the fifth element']::text[]", + )? + .expect("spi result was NULL") + .into_iter() + .collect::>(); + assert_eq!(a, vec![None, None, None, None, Some(String::from("the fifth element"))]); + Ok(()) + } + + #[pg_test] + fn borrow_test_text_array_iter() -> Result<(), Box> { + let a = Spi::get_one::>( + "SELECT ARRAY[NULL, NULL, NULL, NULL, 'the fifth element']::text[]", + )? + .expect("spi result was NULL"); + + let mut iter = a.iter(); + + assert_eq!(iter.next(), Some(None)); + assert_eq!(iter.next(), Some(None)); + assert_eq!(iter.next(), Some(None)); + assert_eq!(iter.next(), Some(None)); + assert_eq!(iter.next(), Some(Some(String::from("the fifth element")))); + assert_eq!(iter.next(), None); + + Ok(()) + } + + #[pg_test] + fn borrow_test_text_array_via_getter() -> Result<(), Box> { + let a = Spi::get_one::>( + "SELECT ARRAY[NULL, NULL, NULL, NULL, 'the fifth element']::text[]", + )? + .expect("spi result was NULL"); + + assert_eq!(a.get(0), Some(None)); + assert_eq!(a.get(1), Some(None)); + assert_eq!(a.get(2), Some(None)); + assert_eq!(a.get(3), Some(None)); + assert_eq!(a.get(4), Some(Some(String::from("the fifth element")))); + assert_eq!(a.get(5), None); + + Ok(()) + } +} diff --git a/pgrx-tests/src/tests/mod.rs b/pgrx-tests/src/tests/mod.rs index fd1658cce0..3cbc88a2ea 100644 --- a/pgrx-tests/src/tests/mod.rs +++ b/pgrx-tests/src/tests/mod.rs @@ -11,6 +11,7 @@ mod aggregate_tests; mod anyarray_tests; mod anyelement_tests; mod anynumeric_tests; +mod array_borrowed; mod array_tests; mod attributes_tests; mod bgworker_tests; From 10072ec0518266f9682241231987a4b8b7321c96 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 26 Sep 2024 21:17:46 -0700 Subject: [PATCH 34/54] test revision --- pgrx-tests/src/tests/array_borrowed.rs | 150 +++++++++++++------------ 1 file changed, 78 insertions(+), 72 deletions(-) diff --git a/pgrx-tests/src/tests/array_borrowed.rs b/pgrx-tests/src/tests/array_borrowed.rs index 3c70020a36..dacf2bb270 100644 --- a/pgrx-tests/src/tests/array_borrowed.rs +++ b/pgrx-tests/src/tests/array_borrowed.rs @@ -7,6 +7,8 @@ //LICENSE All rights reserved. //LICENSE //LICENSE Use of this source code is governed by the MIT license that can be found in the LICENSE file. +#[allow(unused)] +use core::ffi::CStr; use pgrx::array::{FlatArray, RawArray}; use pgrx::prelude::*; use pgrx::Json; @@ -19,7 +21,7 @@ fn borrow_sum_array_i32(values: &FlatArray<'_, i32>) -> i32 { // we implement it this way so we can trap an overflow (as we have a test for this) and // catch it correctly in both --debug and --release modes let mut sum = 0_i32; - for v in values { + for v in values.iter() { let v = v.unwrap_or(0); let (val, overflow) = sum.overflowing_add(v); if overflow { @@ -33,12 +35,12 @@ fn borrow_sum_array_i32(values: &FlatArray<'_, i32>) -> i32 { #[pg_extern(name = "sum_array")] fn borrow_sum_array_i64(values: &FlatArray<'_, i64>) -> i64 { - values.iter().map(|v| v.unwrap_or(0i64)).sum() + values.iter().map(|v| v.into_option().copied().unwrap_or(0i64)).sum() } #[pg_extern] fn borrow_count_true(values: &FlatArray<'_, bool>) -> i32 { - values.iter().filter(|b| b.unwrap_or(false)).count() as i32 + values.iter().filter(|b| b.into_option().copied().unwrap_or(false)).count() as i32 } #[pg_extern] @@ -48,12 +50,12 @@ fn borrow_count_nulls(values: &FlatArray<'_, i32>) -> i32 { #[pg_extern] fn borrow_optional_array_arg(values: Option<&FlatArray<'_, f32>>) -> f32 { - values.unwrap().iter().map(|v| v.unwrap_or(0f32)).sum() + values.unwrap().iter().map(|v| v.into_option().copied().unwrap_or(0f32)).sum() } #[pg_extern] fn borrow_iterate_array_with_deny_null(values: &FlatArray<'_, i32>) { - for _ in values.iter_deny_null() { + for _ in values.iter_non_null() { // noop } } @@ -62,7 +64,7 @@ fn borrow_iterate_array_with_deny_null(values: &FlatArray<'_, i32>) { fn borrow_optional_array_with_default( values: default!(Option<&FlatArray<'_, i32>>, "NULL"), ) -> i32 { - values.unwrap().iter().map(|v| v.unwrap_or(0)).sum() + values.unwrap().iter().map(|v| v.into_option().copied().unwrap_or(0)).sum() } // TODO: fix this test by fixing serde impls for `FlatArray<'a, &'a str> -> Json` @@ -71,15 +73,16 @@ fn borrow_optional_array_with_default( // Json(json! { { "values": values } }) // } -#[pg_extern] -fn borrow_serde_serialize_array_i32(values: &FlatArray<'_, i32>) -> Json { - Json(json! { { "values": values } }) -} +// FIXME: serialize for FlatArray? +// #[pg_extern] +// fn borrow_serde_serialize_array_i32(values: &FlatArray<'_, i32>) -> Json { +// Json(json! { { "values": values } }) +// } -#[pg_extern] -fn borrow_serde_serialize_array_i32_deny_null(values: &FlatArray<'_, i32>) -> Json { - Json(json! { { "values": values.iter_deny_null() } }) -} +// #[pg_extern] +// fn borrow_serde_serialize_array_i32_deny_null(values: &FlatArray<'_, i32>) -> Json { +// Json(json! { { "values": values.iter_non_null() } }) +// } #[pg_extern] fn borrow_return_text_array() -> Vec<&'static str> { @@ -94,7 +97,7 @@ fn borrow_return_zero_length_vec() -> Vec { #[pg_extern] fn borrow_get_arr_nelems(arr: &FlatArray<'_, i32>) -> libc::c_int { // SAFETY: Eh it's fine, it's just a len check. - unsafe { RawFlatArray::from_array(arr) }.unwrap().len() as _ + unsafe { RawArray::from_array(arr) }.unwrap().len() as _ } #[pg_extern] @@ -102,7 +105,7 @@ fn borrow_get_arr_data_ptr_nth_elem(arr: &FlatArray<'_, i32>, elem: i32) -> Opti // SAFETY: this is Known to be an FlatArray from FlatArrayType, // and it's valid-ish to see any bitpattern of an i32 inbounds of a slice. unsafe { - let raw = RawFlatArray::from_array(arr).unwrap().data::(); + let raw = RawArray::from_array(arr).unwrap().data::(); let slice = &(*raw.as_ptr()); slice.get(elem as usize).copied() } @@ -110,7 +113,7 @@ fn borrow_get_arr_data_ptr_nth_elem(arr: &FlatArray<'_, i32>, elem: i32) -> Opti #[pg_extern] fn borrow_display_get_arr_nullbitmap(arr: &FlatArray<'_, i32>) -> String { - let mut raw = unsafe { RawFlatArray::from_array(arr) }.unwrap(); + let mut raw = unsafe { RawArray::from_array(arr) }.unwrap(); if let Some(slice) = raw.nulls() { // SAFETY: If the test has gotten this far, the ptr is good for 0+ bytes, @@ -126,27 +129,27 @@ fn borrow_display_get_arr_nullbitmap(arr: &FlatArray<'_, i32>) -> String { #[pg_extern] fn borrow_get_arr_ndim(arr: &FlatArray<'_, i32>) -> libc::c_int { // SAFETY: This is a valid FlatArrayType and it's just a field access. - unsafe { RawFlatArray::from_array(arr) }.unwrap().dims().len() as _ + unsafe { RawArray::from_array(arr) }.unwrap().dims().len() as _ } // This deliberately iterates the FlatArray. // Because FlatArray::iter currently iterates the FlatArray as Datums, this is guaranteed to be "bug-free" regarding size. #[pg_extern] fn borrow_arr_mapped_vec(arr: &FlatArray<'_, i32>) -> Vec { - arr.iter().filter_map(|x| x).collect() + arr.iter().filter_map(|x| x.into_option()).collect() } /// Naive conversion. #[pg_extern] #[allow(deprecated)] fn borrow_arr_into_vec(arr: &FlatArray<'_, i32>) -> Vec { - arr.iter_deny_null().collect() + arr.iter_non_null().collect() } #[pg_extern] #[allow(deprecated)] fn borrow_arr_sort_uniq(arr: &FlatArray<'_, i32>) -> Vec { - let mut v: Vec = arr.iter_deny_null().collect(); + let mut v: Vec = arr.iter_non_null().collect(); v.sort(); v.dedup(); v @@ -168,18 +171,18 @@ fn borrow_enum_array_roundtrip( #[pg_extern] fn borrow_validate_cstring_array<'a>( - a: &FlatArray<'a, &'a core::ffi::CStr>, + a: &FlatArray<'a, CStr>, ) -> std::result::Result> { assert_eq!( 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")?), + Some(CStr::from_bytes_with_nul(b"one\0")?), + Some(CStr::from_bytes_with_nul(b"two\0")?), None, - Some(core::ffi::CStr::from_bytes_with_nul(b"four\0")?), - Some(core::ffi::CStr::from_bytes_with_nul(b"five\0")?), + Some(CStr::from_bytes_with_nul(b"four\0")?), + Some(CStr::from_bytes_with_nul(b"five\0")?), None, - Some(core::ffi::CStr::from_bytes_with_nul(b"seven\0")?), + Some(CStr::from_bytes_with_nul(b"seven\0")?), None, None ] @@ -398,7 +401,7 @@ mod tests { #[pg_test] fn borrow_test_f64_slice() -> Result<(), Box> { - let array = Spi::get_one::>("SELECT ARRAY[1.0, 2.0, 3.0]::float8[]")? + let array = Spi::get_one::<&FlatArray<'_, f64>>("SELECT ARRAY[1.0, 2.0, 3.0]::float8[]")? .expect("datum was null"); assert_eq!(array.as_slice()?, &[1.0, 2.0, 3.0]); Ok(()) @@ -406,7 +409,7 @@ mod tests { #[pg_test] fn borrow_test_f32_slice() -> Result<(), Box> { - let array = Spi::get_one::>("SELECT ARRAY[1.0, 2.0, 3.0]::float4[]")? + let array = Spi::get_one::<&FlatArray<'_, f32>>("SELECT ARRAY[1.0, 2.0, 3.0]::float4[]")? .expect("datum was null"); assert_eq!(array.as_slice()?, &[1.0, 2.0, 3.0]); Ok(()) @@ -414,7 +417,7 @@ mod tests { #[pg_test] fn borrow_test_i64_slice() -> Result<(), Box> { - let array = Spi::get_one::>("SELECT ARRAY[1, 2, 3]::bigint[]")? + let array = Spi::get_one::<&FlatArray<'_, i64>>("SELECT ARRAY[1, 2, 3]::bigint[]")? .expect("datum was null"); assert_eq!(array.as_slice()?, &[1, 2, 3]); Ok(()) @@ -422,7 +425,7 @@ mod tests { #[pg_test] fn borrow_test_i32_slice() -> Result<(), Box> { - let array = Spi::get_one::>("SELECT ARRAY[1, 2, 3]::integer[]")? + let array = Spi::get_one::<&FlatArray<'_, i32>>("SELECT ARRAY[1, 2, 3]::integer[]")? .expect("datum was null"); assert_eq!(array.as_slice()?, &[1, 2, 3]); Ok(()) @@ -430,7 +433,7 @@ mod tests { #[pg_test] fn borrow_test_i16_slice() -> Result<(), Box> { - let array = Spi::get_one::>("SELECT ARRAY[1, 2, 3]::smallint[]")? + let array = Spi::get_one::<&FlatArray<'_, i16>>("SELECT ARRAY[1, 2, 3]::smallint[]")? .expect("datum was null"); assert_eq!(array.as_slice()?, &[1, 2, 3]); Ok(()) @@ -473,51 +476,54 @@ mod tests { Ok(()) } - #[pg_test] - fn borrow_test_text_array_as_vec_string() -> Result<(), Box> { - let a = Spi::get_one::>( - "SELECT ARRAY[NULL, NULL, NULL, NULL, 'the fifth element']::text[]", - )? - .expect("spi result was NULL") - .into_iter() - .collect::>(); - assert_eq!(a, vec![None, None, None, None, Some(String::from("the fifth element"))]); - Ok(()) - } + // FIXME: needs to be type-subbed to a stringly type + // #[pg_test] + // fn borrow_test_text_array_as_vec_string() -> Result<(), Box> { + // let a = Spi::get_one::<&FlatArray<'_, String>>( + // "SELECT ARRAY[NULL, NULL, NULL, NULL, 'the fifth element']::text[]", + // )? + // .expect("spi result was NULL") + // .into_iter() + // .collect::>(); + // assert_eq!(a, vec![None, None, None, None, Some(String::from("the fifth element"))]); + // Ok(()) + // } - #[pg_test] - fn borrow_test_text_array_iter() -> Result<(), Box> { - let a = Spi::get_one::>( - "SELECT ARRAY[NULL, NULL, NULL, NULL, 'the fifth element']::text[]", - )? - .expect("spi result was NULL"); + // FIXME: needs to be type-subbed to a stringly type + // #[pg_test] + // fn borrow_test_text_array_iter() -> Result<(), Box> { + // let a = Spi::get_one::<&FlatArray<'_, String>>( + // "SELECT ARRAY[NULL, NULL, NULL, NULL, 'the fifth element']::text[]", + // )? + // .expect("spi result was NULL"); - let mut iter = a.iter(); + // let mut iter = a.iter(); - assert_eq!(iter.next(), Some(None)); - assert_eq!(iter.next(), Some(None)); - assert_eq!(iter.next(), Some(None)); - assert_eq!(iter.next(), Some(None)); - assert_eq!(iter.next(), Some(Some(String::from("the fifth element")))); - assert_eq!(iter.next(), None); + // assert_eq!(iter.next(), Some(None)); + // assert_eq!(iter.next(), Some(None)); + // assert_eq!(iter.next(), Some(None)); + // assert_eq!(iter.next(), Some(None)); + // assert_eq!(iter.next(), Some(Some(String::from("the fifth element")))); + // assert_eq!(iter.next(), None); - Ok(()) - } + // Ok(()) + // } - #[pg_test] - fn borrow_test_text_array_via_getter() -> Result<(), Box> { - let a = Spi::get_one::>( - "SELECT ARRAY[NULL, NULL, NULL, NULL, 'the fifth element']::text[]", - )? - .expect("spi result was NULL"); + // FIXME: Needs to be type-subbed to a stringly type + // #[pg_test] + // fn borrow_test_text_array_via_getter() -> Result<(), Box> { + // let a = Spi::get_one::<&FlatArray<'_, String>>( + // "SELECT ARRAY[NULL, NULL, NULL, NULL, 'the fifth element']::text[]", + // )? + // .expect("spi result was NULL"); - assert_eq!(a.get(0), Some(None)); - assert_eq!(a.get(1), Some(None)); - assert_eq!(a.get(2), Some(None)); - assert_eq!(a.get(3), Some(None)); - assert_eq!(a.get(4), Some(Some(String::from("the fifth element")))); - assert_eq!(a.get(5), None); + // assert_eq!(a.nth(0), Some(None)); + // assert_eq!(a.nth(1), Some(None)); + // assert_eq!(a.nth(2), Some(None)); + // assert_eq!(a.nth(3), Some(None)); + // assert_eq!(a.nth(4), Some(Some(String::from("the fifth element")))); + // assert_eq!(a.nth(5), None); - Ok(()) - } + // Ok(()) + // } } From a8ca379526c73f856f918fd5d15e7190dff6c887 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 26 Sep 2024 21:31:01 -0700 Subject: [PATCH 35/54] fixup tests more --- pgrx-tests/src/tests/array_borrowed.rs | 172 ++++++++++++------------- 1 file changed, 84 insertions(+), 88 deletions(-) diff --git a/pgrx-tests/src/tests/array_borrowed.rs b/pgrx-tests/src/tests/array_borrowed.rs index dacf2bb270..16b4d400fe 100644 --- a/pgrx-tests/src/tests/array_borrowed.rs +++ b/pgrx-tests/src/tests/array_borrowed.rs @@ -7,7 +7,7 @@ //LICENSE All rights reserved. //LICENSE //LICENSE Use of this source code is governed by the MIT license that can be found in the LICENSE file. -#[allow(unused)] +#![allow(unused_imports)] use core::ffi::CStr; use pgrx::array::{FlatArray, RawArray}; use pgrx::prelude::*; @@ -22,7 +22,7 @@ fn borrow_sum_array_i32(values: &FlatArray<'_, i32>) -> i32 { // catch it correctly in both --debug and --release modes let mut sum = 0_i32; for v in values.iter() { - let v = v.unwrap_or(0); + let v = v.into_option().copied().unwrap_or(0); let (val, overflow) = sum.overflowing_add(v); if overflow { panic!("attempt to add with overflow"); @@ -45,7 +45,7 @@ fn borrow_count_true(values: &FlatArray<'_, bool>) -> i32 { #[pg_extern] fn borrow_count_nulls(values: &FlatArray<'_, i32>) -> i32 { - values.iter().map(|v| v.is_none()).filter(|v| *v).count() as i32 + values.iter().map(|v| v.as_option().is_none()).filter(|v| *v).count() as i32 } #[pg_extern] @@ -96,29 +96,25 @@ fn borrow_return_zero_length_vec() -> Vec { #[pg_extern] fn borrow_get_arr_nelems(arr: &FlatArray<'_, i32>) -> libc::c_int { - // SAFETY: Eh it's fine, it's just a len check. - unsafe { RawArray::from_array(arr) }.unwrap().len() as _ + arr.count() as _ } -#[pg_extern] -fn borrow_get_arr_data_ptr_nth_elem(arr: &FlatArray<'_, i32>, elem: i32) -> Option { - // SAFETY: this is Known to be an FlatArray from FlatArrayType, - // and it's valid-ish to see any bitpattern of an i32 inbounds of a slice. - unsafe { - let raw = RawArray::from_array(arr).unwrap().data::(); - let slice = &(*raw.as_ptr()); - slice.get(elem as usize).copied() - } -} +// #[pg_extern] +// fn borrow_get_arr_data_ptr_nth_elem(arr: &FlatArray<'_, i32>, elem: i32) -> Option { +// // SAFETY: this is Known to be an FlatArray from FlatArrayType, +// // and it's valid-ish to see any bitpattern of an i32 inbounds of a slice. +// unsafe { +// let raw = RawArray::from_array(arr).unwrap().data::(); +// let slice = &(*raw.as_ptr()); +// slice.get(elem as usize).copied() +// } +// } #[pg_extern] fn borrow_display_get_arr_nullbitmap(arr: &FlatArray<'_, i32>) -> String { - let mut raw = unsafe { RawArray::from_array(arr) }.unwrap(); - - if let Some(slice) = raw.nulls() { + if let Some(slice) = arr.nulls() { // SAFETY: If the test has gotten this far, the ptr is good for 0+ bytes, // so reborrow NonNull<[u8]> as &[u8] for the hot second we're looking at it. - let slice = unsafe { &*slice.as_ptr() }; // might panic if the array is len 0 format!("{:#010b}", slice[0]) } else { @@ -136,20 +132,20 @@ fn borrow_get_arr_ndim(arr: &FlatArray<'_, i32>) -> libc::c_int { // Because FlatArray::iter currently iterates the FlatArray as Datums, this is guaranteed to be "bug-free" regarding size. #[pg_extern] fn borrow_arr_mapped_vec(arr: &FlatArray<'_, i32>) -> Vec { - arr.iter().filter_map(|x| x.into_option()).collect() + arr.iter().filter_map(|x| x.into_option()).copied().collect() } /// Naive conversion. #[pg_extern] #[allow(deprecated)] fn borrow_arr_into_vec(arr: &FlatArray<'_, i32>) -> Vec { - arr.iter_non_null().collect() + arr.iter_non_null().copied().collect() } #[pg_extern] #[allow(deprecated)] fn borrow_arr_sort_uniq(arr: &FlatArray<'_, i32>) -> Vec { - let mut v: Vec = arr.iter_non_null().collect(); + let mut v: Vec = arr.iter_non_null().copied().collect(); v.sort(); v.dedup(); v @@ -166,7 +162,7 @@ pub enum BorrowFlatArrayTestEnum { fn borrow_enum_array_roundtrip( a: &FlatArray<'_, BorrowFlatArrayTestEnum>, ) -> Vec> { - a.into_iter().collect() + a.into_iter().cloned().collect() } #[pg_extern] @@ -174,7 +170,7 @@ fn borrow_validate_cstring_array<'a>( a: &FlatArray<'a, CStr>, ) -> std::result::Result> { assert_eq!( - a.iter().collect::>(), + a.iter().map(|v| v.into_option()).collect::>(), vec![ Some(CStr::from_bytes_with_nul(b"one\0")?), Some(CStr::from_bytes_with_nul(b"two\0")?), @@ -193,7 +189,6 @@ fn borrow_validate_cstring_array<'a>( #[cfg(any(test, feature = "pg_test"))] #[pgrx::pg_schema] mod tests { - #[allow(unused_imports)] use crate as pgrx_tests; use super::*; @@ -399,45 +394,46 @@ mod tests { Ok(()) } - #[pg_test] - fn borrow_test_f64_slice() -> Result<(), Box> { - let array = Spi::get_one::<&FlatArray<'_, f64>>("SELECT ARRAY[1.0, 2.0, 3.0]::float8[]")? - .expect("datum was null"); - assert_eq!(array.as_slice()?, &[1.0, 2.0, 3.0]); - Ok(()) - } + // FIXME: lol SPI + // #[pg_test] + // fn borrow_test_f64_slice() -> Result<(), Box> { + // let array = Spi::get_one::<&FlatArray<'_, f64>>("SELECT ARRAY[1.0, 2.0, 3.0]::float8[]")? + // .expect("datum was null"); + // assert_eq!(array.as_slice()?, &[1.0, 2.0, 3.0]); + // Ok(()) + // } - #[pg_test] - fn borrow_test_f32_slice() -> Result<(), Box> { - let array = Spi::get_one::<&FlatArray<'_, f32>>("SELECT ARRAY[1.0, 2.0, 3.0]::float4[]")? - .expect("datum was null"); - assert_eq!(array.as_slice()?, &[1.0, 2.0, 3.0]); - Ok(()) - } + // #[pg_test] + // fn borrow_test_f32_slice() -> Result<(), Box> { + // let array = Spi::get_one::<&FlatArray<'_, f32>>("SELECT ARRAY[1.0, 2.0, 3.0]::float4[]")? + // .expect("datum was null"); + // assert_eq!(array.as_slice()?, &[1.0, 2.0, 3.0]); + // Ok(()) + // } - #[pg_test] - fn borrow_test_i64_slice() -> Result<(), Box> { - let array = Spi::get_one::<&FlatArray<'_, i64>>("SELECT ARRAY[1, 2, 3]::bigint[]")? - .expect("datum was null"); - assert_eq!(array.as_slice()?, &[1, 2, 3]); - Ok(()) - } + // #[pg_test] + // fn borrow_test_i64_slice() -> Result<(), Box> { + // let array = Spi::get_one::<&FlatArray<'_, i64>>("SELECT ARRAY[1, 2, 3]::bigint[]")? + // .expect("datum was null"); + // assert_eq!(array.as_slice()?, &[1, 2, 3]); + // Ok(()) + // } - #[pg_test] - fn borrow_test_i32_slice() -> Result<(), Box> { - let array = Spi::get_one::<&FlatArray<'_, i32>>("SELECT ARRAY[1, 2, 3]::integer[]")? - .expect("datum was null"); - assert_eq!(array.as_slice()?, &[1, 2, 3]); - Ok(()) - } + // #[pg_test] + // fn borrow_test_i32_slice() -> Result<(), Box> { + // let array = Spi::get_one::<&FlatArray<'_, i32>>("SELECT ARRAY[1, 2, 3]::integer[]")? + // .expect("datum was null"); + // assert_eq!(array.as_slice()?, &[1, 2, 3]); + // Ok(()) + // } - #[pg_test] - fn borrow_test_i16_slice() -> Result<(), Box> { - let array = Spi::get_one::<&FlatArray<'_, i16>>("SELECT ARRAY[1, 2, 3]::smallint[]")? - .expect("datum was null"); - assert_eq!(array.as_slice()?, &[1, 2, 3]); - Ok(()) - } + // #[pg_test] + // fn borrow_test_i16_slice() -> Result<(), Box> { + // let array = Spi::get_one::<&FlatArray<'_, i16>>("SELECT ARRAY[1, 2, 3]::smallint[]")? + // .expect("datum was null"); + // assert_eq!(array.as_slice()?, &[1, 2, 3]); + // Ok(()) + // } // #[pg_test] // fn borrow_test_slice_with_null() -> Result<(), Box> { @@ -447,34 +443,34 @@ mod tests { // Ok(()) // } - #[pg_test] - fn borrow_test_array_of_points() -> Result<(), Box> { - let points: &FlatArray<'_, pg_sys::Point> = Spi::get_one( - "SELECT ARRAY['(1,1)', '(2, 2)', '(3,3)', '(4,4)', NULL, '(5,5)']::point[]", - )? - .unwrap(); - let points = points.into_iter().collect::>(); - let expected = vec![ - Some(pg_sys::Point { x: 1.0, y: 1.0 }), - Some(pg_sys::Point { x: 2.0, y: 2.0 }), - Some(pg_sys::Point { x: 3.0, y: 3.0 }), - Some(pg_sys::Point { x: 4.0, y: 4.0 }), - None, - Some(pg_sys::Point { x: 5.0, y: 5.0 }), - ]; - - for (p, expected) in points.into_iter().zip(expected.into_iter()) { - match (p, expected) { - (Some(l), Some(r)) => { - assert_eq!(l.x, r.x); - assert_eq!(l.y, r.y); - } - (None, None) => (), - _ => panic!("points not equal"), - } - } - Ok(()) - } + // #[pg_test] + // fn borrow_test_array_of_points() -> Result<(), Box> { + // let points: &FlatArray<'_, pg_sys::Point> = Spi::get_one( + // "SELECT ARRAY['(1,1)', '(2, 2)', '(3,3)', '(4,4)', NULL, '(5,5)']::point[]", + // )? + // .unwrap(); + // let points = points.into_iter().collect::>(); + // let expected = vec![ + // Some(pg_sys::Point { x: 1.0, y: 1.0 }), + // Some(pg_sys::Point { x: 2.0, y: 2.0 }), + // Some(pg_sys::Point { x: 3.0, y: 3.0 }), + // Some(pg_sys::Point { x: 4.0, y: 4.0 }), + // None, + // Some(pg_sys::Point { x: 5.0, y: 5.0 }), + // ]; + + // for (p, expected) in points.into_iter().zip(expected.into_iter()) { + // match (p, expected) { + // (Some(l), Some(r)) => { + // assert_eq!(l.x, r.x); + // assert_eq!(l.y, r.y); + // } + // (None, None) => (), + // _ => panic!("points not equal"), + // } + // } + // Ok(()) + // } // FIXME: needs to be type-subbed to a stringly type // #[pg_test] From 960869e8dbc5be53587691cfffe2d21705c4def5 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 26 Sep 2024 21:46:13 -0700 Subject: [PATCH 36/54] fix all compile blockers for tests --- pgrx-tests/src/tests/array_borrowed.rs | 126 +++++++++++++------------ 1 file changed, 64 insertions(+), 62 deletions(-) diff --git a/pgrx-tests/src/tests/array_borrowed.rs b/pgrx-tests/src/tests/array_borrowed.rs index 16b4d400fe..22b0a3debc 100644 --- a/pgrx-tests/src/tests/array_borrowed.rs +++ b/pgrx-tests/src/tests/array_borrowed.rs @@ -122,11 +122,11 @@ fn borrow_display_get_arr_nullbitmap(arr: &FlatArray<'_, i32>) -> String { } } -#[pg_extern] -fn borrow_get_arr_ndim(arr: &FlatArray<'_, i32>) -> libc::c_int { - // SAFETY: This is a valid FlatArrayType and it's just a field access. - unsafe { RawArray::from_array(arr) }.unwrap().dims().len() as _ -} +// #[pg_extern] +// fn borrow_get_arr_ndim(arr: &FlatArray<'_, i32>) -> libc::c_int { +// // SAFETY: This is a valid FlatArrayType and it's just a field access. +// unsafe { RawArray::from_array(arr) }.unwrap().dims().len() as _ +// } // This deliberately iterates the FlatArray. // Because FlatArray::iter currently iterates the FlatArray as Datums, this is guaranteed to be "bug-free" regarding size. @@ -151,40 +151,42 @@ fn borrow_arr_sort_uniq(arr: &FlatArray<'_, i32>) -> Vec { v } -#[derive(Debug, Eq, PartialEq, PostgresEnum, Serialize)] -pub enum BorrowFlatArrayTestEnum { - One, - Two, - Three, -} +// FIXME: BorrowDatum for PostgresEnum? +// #[derive(Debug, Eq, PartialEq, PostgresEnum, Serialize)] +// pub enum BorrowFlatArrayTestEnum { +// One, +// Two, +// Three, +// } -#[pg_extern] -fn borrow_enum_array_roundtrip( - a: &FlatArray<'_, BorrowFlatArrayTestEnum>, -) -> Vec> { - a.into_iter().cloned().collect() -} +// #[pg_extern] +// fn borrow_enum_array_roundtrip( +// a: &FlatArray<'_, BorrowFlatArrayTestEnum>, +// ) -> Vec> { +// a.iter().cloned().collect() +// } -#[pg_extern] -fn borrow_validate_cstring_array<'a>( - a: &FlatArray<'a, CStr>, -) -> std::result::Result> { - assert_eq!( - a.iter().map(|v| v.into_option()).collect::>(), - vec![ - Some(CStr::from_bytes_with_nul(b"one\0")?), - Some(CStr::from_bytes_with_nul(b"two\0")?), - None, - Some(CStr::from_bytes_with_nul(b"four\0")?), - Some(CStr::from_bytes_with_nul(b"five\0")?), - None, - Some(CStr::from_bytes_with_nul(b"seven\0")?), - None, - None - ] - ); - Ok(true) -} +// FIXME: something about entity? +// #[pg_extern] +// fn borrow_validate_cstring_array( +// a: &FlatArray<'_, CStr>, +// ) -> std::result::Result> { +// assert_eq!( +// a.iter().map(|v| v.into_option()).collect::>(), +// vec![ +// Some(CStr::from_bytes_with_nul(b"one\0")?), +// Some(CStr::from_bytes_with_nul(b"two\0")?), +// None, +// Some(CStr::from_bytes_with_nul(b"four\0")?), +// Some(CStr::from_bytes_with_nul(b"five\0")?), +// None, +// Some(CStr::from_bytes_with_nul(b"seven\0")?), +// None, +// None +// ] +// ); +// Ok(true) +// } #[cfg(any(test, feature = "pg_test"))] #[pgrx::pg_schema] @@ -196,15 +198,15 @@ mod tests { use pgrx::{IntoDatum, Json}; use serde_json::json; - #[pg_test] - fn borrow_test_enum_array_roundtrip() -> spi::Result<()> { - let a = Spi::get_one::>>( - "SELECT borrow_enum_array_roundtrip(ARRAY['One', 'Two']::BorrowFlatArrayTestEnum[])", - )? - .expect("SPI result was null"); - assert_eq!(a, vec![Some(BorrowFlatArrayTestEnum::One), Some(BorrowFlatArrayTestEnum::Two)]); - Ok(()) - } + // #[pg_test] + // fn borrow_test_enum_array_roundtrip() -> spi::Result<()> { + // let a = Spi::get_one::>>( + // "SELECT borrow_enum_array_roundtrip(ARRAY['One', 'Two']::BorrowFlatArrayTestEnum[])", + // )? + // .expect("SPI result was null"); + // assert_eq!(a, vec![Some(BorrowFlatArrayTestEnum::One), Some(BorrowFlatArrayTestEnum::Two)]); + // Ok(()) + // } #[pg_test] fn borrow_test_sum_array_i32() { @@ -347,19 +349,19 @@ mod tests { Ok(()) } - #[pg_test] - fn borrow_test_get_arr_ndim() -> Result<(), pgrx::spi::Error> { - let ndim = Spi::get_one::("SELECT borrow_get_arr_ndim(ARRAY[1,2,3,4,5]::int[])")? - .expect("datum was null"); + // #[pg_test] + // fn borrow_test_get_arr_ndim() -> Result<(), pgrx::spi::Error> { + // let ndim = Spi::get_one::("SELECT borrow_get_arr_ndim(ARRAY[1,2,3,4,5]::int[])")? + // .expect("datum was null"); - assert_eq!(ndim, 1); + // assert_eq!(ndim, 1); - let ndim = Spi::get_one::("SELECT borrow_get_arr_ndim('{{1,2,3},{4,5,6}}'::int[])")? - .expect("datum was null"); + // let ndim = Spi::get_one::("SELECT borrow_get_arr_ndim('{{1,2,3},{4,5,6}}'::int[])")? + // .expect("datum was null"); - assert_eq!(ndim, 2); - Ok(()) - } + // assert_eq!(ndim, 2); + // Ok(()) + // } #[pg_test] fn borrow_test_arr_to_vec() { @@ -387,12 +389,12 @@ mod tests { .map(|_| ()) } - #[pg_test] - fn borrow_test_cstring_array() -> Result<(), pgrx::spi::Error> { - let strings = Spi::get_one::("SELECT borrow_validate_cstring_array(ARRAY['one', 'two', NULL, 'four', 'five', NULL, 'seven', NULL, NULL]::cstring[])")?.expect("datum was NULL"); - assert_eq!(strings, true); - Ok(()) - } + // #[pg_test] + // fn borrow_test_cstring_array() -> Result<(), pgrx::spi::Error> { + // let strings = Spi::get_one::("SELECT borrow_validate_cstring_array(ARRAY['one', 'two', NULL, 'four', 'five', NULL, 'seven', NULL, NULL]::cstring[])")?.expect("datum was NULL"); + // assert_eq!(strings, true); + // Ok(()) + // } // FIXME: lol SPI // #[pg_test] From 2ed093feacf83094ccf6fafe5c61ccd0a19d7948 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 26 Sep 2024 21:54:00 -0700 Subject: [PATCH 37/54] Fix all test issuues --- pgrx-tests/src/tests/array_borrowed.rs | 46 +++++++++++++------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/pgrx-tests/src/tests/array_borrowed.rs b/pgrx-tests/src/tests/array_borrowed.rs index 22b0a3debc..c440bd05b6 100644 --- a/pgrx-tests/src/tests/array_borrowed.rs +++ b/pgrx-tests/src/tests/array_borrowed.rs @@ -16,7 +16,7 @@ use pgrx::PostgresEnum; use serde::Serialize; use serde_json::json; -#[pg_extern(name = "sum_array")] +#[pg_extern(name = "borrow_sum_array")] fn borrow_sum_array_i32(values: &FlatArray<'_, i32>) -> i32 { // we implement it this way so we can trap an overflow (as we have a test for this) and // catch it correctly in both --debug and --release modes @@ -33,7 +33,7 @@ fn borrow_sum_array_i32(values: &FlatArray<'_, i32>) -> i32 { sum } -#[pg_extern(name = "sum_array")] +#[pg_extern(name = "borrow_sum_array")] fn borrow_sum_array_i64(values: &FlatArray<'_, i64>) -> i64 { values.iter().map(|v| v.into_option().copied().unwrap_or(0i64)).sum() } @@ -268,22 +268,22 @@ mod tests { assert_eq!(sum, Ok(Some(6))); } - #[pg_test] - fn borrow_test_serde_serialize_array_i32() -> Result<(), pgrx::spi::Error> { - let json = Spi::get_one::( - "SELECT borrow_serde_serialize_array_i32(ARRAY[1, null, 2, 3, null, 4, 5])", - )? - .expect("returned json was null"); - assert_eq!(json.0, json! {{"values": [1,null,2,3,null,4, 5]}}); - Ok(()) - } + // #[pg_test] + // fn borrow_test_serde_serialize_array_i32() -> Result<(), pgrx::spi::Error> { + // let json = Spi::get_one::( + // "SELECT borrow_serde_serialize_array_i32(ARRAY[1, null, 2, 3, null, 4, 5])", + // )? + // .expect("returned json was null"); + // assert_eq!(json.0, json! {{"values": [1,null,2,3,null,4, 5]}}); + // Ok(()) + // } - #[pg_test(expected = "array contains NULL")] - fn borrow_test_serde_serialize_array_i32_deny_null() -> Result, pgrx::spi::Error> { - Spi::get_one::( - "SELECT borrow_serde_serialize_array_i32_deny_null(ARRAY[1, 2, 3, null, 4, 5])", - ) - } + // #[pg_test(expected = "array contains NULL")] + // fn borrow_test_serde_serialize_array_i32_deny_null() -> Result, pgrx::spi::Error> { + // Spi::get_one::( + // "SELECT borrow_serde_serialize_array_i32_deny_null(ARRAY[1, 2, 3, null, 4, 5])", + // ) + // } #[pg_test] fn borrow_test_return_text_array() { @@ -324,12 +324,12 @@ mod tests { assert_eq!(len, Ok(Some(5))); } - #[pg_test] - fn borrow_test_get_arr_data_ptr_nth_elem() { - let nth = - Spi::get_one::("SELECT borrow_get_arr_data_ptr_nth_elem('{1,2,3,4,5}'::int[], 2)"); - assert_eq!(nth, Ok(Some(3))); - } + // #[pg_test] + // fn borrow_test_get_arr_data_ptr_nth_elem() { + // let nth = + // Spi::get_one::("SELECT borrow_get_arr_data_ptr_nth_elem('{1,2,3,4,5}'::int[], 2)"); + // assert_eq!(nth, Ok(Some(3))); + // } #[pg_test] fn borrow_test_display_get_arr_nullbitmap() -> Result<(), pgrx::spi::Error> { From ea31e2016bdf5e52d8de835a87c0c9c0fb63c12e Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 26 Sep 2024 21:54:35 -0700 Subject: [PATCH 38/54] note stopping point in array iteration --- pgrx/src/array.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pgrx/src/array.rs b/pgrx/src/array.rs index 87da552efe..bcfab51e2a 100644 --- a/pgrx/src/array.rs +++ b/pgrx/src/array.rs @@ -195,6 +195,7 @@ where } fn is_null(p: Option>) -> bool { + // TODO: this if let Some(p) = p { todo!() } else { @@ -209,6 +210,7 @@ where type Item = Nullable<&'arr T>; fn next(&mut self) -> Option> { + // TODO: iteration if todo!() { Some(Nullable::Null) } else { From e2294dcc536715125b567a72aa3368b32da973ec Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Thu, 26 Sep 2024 23:51:43 -0700 Subject: [PATCH 39/54] partially implement iteration --- pgrx/src/array.rs | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/pgrx/src/array.rs b/pgrx/src/array.rs index bcfab51e2a..0673dd8fff 100644 --- a/pgrx/src/array.rs +++ b/pgrx/src/array.rs @@ -194,15 +194,6 @@ where offset: usize, } -fn is_null(p: Option>) -> bool { - // TODO: this - if let Some(p) = p { - todo!() - } else { - false - } -} - impl<'arr, T> Iterator for ArrayIter<'arr, T> where T: ?Sized + BorrowDatum, @@ -210,12 +201,23 @@ where type Item = Nullable<&'arr T>; fn next(&mut self) -> Option> { - // TODO: iteration - if todo!() { + if self.index >= self.nelems { + return None; + } + let is_null = match self.nulls { + Some(nulls) => !nulls.get(index).unwrap(), + None => false, + }; + // note the index freezes when we reach the end of the null bitslice, fusing the iterator + self.index += 1; + + if is_null { Some(Nullable::Null) } else { unsafe { - let ptr = ::point_from(self.data.cast_mut()); + let ptr = ::point_from(self.data.add(self.offset).cast_mut()); + // need some way of determining size using BorrowDatum... + self.offset += todo!(); Some(Nullable::Valid(&*ptr)) } } From 6fca1e3e8f5fccff86bc96d48246bb7a0c4311c9 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Fri, 27 Sep 2024 21:46:21 -0700 Subject: [PATCH 40/54] impl ArrayIter::next fully also remove irrelevant deny-null tests and fix remaining tests. --- pgrx-tests/src/tests/array_borrowed.rs | 62 +++++++++++--------------- pgrx-tests/src/tests/array_tests.rs | 1 + pgrx/src/array.rs | 23 ++++++---- 3 files changed, 40 insertions(+), 46 deletions(-) diff --git a/pgrx-tests/src/tests/array_borrowed.rs b/pgrx-tests/src/tests/array_borrowed.rs index c440bd05b6..390d3b17cc 100644 --- a/pgrx-tests/src/tests/array_borrowed.rs +++ b/pgrx-tests/src/tests/array_borrowed.rs @@ -53,13 +53,6 @@ fn borrow_optional_array_arg(values: Option<&FlatArray<'_, f32>>) -> f32 { values.unwrap().iter().map(|v| v.into_option().copied().unwrap_or(0f32)).sum() } -#[pg_extern] -fn borrow_iterate_array_with_deny_null(values: &FlatArray<'_, i32>) { - for _ in values.iter_non_null() { - // noop - } -} - #[pg_extern] fn borrow_optional_array_with_default( values: default!(Option<&FlatArray<'_, i32>>, "NULL"), @@ -223,7 +216,7 @@ mod tests { #[pg_test(expected = "attempt to add with overflow")] fn borrow_test_sum_array_i32_overflow() -> Result, pgrx::spi::Error> { Spi::get_one::( - "SELECT borrow_sum_array(a) FROM (SELECT borrow_array_agg(s) a FROM generate_series(1, 1000000) s) x;", + "SELECT borrow_sum_array(a) FROM (SELECT array_agg(s) a FROM generate_series(1, 1000000) s) x;", ) } @@ -246,11 +239,6 @@ mod tests { assert_eq!(sum, Ok(Some(6f32))); } - #[pg_test(expected = "array contains NULL")] - fn borrow_test_array_deny_nulls() -> Result<(), spi::Error> { - Spi::run("SELECT borrow_iterate_array_with_deny_null(ARRAY[1,2,3, NULL]::int[])") - } - // TODO: fix this test by redesigning SPI. // #[pg_test] // fn borrow_test_serde_serialize_array() -> Result<(), pgrx::spi::Error> { @@ -297,26 +285,26 @@ mod tests { assert_eq!(rc, Ok(Some(true))); } - #[pg_test] - fn borrow_test_slice_to_array() -> Result<(), pgrx::spi::Error> { - let owned_vec = vec![Some(1), None, Some(2), Some(3), None, Some(4), Some(5)]; - let json = Spi::connect(|client| { - client - .select( - "SELECT borrow_serde_serialize_array_i32($1)", - None, - Some(vec![( - PgBuiltInOids::INT4ARRAYOID.oid(), - owned_vec.as_slice().into_datum(), - )]), - )? - .first() - .get_one::() - })? - .expect("Failed to return json even though it's right there ^^"); - assert_eq!(json.0, json! {{"values": [1, null, 2, 3, null, 4, 5]}}); - Ok(()) - } + // #[pg_test] + // fn borrow_test_slice_to_array() -> Result<(), pgrx::spi::Error> { + // let owned_vec = vec![Some(1), None, Some(2), Some(3), None, Some(4), Some(5)]; + // let json = Spi::connect(|client| { + // client + // .select( + // "SELECT borrow_serde_serialize_array_i32($1)", + // None, + // Some(vec![( + // PgBuiltInOids::INT4ARRAYOID.oid(), + // owned_vec.as_slice().into_datum(), + // )]), + // )? + // .first() + // .get_one::() + // })? + // .expect("Failed to return json even though it's right there ^^"); + // assert_eq!(json.0, json! {{"values": [1, null, 2, 3, null, 4, 5]}}); + // Ok(()) + // } #[pg_test] fn borrow_test_arr_data_ptr() { @@ -383,10 +371,10 @@ mod tests { } #[pg_test] - #[should_panic] - fn borrow_test_arr_sort_uniq_with_null() -> Result<(), pgrx::spi::Error> { - Spi::get_one::>("SELECT borrow_arr_sort_uniq(ARRAY[3,2,NULL,2,1]::integer[])") - .map(|_| ()) + fn borrow_test_arr_sort_uniq_with_null() { + let result = + Spi::get_one::>("SELECT borrow_arr_sort_uniq(ARRAY[3,2,NULL,2,1]::integer[])"); + assert_eq!(result, Ok(Some(vec![1, 2, 3]))); } // #[pg_test] diff --git a/pgrx-tests/src/tests/array_tests.rs b/pgrx-tests/src/tests/array_tests.rs index 855f284a07..b77ddd44cd 100644 --- a/pgrx-tests/src/tests/array_tests.rs +++ b/pgrx-tests/src/tests/array_tests.rs @@ -218,6 +218,7 @@ mod tests { #[pg_test(expected = "attempt to add with overflow")] fn test_sum_array_i32_overflow() -> Result, pgrx::spi::Error> { + // Note that this test is calling a builtin, array_agg Spi::get_one::( "SELECT sum_array(a) FROM (SELECT array_agg(s) a FROM generate_series(1, 1000000) s) x;", ) diff --git a/pgrx/src/array.rs b/pgrx/src/array.rs index 0673dd8fff..dec016bc1c 100644 --- a/pgrx/src/array.rs +++ b/pgrx/src/array.rs @@ -10,6 +10,7 @@ #![allow(clippy::precedence)] #![allow(unused)] use crate::datum::{Array, BorrowDatum, Datum}; +use crate::layout::{Align, Layout}; use crate::nullable::Nullable; use crate::pgrx_sql_entity_graph::metadata::{ ArgumentError, Returns, ReturnsError, SqlMapping, SqlTranslatable, @@ -28,6 +29,7 @@ mod port; /// /// `pgrx::datum::Array` is essentially `&FlatArray` #[repr(C)] +#[derive(Debug)] pub struct FlatArray<'mcx, T: ?Sized> { scalar: PhantomData<&'mcx T>, head: pg_sys::ArrayType, @@ -68,8 +70,9 @@ where let arr = self; let index = 0; let offset = 0; + let align = Layout::lookup_oid(self.head.elemtype).align; - ArrayIter { data, nulls, nelems, arr, index, offset } + ArrayIter { data, nulls, nelems, arr, index, offset, align } } pub fn iter_non_null(&self) -> impl Iterator { @@ -192,6 +195,7 @@ where nelems: usize, index: usize, offset: usize, + align: Align, } impl<'arr, T> Iterator for ArrayIter<'arr, T> @@ -205,21 +209,22 @@ where return None; } let is_null = match self.nulls { - Some(nulls) => !nulls.get(index).unwrap(), + Some(nulls) => !nulls.get(self.index).unwrap(), None => false, }; - // note the index freezes when we reach the end of the null bitslice, fusing the iterator + // note the index freezes when we reach the end, fusing the iterator self.index += 1; if is_null { + // note that we do NOT offset when the value is a null! Some(Nullable::Null) } else { - unsafe { - let ptr = ::point_from(self.data.add(self.offset).cast_mut()); - // need some way of determining size using BorrowDatum... - self.offset += todo!(); - Some(Nullable::Valid(&*ptr)) - } + let borrow = unsafe { + &*::point_from(self.data.byte_add(self.offset).cast_mut()) + }; + // As we always have a borrow, we just ask Rust what the array element's size is + self.offset += self.align.pad(mem::size_of_val(borrow)); + Some(Nullable::Valid(borrow)) } } } From e9ee7bd680d4ede191baa4e2bb6344784831991a Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Sat, 28 Sep 2024 00:49:45 -0700 Subject: [PATCH 41/54] fix up FlatArray for CStr --- pgrx-tests/src/tests/array_borrowed.rs | 53 +++++++++++++------------- pgrx/src/array.rs | 2 +- 2 files changed, 27 insertions(+), 28 deletions(-) diff --git a/pgrx-tests/src/tests/array_borrowed.rs b/pgrx-tests/src/tests/array_borrowed.rs index 390d3b17cc..af0fe6891a 100644 --- a/pgrx-tests/src/tests/array_borrowed.rs +++ b/pgrx-tests/src/tests/array_borrowed.rs @@ -159,27 +159,26 @@ fn borrow_arr_sort_uniq(arr: &FlatArray<'_, i32>) -> Vec { // a.iter().cloned().collect() // } -// FIXME: something about entity? -// #[pg_extern] -// fn borrow_validate_cstring_array( -// a: &FlatArray<'_, CStr>, -// ) -> std::result::Result> { -// assert_eq!( -// a.iter().map(|v| v.into_option()).collect::>(), -// vec![ -// Some(CStr::from_bytes_with_nul(b"one\0")?), -// Some(CStr::from_bytes_with_nul(b"two\0")?), -// None, -// Some(CStr::from_bytes_with_nul(b"four\0")?), -// Some(CStr::from_bytes_with_nul(b"five\0")?), -// None, -// Some(CStr::from_bytes_with_nul(b"seven\0")?), -// None, -// None -// ] -// ); -// Ok(true) -// } +#[pg_extern] +fn borrow_validate_cstring_array( + a: &FlatArray<'_, CStr>, +) -> std::result::Result> { + assert_eq!( + a.iter().map(|v| v.into_option()).collect::>(), + vec![ + Some(CStr::from_bytes_with_nul(b"one\0")?), + Some(CStr::from_bytes_with_nul(b"two\0")?), + None, + Some(CStr::from_bytes_with_nul(b"four\0")?), + Some(CStr::from_bytes_with_nul(b"five\0")?), + None, + Some(CStr::from_bytes_with_nul(b"seven\0")?), + None, + None + ] + ); + Ok(true) +} #[cfg(any(test, feature = "pg_test"))] #[pgrx::pg_schema] @@ -377,12 +376,12 @@ mod tests { assert_eq!(result, Ok(Some(vec![1, 2, 3]))); } - // #[pg_test] - // fn borrow_test_cstring_array() -> Result<(), pgrx::spi::Error> { - // let strings = Spi::get_one::("SELECT borrow_validate_cstring_array(ARRAY['one', 'two', NULL, 'four', 'five', NULL, 'seven', NULL, NULL]::cstring[])")?.expect("datum was NULL"); - // assert_eq!(strings, true); - // Ok(()) - // } + #[pg_test] + fn borrow_test_cstring_array() -> Result<(), pgrx::spi::Error> { + let strings = Spi::get_one::("SELECT borrow_validate_cstring_array(ARRAY['one', 'two', NULL, 'four', 'five', NULL, 'seven', NULL, NULL]::cstring[])")?.expect("datum was NULL"); + assert_eq!(strings, true); + Ok(()) + } // FIXME: lol SPI // #[pg_test] diff --git a/pgrx/src/array.rs b/pgrx/src/array.rs index dec016bc1c..478df09bda 100644 --- a/pgrx/src/array.rs +++ b/pgrx/src/array.rs @@ -158,7 +158,7 @@ unsafe impl BorrowDatum for FlatArray<'_, T> { unsafe impl SqlTranslatable for &FlatArray<'_, T> where - T: SqlTranslatable, + T: ?Sized + SqlTranslatable, { fn argument_sql() -> Result { match T::argument_sql()? { From 4fbe8c41f0da6fd97c0d07b875990606506767e4 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Sat, 28 Sep 2024 02:29:29 -0700 Subject: [PATCH 42/54] Add Dimensions support and another test --- pgrx-tests/src/tests/array_borrowed.rs | 44 ++++++++++---------------- pgrx/src/array.rs | 24 +++++++++++++- 2 files changed, 39 insertions(+), 29 deletions(-) diff --git a/pgrx-tests/src/tests/array_borrowed.rs b/pgrx-tests/src/tests/array_borrowed.rs index af0fe6891a..f5c865c423 100644 --- a/pgrx-tests/src/tests/array_borrowed.rs +++ b/pgrx-tests/src/tests/array_borrowed.rs @@ -72,11 +72,6 @@ fn borrow_optional_array_with_default( // Json(json! { { "values": values } }) // } -// #[pg_extern] -// fn borrow_serde_serialize_array_i32_deny_null(values: &FlatArray<'_, i32>) -> Json { -// Json(json! { { "values": values.iter_non_null() } }) -// } - #[pg_extern] fn borrow_return_text_array() -> Vec<&'static str> { vec!["a", "b", "c", "d"] @@ -115,11 +110,11 @@ fn borrow_display_get_arr_nullbitmap(arr: &FlatArray<'_, i32>) -> String { } } -// #[pg_extern] -// fn borrow_get_arr_ndim(arr: &FlatArray<'_, i32>) -> libc::c_int { -// // SAFETY: This is a valid FlatArrayType and it's just a field access. -// unsafe { RawArray::from_array(arr) }.unwrap().dims().len() as _ -// } +#[pg_extern] +fn borrow_get_arr_ndim(arr: &FlatArray<'_, i32>) -> libc::c_int { + // SAFETY: This is a valid FlatArrayType and it's just a field access. + arr.dims().len() as libc::c_int +} // This deliberately iterates the FlatArray. // Because FlatArray::iter currently iterates the FlatArray as Datums, this is guaranteed to be "bug-free" regarding size. @@ -265,13 +260,6 @@ mod tests { // Ok(()) // } - // #[pg_test(expected = "array contains NULL")] - // fn borrow_test_serde_serialize_array_i32_deny_null() -> Result, pgrx::spi::Error> { - // Spi::get_one::( - // "SELECT borrow_serde_serialize_array_i32_deny_null(ARRAY[1, 2, 3, null, 4, 5])", - // ) - // } - #[pg_test] fn borrow_test_return_text_array() { let rc = Spi::get_one::("SELECT ARRAY['a', 'b', 'c', 'd'] = return_text_array();"); @@ -336,19 +324,19 @@ mod tests { Ok(()) } - // #[pg_test] - // fn borrow_test_get_arr_ndim() -> Result<(), pgrx::spi::Error> { - // let ndim = Spi::get_one::("SELECT borrow_get_arr_ndim(ARRAY[1,2,3,4,5]::int[])")? - // .expect("datum was null"); + #[pg_test] + fn borrow_test_get_arr_ndim() -> Result<(), pgrx::spi::Error> { + let ndim = Spi::get_one::("SELECT borrow_get_arr_ndim(ARRAY[1,2,3,4,5]::int[])")? + .expect("datum was null"); - // assert_eq!(ndim, 1); + assert_eq!(ndim, 1); - // let ndim = Spi::get_one::("SELECT borrow_get_arr_ndim('{{1,2,3},{4,5,6}}'::int[])")? - // .expect("datum was null"); + let ndim = Spi::get_one::("SELECT borrow_get_arr_ndim('{{1,2,3},{4,5,6}}'::int[])")? + .expect("datum was null"); - // assert_eq!(ndim, 2); - // Ok(()) - // } + assert_eq!(ndim, 2); + Ok(()) + } #[pg_test] fn borrow_test_arr_to_vec() { @@ -461,7 +449,7 @@ mod tests { // Ok(()) // } - // FIXME: needs to be type-subbed to a stringly type + // FIXME: needs to be type-subbed to a stringly type and SPI needs to make sense // #[pg_test] // fn borrow_test_text_array_as_vec_string() -> Result<(), Box> { // let a = Spi::get_one::<&FlatArray<'_, String>>( diff --git a/pgrx/src/array.rs b/pgrx/src/array.rs index 478df09bda..e0aa82033e 100644 --- a/pgrx/src/array.rs +++ b/pgrx/src/array.rs @@ -21,7 +21,7 @@ use bitvec::ptr::{self as bitptr, BitPtr, BitPtrError, Const, Mut}; use bitvec::slice::{self as bitslice, BitSlice}; use core::marker::PhantomData; use core::ptr::{self, NonNull}; -use core::{mem, slice}; +use core::{ffi, mem, slice}; mod port; @@ -53,6 +53,17 @@ where pub fn count(&self) -> usize { self.as_raw().len() } + + pub fn dims(&self) -> Dimensions { + // SAFETY: Validity of the ptr and ndim field was asserted upon obtaining the FlatArray ref, + // so can assume the dims ptr is also valid, allowing making the slice. + unsafe { + let ptr = self as *const Self as *const pg_sys::ArrayType; + let ndim = self.head.ndim as usize; + let dims = slice::from_raw_parts(port::ARR_DIMS(ptr.cast_mut()), ndim); + Dimensions { dims } + } + } } impl<'mcx, T> FlatArray<'mcx, T> @@ -183,6 +194,17 @@ where } } +#[derive(Clone)] +pub struct Dimensions<'arr> { + dims: &'arr [ffi::c_int], +} + +impl<'arr> Dimensions<'arr> { + pub fn len(&self) -> usize { + self.dims.len() + } +} + /// Iterator for arrays #[derive(Clone)] struct ArrayIter<'arr, T> From 96b6d5df162a7142bb5d86994915ba1bb26d25e1 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Sat, 28 Sep 2024 02:37:44 -0700 Subject: [PATCH 43/54] Fixup another test --- pgrx-tests/src/tests/array_borrowed.rs | 27 +++++++++++--------------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/pgrx-tests/src/tests/array_borrowed.rs b/pgrx-tests/src/tests/array_borrowed.rs index f5c865c423..eb7b874bba 100644 --- a/pgrx-tests/src/tests/array_borrowed.rs +++ b/pgrx-tests/src/tests/array_borrowed.rs @@ -10,6 +10,7 @@ #![allow(unused_imports)] use core::ffi::CStr; use pgrx::array::{FlatArray, RawArray}; +use pgrx::nullable::Nullable; use pgrx::prelude::*; use pgrx::Json; use pgrx::PostgresEnum; @@ -87,16 +88,10 @@ fn borrow_get_arr_nelems(arr: &FlatArray<'_, i32>) -> libc::c_int { arr.count() as _ } -// #[pg_extern] -// fn borrow_get_arr_data_ptr_nth_elem(arr: &FlatArray<'_, i32>, elem: i32) -> Option { -// // SAFETY: this is Known to be an FlatArray from FlatArrayType, -// // and it's valid-ish to see any bitpattern of an i32 inbounds of a slice. -// unsafe { -// let raw = RawArray::from_array(arr).unwrap().data::(); -// let slice = &(*raw.as_ptr()); -// slice.get(elem as usize).copied() -// } -// } +#[pg_extern] +fn borrow_get_arr_data_ptr_nth_elem(arr: &FlatArray<'_, i32>, elem: i32) -> Option { + arr.nth(elem as usize).unwrap().into_option().copied() +} #[pg_extern] fn borrow_display_get_arr_nullbitmap(arr: &FlatArray<'_, i32>) -> String { @@ -299,12 +294,12 @@ mod tests { assert_eq!(len, Ok(Some(5))); } - // #[pg_test] - // fn borrow_test_get_arr_data_ptr_nth_elem() { - // let nth = - // Spi::get_one::("SELECT borrow_get_arr_data_ptr_nth_elem('{1,2,3,4,5}'::int[], 2)"); - // assert_eq!(nth, Ok(Some(3))); - // } + #[pg_test] + fn borrow_test_get_arr_data_ptr_nth_elem() { + let nth = + Spi::get_one::("SELECT borrow_get_arr_data_ptr_nth_elem('{1,2,3,4,5}'::int[], 2)"); + assert_eq!(nth, Ok(Some(3))); + } #[pg_test] fn borrow_test_display_get_arr_nullbitmap() -> Result<(), pgrx::spi::Error> { From b67f095bcd9d82a5bddadfdb0305741f9086f46d Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Tue, 1 Oct 2024 16:30:47 -0700 Subject: [PATCH 44/54] Use BorrowDatum::borrow_unchecked inside FlatArray --- pgrx/src/array.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pgrx/src/array.rs b/pgrx/src/array.rs index e0aa82033e..01259292f0 100644 --- a/pgrx/src/array.rs +++ b/pgrx/src/array.rs @@ -241,9 +241,7 @@ where // note that we do NOT offset when the value is a null! Some(Nullable::Null) } else { - let borrow = unsafe { - &*::point_from(self.data.byte_add(self.offset).cast_mut()) - }; + let borrow = unsafe { T::borrow_unchecked(self.data.add(self.offset)) }; // As we always have a borrow, we just ask Rust what the array element's size is self.offset += self.align.pad(mem::size_of_val(borrow)); Some(Nullable::Valid(borrow)) From c75dd1e365fc7ecf80940a03968d37ca9a1d20ea Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Sat, 28 Sep 2024 13:26:08 -0700 Subject: [PATCH 45/54] Move BorrowDatum interfaces to ptr::NonNull --- pgrx/src/array.rs | 13 ++++++++----- pgrx/src/text.rs | 8 +++++--- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/pgrx/src/array.rs b/pgrx/src/array.rs index 01259292f0..e059fffee0 100644 --- a/pgrx/src/array.rs +++ b/pgrx/src/array.rs @@ -77,7 +77,7 @@ where let nulls = raw.nulls_bitptr().map(|p| unsafe { bitslice::from_raw_parts(p, nelems).unwrap() }); - let data = raw.data_ptr(); + let data = unsafe { NonNull::new_unchecked(raw.data_ptr().cast_mut()) }; let arr = self; let index = 0; let offset = 0; @@ -159,10 +159,13 @@ where unsafe impl BorrowDatum for FlatArray<'_, T> { const PASS: layout::PassBy = layout::PassBy::Ref; - unsafe fn point_from(ptr: *mut u8) -> *mut Self { + unsafe fn point_from(ptr: ptr::NonNull) -> ptr::NonNull { unsafe { - let len = varlena::varsize_any(ptr.cast()) - mem::size_of::(); - ptr::slice_from_raw_parts_mut(ptr, len) as *mut Self + let len = + varlena::varsize_any(ptr.as_ptr().cast()) - mem::size_of::(); + ptr::NonNull::new_unchecked( + ptr::slice_from_raw_parts_mut(ptr.as_ptr(), len) as *mut Self + ) } } } @@ -212,7 +215,7 @@ where T: ?Sized + BorrowDatum, { arr: &'arr FlatArray<'arr, T>, - data: *const u8, + data: ptr::NonNull, nulls: Option<&'arr BitSlice>, nelems: usize, index: usize, diff --git a/pgrx/src/text.rs b/pgrx/src/text.rs index 7b6c505763..9981c19a43 100644 --- a/pgrx/src/text.rs +++ b/pgrx/src/text.rs @@ -115,10 +115,12 @@ impl Text { unsafe impl BorrowDatum for Text { const PASS: PassBy = PassBy::Ref; - unsafe fn point_from(ptr: *mut u8) -> *mut Self { + unsafe fn point_from(ptr: ptr::NonNull) -> ptr::NonNull { unsafe { - let len = varlena::varsize_any(ptr.cast()); - ptr::slice_from_raw_parts_mut(ptr, len) as *mut Text + let len = varlena::varsize_any(ptr.as_ptr().cast()); + ptr::NonNull::new_unchecked( + ptr::slice_from_raw_parts_mut(ptr.as_ptr(), len) as *mut Text + ) } } } From 82cb03886510b629b83982e951a5f56575dca4d3 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Sat, 28 Sep 2024 13:34:12 -0700 Subject: [PATCH 46/54] deny unsafe op in unsafe fn in all new code --- pgrx/src/array.rs | 1 + pgrx/src/text.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/pgrx/src/array.rs b/pgrx/src/array.rs index e059fffee0..bf39283476 100644 --- a/pgrx/src/array.rs +++ b/pgrx/src/array.rs @@ -9,6 +9,7 @@ //LICENSE Use of this source code is governed by the MIT license that can be found in the LICENSE file. #![allow(clippy::precedence)] #![allow(unused)] +#![deny(unsafe_op_in_unsafe_fn)] use crate::datum::{Array, BorrowDatum, Datum}; use crate::layout::{Align, Layout}; use crate::nullable::Nullable; diff --git a/pgrx/src/text.rs b/pgrx/src/text.rs index 9981c19a43..826c15eb7f 100644 --- a/pgrx/src/text.rs +++ b/pgrx/src/text.rs @@ -1,3 +1,4 @@ +#![deny(unsafe_op_in_unsafe_fn)] use crate::datum::BorrowDatum; use crate::layout::PassBy; use crate::pgrx_sql_entity_graph::metadata::{ From cc020d455920a97f56ade4fd947d0ddb2041ed45 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Sat, 28 Sep 2024 13:53:13 -0700 Subject: [PATCH 47/54] impl TextData --- pgrx/src/text.rs | 88 ++++++++++++++++++++++++++++-------------------- 1 file changed, 52 insertions(+), 36 deletions(-) diff --git a/pgrx/src/text.rs b/pgrx/src/text.rs index 826c15eb7f..2655496bd6 100644 --- a/pgrx/src/text.rs +++ b/pgrx/src/text.rs @@ -8,7 +8,8 @@ use crate::{pg_sys, varlena}; use alloc::borrow::Cow; use alloc::string::String; use core::borrow::Borrow; -use core::{ptr, slice, str}; +use core::ops::{Deref, DerefMut}; +use core::{ptr, str}; use bstr::{BStr, ByteSlice}; @@ -24,44 +25,33 @@ pub use core::str::{Utf8Chunks, Utf8Error}; #[repr(transparent)] pub struct Text([u8]); -// API decision: we could deref to TextData and move some fn to TextData so it can be returned from -// `split_at`, `trim`, etc., and thus preserve conveniences that [u8] doesn't have? +/// Data field of a TEXT varlena +#[repr(transparent)] +pub struct TextData([u8]); + +impl TextData { + /// Reborrow `&Text as `&BStr` + /// + /// We do not implement Deref to BStr or [u8] because we'd like to expose a more selective API. + /// Several fn that [u8] implements are implemented very differently on str, and we would like + /// the API of Text to "feel like" that of str in most cases. + fn as_bstr(&self) -> &BStr { + self.as_bytes().borrow() + } -impl Text { /// Obtain a reference to the Text's data as bytes pub fn as_bytes(&self) -> &[u8] { - let self_ptr = self as *const Text as *const pg_sys::varlena; - unsafe { - let len = varlena::varsize_any_exhdr(self_ptr); - let data = varlena::vardata_any(self_ptr); - - slice::from_raw_parts(data.cast::(), len) - } + &self.0 } - /// Obtain a mutable reference the Text's data as bytes + /// Obtain a mutable reference to the Text's data as bytes /// /// # Safety /// Like [`str::as_bytes_mut`], this can cause problems if you change Text in a way that /// your database is not specified to support, so the caller must assure that it remains in /// a valid encoding for the database. pub unsafe fn as_bytes_mut(&mut self) -> &mut [u8] { - let self_ptr = self as *mut Text as *mut pg_sys::varlena; - unsafe { - let len = varlena::varsize_any_exhdr(self_ptr); - let data = varlena::vardata_any(self_ptr); - - slice::from_raw_parts_mut(data.cast::().cast_mut(), len) - } - } - - /// Reborrow `&Text as `&BStr` - /// - /// We do not implement Deref to BStr or [u8] because we'd like to expose a more selective API. - /// Several fn that [u8] implements are implemented very differently on str, and we would like - /// the API of Text to "feel like" that of str in most cases. - fn as_bstr(&self) -> &BStr { - self.as_bytes().borrow() + &mut self.0 } /// Iterate over the UTF-8 characters of this Text @@ -81,22 +71,17 @@ impl Text { self.as_bytes().is_ascii() } - /// Is the varlena larger than its header? + /// Is this slice nonzero len? pub fn is_empty(&self) -> bool { self.as_bytes().is_empty() } /// Length of the data in bytes - pub fn len_data(&self) -> usize { + pub fn len(&self) -> usize { self.as_bytes().len() } - /// Length of the entire varlena in bytes - pub fn len_full(&self) -> usize { - self.0.len() - } - - /// Obtain a reference to the varlena data if it is a UTF-8 str + /// Obtain a reference to the data if it is a UTF-8 str pub fn to_str(&self) -> Result<&str, Utf8Error> { str::from_utf8(self.as_bytes()) } @@ -114,6 +99,37 @@ impl Text { } } +impl Text { + /// Length of the entire varlena in bytes + pub fn va_len(&self) -> usize { + self.0.len() + } +} + +impl Deref for Text { + type Target = TextData; + fn deref(&self) -> &Self::Target { + let self_ptr = self as *const Text as *const pg_sys::varlena; + unsafe { &*varlena_to_text_data(self_ptr.cast_mut()) } + } +} + +impl DerefMut for Text { + fn deref_mut(&mut self) -> &mut Self::Target { + let self_ptr = self as *mut Text as *mut pg_sys::varlena; + unsafe { &mut *varlena_to_text_data(self_ptr) } + } +} + +unsafe fn varlena_to_text_data(vptr: *mut pg_sys::varlena) -> *mut TextData { + unsafe { + let len = varlena::varsize_any_exhdr(vptr); + let data = varlena::vardata_any(vptr).cast_mut(); + + ptr::slice_from_raw_parts_mut(data.cast::(), len) as *mut TextData + } +} + unsafe impl BorrowDatum for Text { const PASS: PassBy = PassBy::Ref; unsafe fn point_from(ptr: ptr::NonNull) -> ptr::NonNull { From 347e1fb21dd15178d94238101f0a2912bc8a407a Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Sat, 28 Sep 2024 14:03:07 -0700 Subject: [PATCH 48/54] move some doc text --- pgrx/src/text.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pgrx/src/text.rs b/pgrx/src/text.rs index 2655496bd6..720e666921 100644 --- a/pgrx/src/text.rs +++ b/pgrx/src/text.rs @@ -21,11 +21,12 @@ pub use core::str::{Utf8Chunks, Utf8Error}; /// A Postgres string, AKA `TEXT`. /// /// This is a varlena: a reference to a variable-length header followed by a slice of bytes. -/// Usually this will be UTF-8, but this is not always strictly enforced by PostgreSQL. #[repr(transparent)] pub struct Text([u8]); /// Data field of a TEXT varlena +/// +/// Usually this will be UTF-8, but this is not always strictly enforced by PostgreSQL. #[repr(transparent)] pub struct TextData([u8]); From 05ed23d9e9409a43d5b89a8443035e07441b6971 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Sat, 28 Sep 2024 14:07:33 -0700 Subject: [PATCH 49/54] typo in fn name --- pgrx/src/text.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pgrx/src/text.rs b/pgrx/src/text.rs index 720e666921..03040af15f 100644 --- a/pgrx/src/text.rs +++ b/pgrx/src/text.rs @@ -102,7 +102,7 @@ impl TextData { impl Text { /// Length of the entire varlena in bytes - pub fn va_len(&self) -> usize { + pub fn vl_len(&self) -> usize { self.0.len() } } From d69231f3e3a0b102ef1920777a7e7b43aafe6d50 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Sat, 28 Sep 2024 14:56:30 -0700 Subject: [PATCH 50/54] Fixup docs a bit --- pgrx/src/array.rs | 35 ++++++++++++++++------------------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/pgrx/src/array.rs b/pgrx/src/array.rs index bf39283476..58b65ec02b 100644 --- a/pgrx/src/array.rs +++ b/pgrx/src/array.rs @@ -26,9 +26,11 @@ use core::{ffi, mem, slice}; mod port; -/// &FlatArray is akin to &ArrayType -/// -/// `pgrx::datum::Array` is essentially `&FlatArray` +/** `pg_sys::ArrayType` and its unsized varlena + +# Safety +`&FlatArray<'_, T>` assumes its tail is the remainder of a Postgres array of element `T`. +*/ #[repr(C)] #[derive(Debug)] pub struct FlatArray<'mcx, T: ?Sized> { @@ -48,7 +50,7 @@ where } } - /// Number of elements in the Array + /// Number of elements in the Array, including nulls /// /// Note that for many Arrays, this doesn't have a linear relationship with array byte-len. pub fn count(&self) -> usize { @@ -133,8 +135,7 @@ where } } - /** - Oxidized form of [ARR_NULLBITMAP(ArrayType*)][arr_nullbitmap] + /** Oxidized form of [ARR_NULLBITMAP(ArrayType*)][arr_nullbitmap] If this returns None, the array *cannot* have nulls. Note that unlike the `is_null: bool` that appears elsewhere, 1 is "valid" and 0 is "null". @@ -397,8 +398,7 @@ impl RawArray { } } - /** - A slice of the dimensions. + /** A slice describing the array's dimensions. Oxidized form of [ARR_DIMS(ArrayType*)][ARR_DIMS]. The length will be within 0..=[pg_sys::MAXDIM]. @@ -421,6 +421,7 @@ impl RawArray { } /// The flattened length of the array over every single element. + /// /// Includes all items, even the ones that might be null. /// /// # Panics @@ -461,8 +462,7 @@ impl RawArray { // This field is an "int32" in Postgres } - /** - Equivalent to [ARR_HASNULL(ArrayType*)][ARR_HASNULL]. + /** Equivalent to [ARR_HASNULL(ArrayType*)][ARR_HASNULL]. Note this means that it only asserts that there MIGHT be a null @@ -501,8 +501,7 @@ impl RawArray { } } - /** - Oxidized form of [ARR_NULLBITMAP(ArrayType*)][ARR_NULLBITMAP] + /** Oxidized form of [ARR_NULLBITMAP(ArrayType*)][ARR_NULLBITMAP] If this returns None, the array cannot have nulls. If this returns Some, it points to the bitslice that marks nulls in this array. @@ -521,8 +520,8 @@ impl RawArray { NonNull::new(ptr::slice_from_raw_parts_mut(self.nulls_mut_ptr(), len)) } - /** - The [bitvec] equivalent of [RawArray::nulls]. + /** The [bitvec] equivalent of [RawArray::nulls]. + If this returns `None`, the array cannot have nulls. If this returns `Some`, it points to the bitslice that marks nulls in this array. @@ -537,20 +536,18 @@ impl RawArray { NonNull::new(bitptr::bitslice_from_raw_parts_mut(self.nulls_mut_bitptr()?, self.len())) } - /** - Checks the array for any NULL values by assuming it is a proper varlena array, + /** Checks the array for any NULL values # Safety - * This requires every index is valid to read or correctly marked as null. + */ pub unsafe fn any_nulls(&self) -> bool { // SAFETY: Caller asserted safety conditions. unsafe { pg_sys::array_contains_nulls(self.ptr.as_ptr()) } } - /** - Oxidized form of [ARR_DATA_PTR(ArrayType*)][ARR_DATA_PTR] + /** Oxidized form of [ARR_DATA_PTR(ArrayType*)][ARR_DATA_PTR] # Safety From f92a7568bbea78e7118eb65fb495a3ee91f092a1 Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Sat, 28 Sep 2024 14:56:35 -0700 Subject: [PATCH 51/54] Add doc aliases --- pgrx/src/array.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pgrx/src/array.rs b/pgrx/src/array.rs index 58b65ec02b..046e241478 100644 --- a/pgrx/src/array.rs +++ b/pgrx/src/array.rs @@ -53,6 +53,8 @@ where /// Number of elements in the Array, including nulls /// /// Note that for many Arrays, this doesn't have a linear relationship with array byte-len. + #[doc(alias = "cardinality")] + #[doc(alias = "nelems")] pub fn count(&self) -> usize { self.as_raw().len() } @@ -74,6 +76,7 @@ where T: ?Sized + BorrowDatum, { /// Iterate the array + #[doc(alias = "unnest")] pub fn iter(&self) -> ArrayIter<'_, T> { let nelems = self.count(); let raw = self.as_raw(); From e2ba9ac5c68e9182d4a0f54633de0c9ab05340df Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Sat, 28 Sep 2024 16:11:49 -0700 Subject: [PATCH 52/54] Add Iterator subtraits --- pgrx-tests/src/tests/array_borrowed.rs | 2 +- pgrx/src/array.rs | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/pgrx-tests/src/tests/array_borrowed.rs b/pgrx-tests/src/tests/array_borrowed.rs index eb7b874bba..63e97aaca5 100644 --- a/pgrx-tests/src/tests/array_borrowed.rs +++ b/pgrx-tests/src/tests/array_borrowed.rs @@ -22,7 +22,7 @@ fn borrow_sum_array_i32(values: &FlatArray<'_, i32>) -> i32 { // we implement it this way so we can trap an overflow (as we have a test for this) and // catch it correctly in both --debug and --release modes let mut sum = 0_i32; - for v in values.iter() { + for v in values { let v = v.into_option().copied().unwrap_or(0); let (val, overflow) = sum.overflowing_add(v); if overflow { diff --git a/pgrx/src/array.rs b/pgrx/src/array.rs index 046e241478..0aa9481e4e 100644 --- a/pgrx/src/array.rs +++ b/pgrx/src/array.rs @@ -20,6 +20,7 @@ use crate::toast::{Toast, Toasty}; use crate::{layout, pg_sys, varlena}; use bitvec::ptr::{self as bitptr, BitPtr, BitPtrError, Const, Mut}; use bitvec::slice::{self as bitslice, BitSlice}; +use core::iter::{ExactSizeIterator, FusedIterator}; use core::marker::PhantomData; use core::ptr::{self, NonNull}; use core::{ffi, mem, slice}; @@ -257,6 +258,20 @@ where } } +impl<'arr, 'mcx, T> IntoIterator for &'arr FlatArray<'mcx, T> +where + T: ?Sized + BorrowDatum, +{ + type IntoIter = ArrayIter<'arr, T>; + type Item = Nullable<&'arr T>; + fn into_iter(self) -> Self::IntoIter { + self.iter() + } +} + +impl<'arr, T> ExactSizeIterator for ArrayIter<'arr, T> where T: ?Sized + BorrowDatum {} +impl<'arr, T> FusedIterator for ArrayIter<'arr, T> where T: ?Sized + BorrowDatum {} + /** An aligned, dereferenceable `NonNull` with low-level accessors. From 7bce001d42c9a22873caffc76d80a61cb0620c0c Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Tue, 1 Oct 2024 16:32:11 -0700 Subject: [PATCH 53/54] publicize ArrayIter --- pgrx/src/array.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pgrx/src/array.rs b/pgrx/src/array.rs index 0aa9481e4e..26e4444a32 100644 --- a/pgrx/src/array.rs +++ b/pgrx/src/array.rs @@ -216,7 +216,7 @@ impl<'arr> Dimensions<'arr> { /// Iterator for arrays #[derive(Clone)] -struct ArrayIter<'arr, T> +pub struct ArrayIter<'arr, T> where T: ?Sized + BorrowDatum, { From aaa6ed2e528c7c23dee4d92295e4f767253f626b Mon Sep 17 00:00:00 2001 From: Jubilee Young Date: Tue, 1 Oct 2024 16:46:26 -0700 Subject: [PATCH 54/54] make some cstrings prettier --- pgrx-tests/src/tests/array_borrowed.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pgrx-tests/src/tests/array_borrowed.rs b/pgrx-tests/src/tests/array_borrowed.rs index 63e97aaca5..dc59bd47bf 100644 --- a/pgrx-tests/src/tests/array_borrowed.rs +++ b/pgrx-tests/src/tests/array_borrowed.rs @@ -156,13 +156,13 @@ fn borrow_validate_cstring_array( assert_eq!( a.iter().map(|v| v.into_option()).collect::>(), vec![ - Some(CStr::from_bytes_with_nul(b"one\0")?), - Some(CStr::from_bytes_with_nul(b"two\0")?), + Some(c"one"), + Some(c"two"), None, - Some(CStr::from_bytes_with_nul(b"four\0")?), - Some(CStr::from_bytes_with_nul(b"five\0")?), + Some(c"four"), + Some(c"five"), None, - Some(CStr::from_bytes_with_nul(b"seven\0")?), + Some(c"seven"), None, None ]