Skip to content

Commit 273d16b

Browse files
committed
Add MemoryToken::parse
Currently if you want to manipulate a query immediately after parsing it, you have to call `pg_raw_parse::parse`, then `MemoryToken::make_unique` to perform a deep copy onto a new memory context. We have places in `pgdog` which are using this pattern, and immediately throwing away the unmodified AST/MemoryContext. We can skip the extra copy/allocation by allowing you to parse within `make::try_owned` directly. We ended up never actually exposing the `warnings` field of `ParseResult`, so I've gone ahead and removed it entirely. If we do ever want to expose them, I'll either need to change the signature of `parse` to return them as well as the stmts, or provide a new function that returns both. At this point `ParseResult` is a pointless type and can be removed, but doing so is an API breaking change so I've left it in for now.
1 parent 4843f8b commit 273d16b

2 files changed

Lines changed: 33 additions & 45 deletions

File tree

src/lib.rs

Lines changed: 4 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#![cfg_attr(feature = "field_offset_assertions", feature(offset_of_enum))]
2-
use std::{ffi, fmt, ops, ptr};
2+
use std::{fmt, ops};
33

44
pub mod const_val;
55
mod deparse;
@@ -29,36 +29,14 @@ pub(crate) use node_ptr::{
2929
};
3030

3131
pub fn parse(sql: &str) -> Result<ParseResult, error::Error> {
32-
let mem = mem::MemoryContext::new(c"pg_raw_parse");
33-
let cstring = ffi::CString::new(sql).map_err(error::Error::StatementContainedNul)?;
34-
// SAFETY: we never panic within the provided block
35-
let c_result = unsafe {
36-
mem.within(|| {
37-
raw::pg_query_raw_parse(
38-
cstring.as_ptr(),
39-
raw::PgQueryParseMode::PG_QUERY_PARSE_DEFAULT as _,
40-
)
41-
})
42-
};
43-
// Any warnings that were emitted during parsing went into a malloc'd
44-
// buffer, so we need to construct this even if we're going to return Err
45-
// to ensure that buffer is freed.
46-
let warnings = Warnings {
47-
stderr_buffer: ptr::NonNull::new(c_result.stderr_buffer),
48-
};
49-
match ptr::NonNull::new(c_result.error) {
50-
Some(e) => Err(Error::from_pg_query_error(e)),
51-
None => Ok(ParseResult {
52-
_warnings: warnings,
53-
tree: Owned::new(mem, c_result.tree.cast()),
54-
}),
55-
}
32+
Ok(ParseResult {
33+
tree: make::try_owned(|mem| mem.parse(sql))?,
34+
})
5635
}
5736

5837
pub type StmtList = list::CastNodeList<nodes::RawStmt>;
5938

6039
pub struct ParseResult {
61-
_warnings: Warnings,
6240
tree: Owned<StmtList>,
6341
}
6442

@@ -85,20 +63,3 @@ impl fmt::Debug for ParseResult {
8563
.finish_non_exhaustive()
8664
}
8765
}
88-
89-
struct Warnings {
90-
stderr_buffer: Option<ptr::NonNull<ffi::c_char>>,
91-
}
92-
93-
impl Drop for Warnings {
94-
fn drop(&mut self) {
95-
// tree was created with palloc, so is managed by postgres.
96-
// stderr_buffer was malloc'd and must be freed
97-
// SAFETY: libpg_query documents that the caller must free this.
98-
unsafe {
99-
if let Some(ptr) = self.stderr_buffer.take() {
100-
libc::free(ptr.as_ptr() as _);
101-
}
102-
}
103-
}
104-
}

src/make.rs

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,12 @@ use crate::list::{CastNodeList, NodeList};
22
use crate::mem::MemoryContext;
33
use crate::raw::{self, *};
44
use crate::{
5-
AsNodePtr, ConstValue, ConstructableNode, FromNodeMut, FromNodePtr, Node, Owned, nodes,
5+
AsNodePtr, ConstValue, ConstructableNode, Error, FromNodeMut, FromNodePtr, Node, Owned, Result,
6+
StmtList, nodes,
67
};
78
use generativity::Id;
89
use std::any::type_name;
9-
use std::ffi::{c_char, c_int};
10+
use std::ffi::{CString, c_char, c_int};
1011
use std::marker::PhantomData;
1112
use std::ops::Deref;
1213
use std::ptr;
@@ -45,6 +46,32 @@ pub struct MemoryToken<'mem> {
4546
}
4647

4748
impl<'mem> MemoryToken<'mem> {
49+
/// Parse the given `sql` into a new AST on this memory context.
50+
///
51+
/// This function can be used if you need to parse and then immediately
52+
/// modify an AST, without copying it. If you only need to parse an AST,
53+
/// use [`crate::parse`]
54+
pub fn parse(self, sql: &str) -> Result<Unique<'mem, &'mem StmtList>> {
55+
let cstring = CString::new(sql).map_err(Error::StatementContainedNul)?;
56+
// SAFETY: we never panic within the provided block
57+
let c_result = unsafe {
58+
self.mem.within(|| {
59+
raw::pg_query_raw_parse(
60+
cstring.as_ptr(),
61+
raw::PgQueryParseMode::PG_QUERY_PARSE_DEFAULT as _,
62+
)
63+
})
64+
};
65+
if !c_result.stderr_buffer.is_null() {
66+
// SAFETY: libpg_query documents that the caller must free this.
67+
unsafe { libc::free(c_result.stderr_buffer as _) };
68+
}
69+
match ptr::NonNull::new(c_result.error) {
70+
Some(e) => Err(Error::from_pg_query_error(e)),
71+
None => Ok(Unique(c_result.tree.cast(), self.id, PhantomData)),
72+
}
73+
}
74+
4875
pub fn make_a_const(self, val: ConstValue<'_>) -> Unique<'mem, &'mem nodes::A_Const> {
4976
let mut node = self.make_node::<nodes::A_Const>();
5077
node.as_mut().set_isnull(false);

0 commit comments

Comments
 (0)