Skip to content

Commit

Permalink
Unleash clippy --fix on pgrx::datum
Browse files Browse the repository at this point in the history
Reviewed and backed out some of the less-desirable changes,
the rest seem to be pretty good.
  • Loading branch information
workingjubilee committed Oct 28, 2023
1 parent 2f15665 commit c20d317
Show file tree
Hide file tree
Showing 10 changed files with 48 additions and 96 deletions.
5 changes: 2 additions & 3 deletions pgrx/src/datum/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ impl NullKind<'_> {
// Note this flips the bit:
// Postgres nullbitmaps are 1 for "valid" and 0 for "null"
Self::Bits(b1) => b1.get(index).map(|b| !b),
Self::Strict(len) => index.lt(len).then(|| false),
Self::Strict(len) => index.lt(len).then_some(false),
}
}

Expand Down Expand Up @@ -848,8 +848,7 @@ where
fn composite_type_oid(&self) -> Option<Oid> {
// the composite type oid for a vec of composite types is the array type of the base composite type
self.get(0)
.map(|v| v.composite_type_oid().map(|oid| unsafe { pg_sys::get_array_type(oid) }))
.flatten()
.and_then(|v| v.composite_type_oid().map(|oid| unsafe { pg_sys::get_array_type(oid) }))
}

