address feedback

cargo-pgrx/src/command/install.rs

@@ -207,7 +207,7 @@ pub(crate) fn install_extension(
         // process which will mash up all pointers in the .TEXT segment.
         // this simulate linux's install(1) behavior
         if dest.exists() {
-            std::fs::remove_file(&dest)
+            fs::remove_file(&dest)
                 .wrap_err_with(|| format!("unable to remove existing file {}", dest.display()))?;
         }
 
@@ -267,19 +267,19 @@ fn copy_file(
 
     if do_filter {
         // we want to filter the contents of the file we're to copy
-        let input = std::fs::read_to_string(src)
+        let input = fs::read_to_string(src)
             .wrap_err_with(|| format!("failed to read `{}`", src.display()))?;
         let mut input = filter_contents(package_manifest_path, input)?;
 
         if src.display().to_string().ends_with(".control") {
             input = filter_out_fields_in_control(pg_config, input)?;
         }
 
-        std::fs::write(&dest, input).wrap_err_with(|| {
+        fs::write(&dest, input).wrap_err_with(|| {
             format!("failed writing `{}` to `{}`", src.display(), dest.display())
         })?;
     } else {
-        std::fs::copy(src, &dest).wrap_err_with(|| {
+        fs::copy(src, &dest).wrap_err_with(|| {
             format!("failed copying `{}` to `{}`", src.display(), dest.display())
         })?;
     }
@@ -418,7 +418,7 @@ fn copy_sql_files(
 
 #[tracing::instrument(level = "error", skip_all)]
 pub(crate) fn find_library_file(
-    manifest: &cargo_toml::Manifest,
+    manifest: &Manifest,
     build_command_messages: &Vec<cargo_metadata::Message>,
 ) -> eyre::Result<PathBuf> {
     let target_name = manifest.target_name()?;

pgrx-examples/bad_ideas/src/lib.rs

@@ -53,8 +53,8 @@ fn warning(s: &str) -> bool {
 }
 
 #[pg_extern]
-fn exec(
-    command: &str,
+fn exec<'a>(
+    command: &'a str,
     args: default!(Vec<Option<String>>, "ARRAY[]::text[]"),
 ) -> TableIterator<'static, (name!(status, Option<i32>), name!(stdout, String))> {
     let mut command = &mut Command::new(command);

pgrx-tests/src/tests/derive_pgtype_lifetimes.rs

@@ -12,7 +12,7 @@
 use pgrx::prelude::*;
 use serde::*;
 
-fn foo(_s: Vec<Option<&str>>) {
+fn foo<'a>(_s: Vec<Option<&'a str>>) {
     unimplemented!()
 }
 

pgrx-tests/src/tests/lifetime_tests.rs

@@ -21,7 +21,7 @@ pub struct CustomType<'s> {
 fn type_with_lifetime<'s>(_value: Option<CustomType<'s>>) {}
 
 #[pg_extern]
-fn type_ref_with_lifetime(_value: &str) {}
+fn type_ref_with_lifetime<'a>(_value: &'a str) {}
 
 #[pg_extern]
 fn returns_lifetime() -> Option<CustomType<'static>> {

pgrx/src/bgworkers.rs

@@ -467,7 +467,7 @@ impl BackgroundWorkerBuilder {
     /// `startup` allows specifying shared memory initialization startup hook. Ignored
     /// if [`BackgroundWorkerBuilder::load_dynamic`] is used.
     pub fn enable_shmem_access(mut self, startup: Option<unsafe extern "C" fn()>) -> Self {
-        self.bgw_flags |= BGWflags::BGWORKER_SHMEM_ACCESS;
+        self.bgw_flags = self.bgw_flags | BGWflags::BGWORKER_SHMEM_ACCESS;
         self.shared_memory_startup_fn = startup;
         self
     }

pgrx/src/datum/array.rs

@@ -186,7 +186,7 @@ impl<'mcx, T: UnboxDatum> Array<'mcx, T> {
 
     #[allow(clippy::option_option)]
     #[inline]
-    pub fn get(&self, index: usize) -> Option<Option<T::As<'_>>> {
+    pub fn get<'arr>(&'arr self, index: usize) -> Option<Option<T::As<'arr>>> {
         let Some(is_null) = self.null_slice.get(index) else { return None };
         if is_null {
             return Some(None);
@@ -222,7 +222,11 @@ impl<'mcx, T: UnboxDatum> Array<'mcx, T> {
     /// # Safety
     /// This assumes the pointer is to a valid element of that type.
     #[inline]
-    unsafe fn bring_it_back_now(&self, ptr: *const u8, is_null: bool) -> Option<T::As<'_>> {
+    unsafe fn bring_it_back_now<'arr>(
+        &'arr self,
+        ptr: *const u8,
+        is_null: bool,
+    ) -> Option<T::As<'arr>> {
         match is_null {
             true => None,
             false => unsafe { self.slide_impl.bring_it_back_now(self, ptr) },
@@ -399,9 +403,9 @@ mod casper {
         ///
         /// This function is unsafe as it cannot guarantee that `ptr` points to the proper bytes
         /// that represent a `T`, or even that it belongs to `array`.  Both of which must be true
-        unsafe fn bring_it_back_now<'arr>(
+        unsafe fn bring_it_back_now<'arr, 'mcx>(
             &self,
-            array: &'arr Array<'_, T>,
+            array: &'arr Array<'mcx, T>,
             ptr: *const u8,
         ) -> Option<T::As<'arr>>;
 
@@ -435,9 +439,9 @@ mod casper {
     pub(super) struct FixedSizeByVal<const N: usize>;
     impl<T: UnboxDatum, const N: usize> ChaChaSlide<T> for FixedSizeByVal<N> {
         #[inline(always)]
-        unsafe fn bring_it_back_now<'arr>(
+        unsafe fn bring_it_back_now<'arr, 'mcx>(
             &self,
-            _array: &'arr Array<'_, T>,
+            _array: &'arr Array<'mcx, T>,
             ptr: *const u8,
         ) -> Option<T::As<'arr>> {
             // This branch is optimized away (because `N` is constant).
@@ -465,10 +469,10 @@ mod casper {
     }
     impl<T: UnboxDatum> ChaChaSlide<T> for PassByVarlena {
         #[inline]
-        unsafe fn bring_it_back_now<'arr>(
+        unsafe fn bring_it_back_now<'arr, 'mcx>(
             &self,
             // May need this array param for MemCx reasons?
-            _array: &'arr Array<'_, T>,
+            _array: &'arr Array<'mcx, T>,
             ptr: *const u8,
         ) -> Option<T::As<'arr>> {
             let datum = pg_sys::Datum::from(ptr);
@@ -490,9 +494,9 @@ mod casper {
     pub(super) struct PassByCStr;
     impl<T: UnboxDatum> ChaChaSlide<T> for PassByCStr {
         #[inline]
-        unsafe fn bring_it_back_now<'arr>(
+        unsafe fn bring_it_back_now<'arr, 'mcx>(
             &self,
-            _array: &'arr Array<'_, T>,
+            _array: &'arr Array<'mcx, T>,
             ptr: *const u8,
         ) -> Option<T::As<'arr>> {
             let datum = pg_sys::Datum::from(ptr);
@@ -515,9 +519,9 @@ mod casper {
 
     impl<T: UnboxDatum> ChaChaSlide<T> for PassByFixed {
         #[inline]
-        unsafe fn bring_it_back_now<'arr>(
+        unsafe fn bring_it_back_now<'arr, 'mcx>(
             &self,
-            _array: &'arr Array<'_, T>,
+            _array: &'arr Array<'mcx, T>,
             ptr: *const u8,
         ) -> Option<T::As<'arr>> {
             let datum = pg_sys::Datum::from(ptr);
@@ -563,7 +567,7 @@ impl<'mcx, T: UnboxDatum> VariadicArray<'mcx, T> {
 
     #[allow(clippy::option_option)]
     #[inline]
-    pub fn get(&self, i: usize) -> Option<Option<<T as UnboxDatum>::As<'_>>> {
+    pub fn get<'arr>(&'arr self, i: usize) -> Option<Option<<T as UnboxDatum>::As<'arr>>> {
         self.0.get(i)
     }
 }
@@ -908,7 +912,9 @@ where
     #[allow(clippy::get_first)] // https://github.com/pgcentralfoundation/pgrx/issues/1363
     fn composite_type_oid(&self) -> Option<pg_sys::Oid> {
         // the composite type oid for a vec of composite types is the array type of the base composite type
-        self.first()
+        // the use of first() would have presented a false certainty here: it's not actually relevant that it be the first.
+        #[allow(clippy::get_first)]
+        self.get(0)
             .and_then(|v| v.composite_type_oid().map(|oid| unsafe { pg_sys::get_array_type(oid) }))
     }
 

pgrx/src/datum/date.rs

@@ -151,8 +151,8 @@ impl Date {
     /// This function will panic, aborting the current transaction, if any part is out-of-bounds.
     pub fn new_unchecked(year: isize, month: u8, day: u8) -> Self {
         let year: i32 = year.try_into().expect("invalid year");
-        let month: i32 = month.into();
-        let day: i32 = day.into();
+        let month: i32 = month.try_into().expect("invalid month");
+        let day: i32 = day.try_into().expect("invalid day");
 
         unsafe {
             direct_function_call(
@@ -245,7 +245,7 @@ impl Date {
     #[inline]
     pub fn to_posix_time(&self) -> libc::time_t {
         let secs_per_day: libc::time_t =
-            pg_sys::SECS_PER_DAY.into();
+            pg_sys::SECS_PER_DAY.try_into().expect("couldn't fit time into time_t");
         libc::time_t::from(self.to_unix_epoch_days()) * secs_per_day
     }
 

pgrx/src/datum/datetime_support/ctor.rs

@@ -71,7 +71,7 @@ pub fn current_timestamp(precision: TimestampPrecision) -> TimestampWithTimeZone
 
 /// implements LOCALTIMESTAMP, LOCALTIMESTAMP(n)
 pub fn local_timestamp(precision: TimestampPrecision) -> Timestamp {
-    unsafe { pg_sys::GetSQLLocalTimestamp(precision.into()).into() }
+    unsafe { pg_sys::GetSQLLocalTimestamp(precision.into()).try_into().unwrap() }
 }
 
 /// Returns the current time as String (changes during statement execution)
@@ -113,8 +113,8 @@ pub fn to_timestamp(epoch_seconds: f64) -> TimestampWithTimeZone {
 /// SELECT date_bin('15 minutes', TIMESTAMP '2020-02-11 15:44:17', TIMESTAMP '2001-01-01 00:02:30');
 /// Result: 2020-02-11 15:32:30
 /// ```
-// BUG!!!  This will NEVER compile because of a spelling mistake:
-//  #[cfg(any(feature = "pg14", feature = "pg15"))]
+///
+/// TODO: See https://github.com/pgcentralfoundation/pgrx/pull/1414
 #[cfg(any(features = "pg14", features = "pg15"))]
 pub fn date_bin(
     stride: crate::datum::interval::Interval,

pgrx/src/datum/time.rs

@@ -141,8 +141,8 @@ impl Time {
     ///
     /// This function panics if any part is out-of-bounds
     pub fn new_unchecked(hour: u8, minute: u8, second: f64) -> Time {
-        let hour: i32 = hour.into();
-        let minute: i32 = minute.into();
+        let hour: i32 = hour.try_into().expect("invalid hour");
+        let minute: i32 = minute.try_into().expect("invalid minute");
 
         unsafe {
             direct_function_call(

pgrx/src/datum/uuid.rs

@@ -122,13 +122,13 @@ impl std::fmt::Display for Uuid {
     }
 }
 
-impl std::fmt::LowerHex for Uuid {
+impl<'a> std::fmt::LowerHex for Uuid {
     fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> {
         self.format(f, UuidFormatCase::Lowercase)
     }
 }
 
-impl std::fmt::UpperHex for Uuid {
+impl<'a> std::fmt::UpperHex for Uuid {
     fn fmt(&self, f: &mut std::fmt::Formatter) -> Result<(), std::fmt::Error> {
         self.format(f, UuidFormatCase::Uppercase)
     }

pgrx/src/lwlock.rs

@@ -96,7 +96,7 @@ impl<T> fmt::Debug for PgLwLockInner<T> {
     }
 }
 
-impl<T> PgLwLockInner<T> {
+impl<'a, T> PgLwLockInner<T> {
     fn new(name: &'static str, data: *mut T) -> Self {
         unsafe {
             let lock = alloc::ffi::CString::new(name).expect("CString::new failed");

pgrx/src/memcx.rs

@@ -42,6 +42,6 @@ where
     F: for<'clos> FnOnce(&'clos MemCx<'curr>) -> T,
 {
     let memcx = unsafe { MemCx::from_ptr(pg_sys::CurrentMemoryContext) };
-    
+
     f(&memcx)
 }

pgrx/src/spi/tuple.rs

@@ -330,7 +330,6 @@ impl<'conn> SpiHeapTupleData<'conn> {
         unsafe {
             // SAFETY:  we know tupdesc is not null
             let natts = (*tupdesc).natts;
-            // BUG: usize::try_from will always succeed here, and is not needed.
             data.entries.reserve(usize::try_from(natts as usize).unwrap_or_default());
             for i in 1..=natts {
                 let mut is_null = false;

pgrx/src/trigger_support/pg_trigger_level.rs

@@ -37,9 +37,9 @@ impl From<TriggerEvent> for PgTriggerLevel {
 
 impl Display for PgTriggerLevel {
     fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
-        match self {
-            PgTriggerLevel::Row => f.write_str("ROW"),
-            PgTriggerLevel::Statement => f.write_str("STATEMENT"),
-        }
+        f.write_str(match self {
+            PgTriggerLevel::Row => "ROW",
+            PgTriggerLevel::Statement => "STATEMENT",
+        })
     }
 }

pgrx/src/trigger_support/pg_trigger_option.rs

@@ -45,11 +45,11 @@ impl TryFrom<TriggerEvent> for PgTriggerOperation {
 
 impl Display for PgTriggerOperation {
     fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
-        match self {
-            PgTriggerOperation::Insert => f.write_str("INSERT"),
-            PgTriggerOperation::Update => f.write_str("UPDATE"),
-            PgTriggerOperation::Delete => f.write_str("DELETE"),
-            PgTriggerOperation::Truncate => f.write_str("TRUNCATE"),
-        }
+        f.write_str(match self {
+            PgTriggerOperation::Insert => "INSERT",
+            PgTriggerOperation::Update => "UPDATE",
+            PgTriggerOperation::Delete => "DELETE",
+            PgTriggerOperation::Truncate => "TRUNCATE",
+        })
     }
 }

pgrx/src/trigger_support/pg_trigger_when.rs

@@ -42,10 +42,10 @@ impl TryFrom<TriggerEvent> for PgTriggerWhen {
 
 impl Display for PgTriggerWhen {
     fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
-        match self {
-            PgTriggerWhen::Before => f.write_str("BEFORE"),
-            PgTriggerWhen::After => f.write_str("AFTER"),
-            PgTriggerWhen::InsteadOf => f.write_str("INSTEAD OF"),
-        }
+        f.write_str(match self {
+            PgTriggerWhen::Before => "BEFORE",
+            PgTriggerWhen::After => "AFTER",
+            PgTriggerWhen::InsteadOf => "INSTEAD OF",
+        })
     }
 }