diff --git a/pgrx-macros/src/lib.rs b/pgrx-macros/src/lib.rs index b7c87350ad..3b0cd07ad7 100644 --- a/pgrx-macros/src/lib.rs +++ b/pgrx-macros/src/lib.rs @@ -1039,7 +1039,7 @@ Unlike some derives, this does not implement a "real" Rust trait, thus PostgresHash cannot be used in trait bounds, nor can it be manually implemented. */ #[proc_macro_derive(PostgresHash, attributes(pgrx))] -pub fn postgres_hash(input: TokenStream) -> TokenStream { +pub fn derive_postgres_hash(input: TokenStream) -> TokenStream { let ast = parse_macro_input!(input as syn::DeriveInput); deriving_postgres_hash(ast).unwrap_or_else(syn::Error::into_compile_error).into() } diff --git a/pgrx-macros/src/operators.rs b/pgrx-macros/src/operators.rs index 07c5f08864..f0c3834075 100644 --- a/pgrx-macros/src/operators.rs +++ b/pgrx-macros/src/operators.rs @@ -95,9 +95,6 @@ pub(crate) fn deriving_postgres_hash(ast: DeriveInput) -> syn::Result proc_macro2::TokenStream { let pg_name = Ident::new(&format!("{}_eq", name).to_lowercase(), name.span()); quote! { - #[doc(hidden)] - impl ::pgrx::deriving::PostgresEqRequiresTotalEq for #name {} - #[allow(non_snake_case)] #[::pgrx::pgrx_macros::pg_operator(immutable, parallel_safe)] #[::pgrx::pgrx_macros::opname(=)] @@ -107,7 +104,10 @@ pub fn derive_pg_eq(name: &Ident, path: &proc_macro2::TokenStream) -> proc_macro #[::pgrx::pgrx_macros::join(eqjoinsel)] #[::pgrx::pgrx_macros::merges] #[::pgrx::pgrx_macros::hashes] - fn #pg_name(left: #path, right: #path) -> bool { + fn #pg_name(left: #path, right: #path) -> bool + where + #path: ::core::cmp::Eq, + { left == right } } @@ -213,12 +213,29 @@ pub fn derive_pg_cmp(name: &Ident, path: &proc_macro2::TokenStream) -> proc_macr } } +/// Derive a Postgres hash operator using a provided hash function +/// +/// # HashEq? +/// +/// To quote the std documentation: +/// +/// "When implementing both Hash and Eq, it is important that the following property holds: +/// ```text +/// k1 == k2 -> hash(k1) == hash(k2) +/// ``` +/// In other words, if two keys are equal, their hashes must also be equal. HashMap and HashSet both rely on this behavior." +/// +/// Postgres is no different: this hashing is for the explicit purpose of equality checks, +/// and it also needs to be able to reason from hash equality to actual equality. pub fn derive_pg_hash(name: &Ident, path: &proc_macro2::TokenStream) -> proc_macro2::TokenStream { let pg_name = Ident::new(&format!("{}_hash", name).to_lowercase(), name.span()); quote! { #[allow(non_snake_case)] #[::pgrx::pgrx_macros::pg_extern(immutable, parallel_safe)] - fn #pg_name(value: #path) -> i32 { + fn #pg_name(value: #path) -> i32 + where + #path: ::core::hash::Hash + ::core::cmp::Eq, + { ::pgrx::misc::pgrx_seahash(&value) as i32 } } diff --git a/pgrx-tests/tests/ui/eq_for_postgres_hash.rs b/pgrx-tests/tests/ui/eq_for_postgres_hash.rs new file mode 100644 index 0000000000..34a2c735c3 --- /dev/null +++ b/pgrx-tests/tests/ui/eq_for_postgres_hash.rs @@ -0,0 +1,9 @@ +use pgrx::prelude::*; +use serde::{Serialize, Deserialize}; + +#[derive(Serialize, Deserialize, PostgresType, PostgresHash)] +pub struct BrokenType { + int: i32, +} + +fn main() {} diff --git a/pgrx-tests/tests/ui/eq_for_postgres_hash.stderr b/pgrx-tests/tests/ui/eq_for_postgres_hash.stderr new file mode 100644 index 0000000000..34894d1d4c --- /dev/null +++ b/pgrx-tests/tests/ui/eq_for_postgres_hash.stderr @@ -0,0 +1,27 @@ +error[E0277]: the trait bound `BrokenType: std::cmp::Eq` is not satisfied + --> tests/ui/eq_for_postgres_hash.rs:4:48 + | +4 | #[derive(Serialize, Deserialize, PostgresType, PostgresHash)] + | ^^^^^^^^^^^^ the trait `std::cmp::Eq` is not implemented for `BrokenType` + | + = help: see issue #48214 + = note: this error originates in the derive macro `PostgresHash` (in Nightly builds, run with -Z macro-backtrace for more info) +help: consider annotating `BrokenType` with `#[derive(Eq)]` + | +5 + #[derive(Eq)] +6 | pub struct BrokenType { + | + +error[E0277]: the trait bound `BrokenType: std::hash::Hash` is not satisfied + --> tests/ui/eq_for_postgres_hash.rs:4:48 + | +4 | #[derive(Serialize, Deserialize, PostgresType, PostgresHash)] + | ^^^^^^^^^^^^ the trait `std::hash::Hash` is not implemented for `BrokenType` + | + = help: see issue #48214 + = note: this error originates in the derive macro `PostgresHash` (in Nightly builds, run with -Z macro-backtrace for more info) +help: consider annotating `BrokenType` with `#[derive(Hash)]` + | +5 + #[derive(Hash)] +6 | pub struct BrokenType { + | diff --git a/pgrx-tests/tests/ui/total_eq_for_postgres_eq.stderr b/pgrx-tests/tests/ui/total_eq_for_postgres_eq.stderr index 4743611920..5f49237565 100644 --- a/pgrx-tests/tests/ui/total_eq_for_postgres_eq.stderr +++ b/pgrx-tests/tests/ui/total_eq_for_postgres_eq.stderr @@ -1,14 +1,11 @@ error[E0277]: the trait bound `BrokenType: std::cmp::Eq` is not satisfied - --> tests/ui/total_eq_for_postgres_eq.rs:5:12 + --> tests/ui/total_eq_for_postgres_eq.rs:4:59 | -5 | pub struct BrokenType { - | ^^^^^^^^^^ the trait `std::cmp::Eq` is not implemented for `BrokenType` +4 | #[derive(Serialize, Deserialize, PartialEq, PostgresType, PostgresEq)] + | ^^^^^^^^^^ the trait `std::cmp::Eq` is not implemented for `BrokenType` | -note: required by a bound in `PostgresEqRequiresTotalEq` - --> $WORKSPACE/pgrx/src/deriving.rs - | - | pub trait PostgresEqRequiresTotalEq: Eq {} - | ^^ required by this bound in `PostgresEqRequiresTotalEq` + = help: see issue #48214 + = note: this error originates in the derive macro `PostgresEq` (in Nightly builds, run with -Z macro-backtrace for more info) help: consider annotating `BrokenType` with `#[derive(Eq)]` | 5 + #[derive(Eq)] diff --git a/pgrx/src/deriving.rs b/pgrx/src/deriving.rs deleted file mode 100644 index 9754fff16f..0000000000 --- a/pgrx/src/deriving.rs +++ /dev/null @@ -1,3 +0,0 @@ -#![doc(hidden)] - -pub trait PostgresEqRequiresTotalEq: Eq {} diff --git a/pgrx/src/lib.rs b/pgrx/src/lib.rs index db850df14b..881a328278 100644 --- a/pgrx/src/lib.rs +++ b/pgrx/src/lib.rs @@ -43,7 +43,6 @@ pub mod atomics; pub mod bgworkers; pub mod callbacks; pub mod datum; -pub mod deriving; pub mod enum_helper; #[cfg(feature = "cshim")] pub mod fcall;