#[inline]
Expand Down
7 changes: 3 additions & 4 deletions pgrx/src/datum/datetime_support/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -583,16 +583,15 @@ pub fn get_timezone_offset<Tz: AsRef<str>>(zone: Tz) -> Result<i32, DateTimeConv
unsafe {
let mut tz = 0;
let tzname = alloc::ffi::CString::new(zone.as_ref()).unwrap();
let lowzone;
let tztype: u32;

let mut val = 0;
let mut tzp: *mut pg_tz = 0 as _;

/* DecodeTimezoneAbbrev requires lowercase input */
lowzone =
let lowzone =
pg_sys::downcase_truncate_identifier(tzname.as_ptr(), zone.as_ref().len() as _, false);

tztype = {
let tztype: u32 = {
#[cfg(any(
feature = "pg11",
feature = "pg12",
Expand Down
5 changes: 2 additions & 3 deletions pgrx/src/datum/internal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,8 @@ impl Internal {
{
let ptr = self.0.get_or_insert_with(|| {
let result = f();
let datum =
PgMemoryContexts::CurrentMemoryContext.leak_and_drop_on_delete(result).into();
datum

PgMemoryContexts::CurrentMemoryContext.leak_and_drop_on_delete(result).into()
});
&mut *(ptr.cast_mut_ptr::<T>())
}
Expand Down
6 changes: 2 additions & 4 deletions pgrx/src/datum/interval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,8 @@ impl Interval {
if days > 0 || micros > 0 {
return Err(IntervalConversionError::MismatchedSigns);
}
} else if months > 0 {
if days < 0 || micros < 0 {
return Err(IntervalConversionError::MismatchedSigns);
}
} else if months > 0 && (days < 0 || micros < 0) {
return Err(IntervalConversionError::MismatchedSigns);
}

Ok(Interval(pg_sys::Interval { time: micros, day: days, month: months }))
Expand Down
102 changes: 31 additions & 71 deletions pgrx/src/datum/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,13 +162,12 @@ pub trait WithTypeIds {
Self: 'static,
{
let rust = core::any::type_name::<Self>();
assert_eq!(
assert!(
set.insert(RustSqlMapping {
sql: single_sql.clone(),
rust: rust.to_string(),
id: *Self::ITEM_ID,
}),
true,
"Cannot set mapping of `{}` twice, was already `{}`.",
rust,
single_sql,
Expand Down Expand Up @@ -248,88 +247,68 @@ impl<T: 'static> WithSizedTypeIds<T> {

if let Some(id) = *WithSizedTypeIds::<T>::PG_BOX_ID {
let rust = core::any::type_name::<crate::PgBox<T>>().to_string();
assert_eq!(
map.insert(RustSqlMapping {
sql: single_sql.clone(),
rust: rust.to_string(),
id: id,
}),
true,
assert!(
map.insert(RustSqlMapping { sql: single_sql.clone(), rust: rust.to_string(), id }),
"Cannot map `{}` twice.",
rust,
);
}

if let Some(id) = *WithSizedTypeIds::<T>::PG_BOX_OPTION_ID {
let rust = core::any::type_name::<crate::PgBox<Option<T>>>().to_string();
assert_eq!(
map.insert(RustSqlMapping {
sql: single_sql.clone(),
rust: rust.to_string(),
id: id,
}),
true,
assert!(
map.insert(RustSqlMapping { sql: single_sql.clone(), rust: rust.to_string(), id }),
"Cannot map `{}` twice.",
rust,
);
}

if let Some(id) = *WithSizedTypeIds::<T>::PG_BOX_VEC_ID {
let rust = core::any::type_name::<crate::PgBox<Vec<T>>>().to_string();
assert_eq!(
map.insert(RustSqlMapping { sql: set_sql.clone(), rust: rust.to_string(), id: id }),
true,
assert!(
map.insert(RustSqlMapping { sql: set_sql.clone(), rust: rust.to_string(), id }),
"Cannot map `{}` twice.",
rust,
);
}

if let Some(id) = *WithSizedTypeIds::<T>::OPTION_ID {
let rust = core::any::type_name::<Option<T>>().to_string();
assert_eq!(
map.insert(RustSqlMapping {
sql: single_sql.clone(),
rust: rust.to_string(),
id: id,
}),
true,
assert!(
map.insert(RustSqlMapping { sql: single_sql.clone(), rust: rust.to_string(), id }),
"Cannot map `{}` twice.",
rust,
);
}

if let Some(id) = *WithSizedTypeIds::<T>::VEC_ID {
let rust = core::any::type_name::<T>().to_string();
assert_eq!(
map.insert(RustSqlMapping { sql: set_sql.clone(), rust: rust.to_string(), id: id }),
true,
assert!(
map.insert(RustSqlMapping { sql: set_sql.clone(), rust: rust.to_string(), id }),
"Cannot map `{}` twice.",
rust,
);
}
if let Some(id) = *WithSizedTypeIds::<T>::VEC_OPTION_ID {
let rust = core::any::type_name::<Vec<Option<T>>>();
assert_eq!(
map.insert(RustSqlMapping { sql: set_sql.clone(), rust: rust.to_string(), id: id }),
true,
assert!(
map.insert(RustSqlMapping { sql: set_sql.clone(), rust: rust.to_string(), id }),
"Cannot map `{}` twice.",
rust,
);
}
if let Some(id) = *WithSizedTypeIds::<T>::OPTION_VEC_ID {
let rust = core::any::type_name::<Option<Vec<T>>>();
assert_eq!(
map.insert(RustSqlMapping { sql: set_sql.clone(), rust: rust.to_string(), id: id }),
true,
assert!(
map.insert(RustSqlMapping { sql: set_sql.clone(), rust: rust.to_string(), id }),
"Cannot map `{}` twice.",
rust,
);
}
if let Some(id) = *WithSizedTypeIds::<T>::OPTION_VEC_OPTION_ID {
let rust = core::any::type_name::<Option<Vec<Option<T>>>>();
assert_eq!(
map.insert(RustSqlMapping { sql: set_sql.clone(), rust: rust.to_string(), id: id }),
true,
assert!(
map.insert(RustSqlMapping { sql: set_sql.clone(), rust: rust.to_string(), id }),
"Cannot map `{}` twice.",
rust,
);
Expand Down Expand Up @@ -388,37 +367,33 @@ impl<T: FromDatum + 'static> WithArrayTypeIds<T> {

if let Some(id) = *WithArrayTypeIds::<T>::ARRAY_ID {
let rust = core::any::type_name::<Array<T>>().to_string();
assert_eq!(
map.insert(RustSqlMapping { sql: set_sql.clone(), rust: rust.to_string(), id: id }),
true,
assert!(
map.insert(RustSqlMapping { sql: set_sql.clone(), rust: rust.to_string(), id }),
"Cannot map `{}` twice.",
rust,
);
}
if let Some(id) = *WithArrayTypeIds::<T>::OPTION_ARRAY_ID {
let rust = core::any::type_name::<Option<Array<T>>>().to_string();
assert_eq!(
map.insert(RustSqlMapping { sql: set_sql.clone(), rust: rust.to_string(), id: id }),
true,
assert!(
map.insert(RustSqlMapping { sql: set_sql.clone(), rust: rust.to_string(), id }),
"Cannot map `{}` twice.",
rust,
);
}

if let Some(id) = *WithArrayTypeIds::<T>::VARIADICARRAY_ID {
let rust = core::any::type_name::<VariadicArray<T>>().to_string();
assert_eq!(
map.insert(RustSqlMapping { sql: set_sql.clone(), rust: rust.to_string(), id: id }),
true,
assert!(
map.insert(RustSqlMapping { sql: set_sql.clone(), rust: rust.to_string(), id }),
"Cannot map `{}` twice.",
rust,
);
}
if let Some(id) = *WithArrayTypeIds::<T>::OPTION_VARIADICARRAY_ID {
let rust = core::any::type_name::<Option<VariadicArray<T>>>().to_string();
assert_eq!(
map.insert(RustSqlMapping { sql: set_sql.clone(), rust: rust.to_string(), id: id }),
true,
assert!(
map.insert(RustSqlMapping { sql: set_sql.clone(), rust: rust.to_string(), id }),
"Cannot map `{}` twice.",
rust,
);
Expand Down Expand Up @@ -476,40 +451,25 @@ impl<T: Copy + 'static> WithVarlenaTypeIds<T> {
) {
if let Some(id) = *WithVarlenaTypeIds::<T>::VARLENA_ID {
let rust = core::any::type_name::<PgVarlena<T>>();
assert_eq!(
map.insert(RustSqlMapping {
sql: single_sql.clone(),
rust: rust.to_string(),
id: id,
}),
true,
assert!(
map.insert(RustSqlMapping { sql: single_sql.clone(), rust: rust.to_string(), id }),
"Cannot map `{}` twice.",
rust,
);
}

if let Some(id) = *WithVarlenaTypeIds::<T>::PG_BOX_VARLENA_ID {
let rust = core::any::type_name::<PgBox<PgVarlena<T>>>().to_string();
assert_eq!(
map.insert(RustSqlMapping {
sql: single_sql.clone(),
rust: rust.to_string(),
id: id,
}),
true,
assert!(
map.insert(RustSqlMapping { sql: single_sql.clone(), rust: rust.to_string(), id }),
"Cannot map `{}` twice.",
rust,
);
}
if let Some(id) = *WithVarlenaTypeIds::<T>::OPTION_VARLENA_ID {
let rust = core::any::type_name::<Option<PgVarlena<T>>>().to_string();
assert_eq!(
map.insert(RustSqlMapping {
sql: single_sql.clone(),
rust: rust.to_string(),
id: id,
}),
true,
assert!(
map.insert(RustSqlMapping { sql: single_sql.clone(), rust: rust.to_string(), id }),
"Cannot map `{}` twice.",
rust,
);
Expand Down
2 changes: 1 addition & 1 deletion pgrx/src/datum/range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ where
let lower = RangeBound::from_pg(lower_bound);
let upper = RangeBound::from_pg(upper_bound);

if std::ptr::eq(ptr, range_type.cast()) == false {
if !std::ptr::eq(ptr, range_type.cast()) {
// SAFETY: range_type was allocated by Postgres in the call to
// pg_detoast_datum above, so we know it's a valid pointer and needs to be freed
pg_sys::pfree(range_type.cast());
Expand Down
2 changes: 1 addition & 1 deletion pgrx/src/datum/time_stamp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ impl TryFrom<pg_sys::Datum> for Timestamp {
type Error = TryFromIntError;

fn try_from(datum: pg_sys::Datum) -> Result<Self, Self::Error> {
pg_sys::Timestamp::try_from(datum.value() as isize).map(|d| Timestamp(d))
pg_sys::Timestamp::try_from(datum.value() as isize).map(Timestamp)
}
}

Expand Down
2 changes: 1 addition & 1 deletion pgrx/src/datum/time_stamp_with_timezone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ impl TimestampWithTimeZone {
PgTryBuilder::new(|| unsafe {
Ok(direct_function_call(
pg_sys::timestamptz_zone,
&[timezone_datum, self.clone().into_datum()],
&[timezone_datum, (*self).into_datum()],
)
.unwrap())
})
Expand Down
9 changes: 3 additions & 6 deletions pgrx/src/datum/time_with_timezone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ impl From<TimeWithTimeZone> for (pg_sys::TimeADT, i32) {
impl From<(pg_sys::TimeADT, i32)> for TimeWithTimeZone {
#[inline]
fn from(value: (pg_sys::TimeADT, i32)) -> Self {
TimeWithTimeZone { 0: pg_sys::TimeTzADT { time: value.0, zone: value.1 } }
TimeWithTimeZone(pg_sys::TimeTzADT { time: value.0, zone: value.1 })
}
}

Expand Down Expand Up @@ -267,11 +267,8 @@ impl TimeWithTimeZone {
) -> Result<Self, DateTimeConversionError> {
let timezone_datum = timezone.as_ref().into_datum();
PgTryBuilder::new(|| unsafe {
Ok(direct_function_call(
pg_sys::timetz_zone,
&[timezone_datum, self.clone().into_datum()],
)
.unwrap())
Ok(direct_function_call(pg_sys::timetz_zone, &[timezone_datum, (*self).into_datum()])
.unwrap())
})
.catch_when(PgSqlErrorCode::ERRCODE_DATETIME_FIELD_OVERFLOW, |_| {
Err(DateTimeConversionError::FieldOverflow)
Expand Down
4 changes: 2 additions & 2 deletions pgrx/src/datum/varlena.rs
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ where
serialized.push_bytes(&[0u8; pg_sys::VARHDRSZ]); // reserve space for the header
serde_cbor::to_writer(&mut serialized, &input).expect("failed to encode as CBOR");

let size = serialized.len() as usize;
let size = serialized.len();
let varlena = serialized.into_char_ptr();
unsafe {
set_varsize(varlena as *mut pg_sys::varlena, size as i32);
Expand Down Expand Up @@ -475,7 +475,7 @@ where
serialized.push_bytes(&[0u8; pg_sys::VARHDRSZ]); // reserve space for the header
serde_json::to_writer(&mut serialized, &input).expect("failed to encode as JSON");

let size = serialized.len() as usize;
let size = serialized.len();
let varlena = serialized.into_char_ptr();
unsafe {
set_varsize(varlena as *mut pg_sys::varlena, size as i32);
Expand Down

0 comments on commit c20d317

Please sign in to comment.