From a6444584f3819ec4194993aebbef35e53a9c35d8 Mon Sep 17 00:00:00 2001 From: Jubilee <46493976+workingjubilee@users.noreply.github.com> Date: Thu, 29 Feb 2024 19:16:08 -0800 Subject: [PATCH] Address pg16 rename of `*_tree_walker` fn (#1596) These functions had been renamed in Postgres 16, and they are now fn macros. That means this `extern "C"` declaration is simply incorrect in Postgres 16. Implement a relatively logical version-by-version handling. Fixes #1583. --- pgrx-pg-sys/build.rs | 2 +- pgrx-pg-sys/src/port.rs | 64 ++++++++++++++++++++++------ pgrx-tests/src/tests/pg_try_tests.rs | 40 ++++------------- 3 files changed, 61 insertions(+), 45 deletions(-) diff --git a/pgrx-pg-sys/build.rs b/pgrx-pg-sys/build.rs index fecc9ab16..761317074 100644 --- a/pgrx-pg-sys/build.rs +++ b/pgrx-pg-sys/build.rs @@ -741,7 +741,7 @@ fn add_blocklists(bind: bindgen::Builder) -> bindgen::Builder { bind.blocklist_type("Datum") // manually wrapping datum for correctness .blocklist_type("Oid") // "Oid" is not just any u32 .blocklist_function("varsize_any") // pgrx converts the VARSIZE_ANY macro, so we don't want to also have this function, which is in heaptuple.c - .blocklist_function("(?:query|expression)_tree_walker") + .blocklist_function("(?:raw_)?(?:query|expression)_tree_walker") .blocklist_function(".*(?:set|long)jmp") .blocklist_function("pg_re_throw") .blocklist_function("err(start|code|msg|detail|context_msg|hint|finish)") diff --git a/pgrx-pg-sys/src/port.rs b/pgrx-pg-sys/src/port.rs index 170679254..4bd43e8b4 100644 --- a/pgrx-pg-sys/src/port.rs +++ b/pgrx-pg-sys/src/port.rs @@ -1,7 +1,6 @@ use crate as pg_sys; use memoffset::*; -use pgrx_macros::pg_guard; use std::str::FromStr; /// this comes from `postgres_ext.h` @@ -229,29 +228,70 @@ pub unsafe fn heap_tuple_get_struct(htup: super::HeapTuple) -> *mut T { } } -#[pg_guard] +// All of this weird code is in response to Postgres having a relatively cavalier attitude about types: +// - https://github.com/postgres/postgres/commit/1c27d16e6e5c1f463bbe1e9ece88dda811235165 +// +// As a result, we redeclare their functions with the arguments they should have on earlier Postgres +// and we route people to the old symbols they were using before on later ones. +#[cfg(any(feature = "pg12", feature = "pg13", feature = "pg14", feature = "pg15"))] +#[::pgrx_macros::pg_guard] extern "C" { pub fn query_tree_walker( query: *mut super::Query, - walker: ::std::option::Option< - unsafe extern "C" fn(*mut super::Node, *mut ::std::os::raw::c_void) -> bool, + walker: ::core::option::Option< + unsafe extern "C" fn(*mut super::Node, *mut ::core::ffi::c_void) -> bool, >, - context: *mut ::std::os::raw::c_void, - flags: ::std::os::raw::c_int, + context: *mut ::core::ffi::c_void, + flags: ::core::ffi::c_int, ) -> bool; -} -#[pg_guard] -extern "C" { pub fn expression_tree_walker( node: *mut super::Node, - walker: ::std::option::Option< - unsafe extern "C" fn(*mut super::Node, *mut ::std::os::raw::c_void) -> bool, + walker: ::core::option::Option< + unsafe extern "C" fn(*mut super::Node, *mut ::core::ffi::c_void) -> bool, + >, + context: *mut ::core::ffi::c_void, + ) -> bool; + + pub fn raw_expression_tree_walker( + node: *mut super::Node, + walker: ::core::option::Option< + unsafe extern "C" fn(*mut super::Node, *mut ::core::ffi::c_void) -> bool, >, - context: *mut ::std::os::raw::c_void, + context: *mut ::core::ffi::c_void, ) -> bool; } +#[cfg(feature = "pg16")] +pub unsafe fn query_tree_walker( + query: *mut super::Query, + walker: ::core::option::Option< + unsafe extern "C" fn(*mut super::Node, *mut ::core::ffi::c_void) -> bool, + >, + context: *mut ::core::ffi::c_void, + flags: ::core::ffi::c_int, +) -> bool { + crate::query_tree_walker_impl(query, walker, context, flags) +} + +#[cfg(feature = "pg16")] +pub unsafe fn expression_tree_walker( + arg_node: *mut crate::Node, + arg_walker: Option bool>, + arg_context: *mut ::core::ffi::c_void, +) -> bool { + crate::expression_tree_walker_impl(arg_node, arg_walker, arg_context) +} + +#[cfg(feature = "pg16")] +pub unsafe fn raw_expression_tree_walker( + arg_node: *mut crate::Node, + arg_walker: Option bool>, + arg_context: *mut ::core::ffi::c_void, +) -> bool { + crate::raw_expression_tree_walker_impl(arg_node, arg_walker, arg_context) +} + #[inline(always)] pub unsafe fn MemoryContextSwitchTo(context: crate::MemoryContext) -> crate::MemoryContext { let old = crate::CurrentMemoryContext; diff --git a/pgrx-tests/src/tests/pg_try_tests.rs b/pgrx-tests/src/tests/pg_try_tests.rs index a290f5a86..f75fd3450 100644 --- a/pgrx-tests/src/tests/pg_try_tests.rs +++ b/pgrx-tests/src/tests/pg_try_tests.rs @@ -7,48 +7,24 @@ //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 core::ptr; use pgrx::prelude::*; // if our Postgres ERROR and Rust panic!() handling is incorrect, this little bit of useless code // will crash postgres. If things are correct it'll simply raise an ERROR saying "panic in walker". #[pg_extern] fn crash() { - #[cfg(feature = "cshim")] - { - use pgrx::PgList; - let mut node = PgList::::new(); - node.push(PgList::::new().into_pg() as *mut pg_sys::Node); - - #[cfg(any(feature = "pg12", feature = "pg13", feature = "pg14", feature = "pg15"))] - unsafe { - pg_sys::raw_expression_tree_walker( - node.into_pg() as *mut pg_sys::Node, - Some(walker), - std::ptr::null_mut(), - ); - } - - #[cfg(feature = "pg16")] - error!("panic in walker"); - } + use pgrx::{list::List, memcx}; + memcx::current_context(|current| unsafe { + let mut list = List::downcast_ptr_in_memcx(ptr::null_mut(), current).unwrap(); + list.unstable_push_in_context(ptr::null_mut(), current); - #[cfg(not(feature = "cshim"))] - { - walker(); - } -} - -#[cfg(not(feature = "cshim"))] -fn walker() -> bool { - panic!("panic in walker"); + pg_sys::raw_expression_tree_walker(list.as_mut_ptr().cast(), Some(walker), ptr::null_mut()); + }); } -#[cfg(all( - feature = "cshim", - any(feature = "pg12", feature = "pg13", feature = "pg14", feature = "pg15") -))] #[pg_guard] -extern "C" fn walker() -> bool { +extern "C" fn walker(_node: *mut pg_sys::Node, _void: *mut ::core::ffi::c_void) -> bool { panic!("panic in walker"); }