Skip to content

Commit

Permalink
Address pg16 rename of *_tree_walker fn (#1596)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
workingjubilee authored Mar 1, 2024
1 parent 2ad3838 commit a644458
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 45 deletions.
2 changes: 1 addition & 1 deletion pgrx-pg-sys/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)")
Expand Down
64 changes: 52 additions & 12 deletions pgrx-pg-sys/src/port.rs
Original file line number Diff line number Diff line change
@@ -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`
Expand Down Expand Up @@ -229,29 +228,70 @@ pub unsafe fn heap_tuple_get_struct<T>(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<unsafe extern "C" fn(*mut crate::Node, *mut ::core::ffi::c_void) -> 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<unsafe extern "C" fn(*mut crate::Node, *mut ::core::ffi::c_void) -> 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;
Expand Down
40 changes: 8 additions & 32 deletions pgrx-tests/src/tests/pg_try_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<pg_sys::Node>::new();
node.push(PgList::<pg_sys::Node>::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");
}

Expand Down

0 comments on commit a644458

Please sign in to comment.