Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] support casting between pgnodes #1048

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion pgx-pg-sys/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -502,14 +502,39 @@ fn impl_pg_node(
Box::new(node_set.into_iter())
};

// Though these structs looks like a node, they don't seem to have a corresponding [`NodeTag`].
// Also for `Node`, it doesn't have a node tag.
let block_list = [
"Node",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why block Node, exactly?

"MemoryContextData",
"JoinPath",
"Expr",
"HeapTupleTableSlot",
"VirtualTupleTableSlot",
"PartitionPruneStep",
"BufferHeapTupleTableSlot",
"MinimalTupleTableSlot",
]
.into_iter()
.collect::<HashSet<_>>();

// now we can finally iterate the Nodes and emit out Display impl
for node_struct in nodes {
let struct_name = &node_struct.struct_.ident;

let node_tag = if !block_list.contains(struct_name.to_string().as_str()) {
let node_tag = quote::format_ident!("NodeTag_T_{}", struct_name);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I changed NodeTag to a real enum, so this would have to be

Suggested change
let node_tag = quote::format_ident!("NodeTag_T_{}", struct_name);
let node_tag = quote::format_ident!("NodeTag_T::{}", struct_name);

quote! { Some(pg_sys::#node_tag) }
} else {
quote! { None }
};

// impl the PgNode trait for all nodes
pgnode_impls.extend(quote! {
impl pg_sys::seal::Sealed for #struct_name {}
impl pg_sys::PgNode for #struct_name {}
impl pg_sys::PgNode for #struct_name {
const NODE_TAG: Option<NodeTag> = #node_tag;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't describe polymorphic types with a monomorphic tag like this.

}
});

// impl Rust's Display trait for all nodes
Expand Down
64 changes: 64 additions & 0 deletions pgx-pg-sys/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,10 @@ pub use internal::pg15::*;

/// A trait applied to all Postgres `pg_sys::Node` types and subtypes
pub trait PgNode: seal::Sealed {
/// The [`NodeTag`] for this node. If it is `None` then it doesn't have a corresponding
/// [`NodeTag`].
const NODE_TAG: Option<NodeTag>;

/// Format this node
///
/// # Safety
Expand Down Expand Up @@ -269,6 +273,66 @@ impl AsPgCStr for &Option<std::string::String> {
}
}

pub trait PgNodeCast<T> {
fn cast_from(pg_node: T) -> Self
where
Self: Sized;
}

pub trait PgNodeTryCast<T> {
fn try_cast_from(pg_node: T) -> Option<Self>
where
Self: Sized;
}

impl<A: PgNode, B: PgNode> PgNodeCast<&A> for &B {
fn cast_from(pg_node: &A) -> Self {
Self::try_cast_from(pg_node).unwrap()
}
Comment on lines +289 to +291
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with Eric, we should just make it always-fallible and ditch this.

}

impl<A: PgNode, B: PgNode> PgNodeTryCast<&A> for &B {
fn try_cast_from(pg_node: &A) -> Option<Self> {
unsafe {
let node = std::mem::transmute::<_, &Node>(pg_node);
match B::NODE_TAG {
Some(tag) => {
if node.type_ == tag {
Some(std::mem::transmute::<_, &B>(node))
} else {
None
}
}
None => None,
}
}
Comment on lines +296 to +308
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hrmmm. What "directionality" of casting is this intended to handle? Upcasting? Downcasting? Something else? I feel we should support checked casting, for sure, instead of just making people do pointer-casts, but I don't feel confident about what this is "for", basically.

}
}

impl<A: PgNode, B: PgNode> PgNodeCast<&mut A> for &mut B {
fn cast_from(pg_node: &mut A) -> Self {
Self::try_cast_from(pg_node).unwrap()
}
}

impl<A: PgNode, B: PgNode> PgNodeTryCast<&mut A> for &mut B {
fn try_cast_from(pg_node: &mut A) -> Option<Self> {
unsafe {
let node = std::mem::transmute::<_, &mut Node>(pg_node);
match B::NODE_TAG {
Some(tag) => {
if node.type_ == tag {
Some(std::mem::transmute::<_, &mut B>(node))
} else {
None
}
}
None => None,
}
}
}
}

/// item declarations we want to add to all versions
mod all_versions {
use crate as pg_sys;
Expand Down