Make PostgresHash also need Eq (#1264)

Latest in series of fixing the derive macros to require the correct
bounds. This time, basically, "The hash operators are used for hashmappy
things, so they should have the same bounds as hashmaps."

Moves away from the briefly-adopted `#[doc(hidden)]` trait pattern to
instead using a "trivial bound" for these. It didn't actually occur to
me, first go-around, that such was correct in Rust and would emit a
similar, reasonably-nice error.

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()
 }

pgrx-macros/src/operators.rs

@@ -95,9 +95,6 @@ pub(crate) fn deriving_postgres_hash(ast: DeriveInput) -> syn::Result<proc_macro
 pub fn derive_pg_eq(name: &Ident, path: &proc_macro2::TokenStream) -> 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
         }
     }

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() {}

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 {
+  |

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)]

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;
 pub mod fcinfo;
 pub mod ffi;