Add support for post_parse_analyze to PgHooks

[jteplitz]

No description provided.

[workingjubilee]

There's a lot of code duplication in this PR to satisfy the cfg requirements? But it looks almost exactly the same? Why not just #[cfg] on a single let binding inside the pgx_post_parse_analyze function?

[jteplitz]

Why not just #[cfg] on a single let binding inside the pgx_post_parse_analyze function?

I'd like to remove the duplicate code, but am not sure how to do it. I'm still new to rust so please let me know if there's something I can learn here!

The function signature is different in pg14 since it takes an extra argument. Is there a way to conditionally compile part of the parameter list in rust? We'd need something equivalent to this C code:

void pgx_post_parse_analyze(
ParseState *ps,
Query *query,
#if PG14 
JumbleState *jumble_state
#endif /* PG14 */
) {

My naive attempt to mark the parameter itself with a cfg attribute doesn't work

  unsafe extern "C" fn pgx_post_parse_analyze(
      parse_state: *mut pg_sys::ParseState,
      query: *mut pg_sys::Query,
      #[cfg(any(feature = "pg14"))]
      jumble_state: *mut JumbleState
  ) {

❯ cargo build --features pg11 --no-default-features
   Compiling pgx-pg-sys v0.5.3 (/home/eng/pgx/pgx-pg-sys)
   Compiling pgx v0.5.3 (/home/eng/pgx/pgx)
error[E0425]: cannot find value `jumble_state` in this scope
   --> pgx/src/hooks.rs:493:5
    |
493 |     jumble_state: *mut JumbleState
    |     ^^^^^^^^^^^^ not found in this scope

Also, I think we'd need 3 cfg blocks to make it work. Something like this:

+    #[cfg(any(feature = "pg14"))]
+    jumble_state: *mut JumbleState
 ) {
     fn prev(
         parse_state: PgBox<pg_sys::ParseState>,
         query: PgBox<pg_sys::Query>,
-        _jumble_state: Option<PgBox<JumbleState>>,
+        jumble_state: Option<PgBox<JumbleState>>,
     ) -> HookResult<()> {
         HookResult::new(unsafe {
             match HOOKS.as_mut().unwrap().prev_post_parse_analyze_hook.as_ref() {
                 None => (),
+                #[cfg(any(feature = "pg10", feature = "pg11", feature="pg12", feature="pg13"))]
                 Some(f) => (f)(parse_state.as_ptr(), query.as_ptr()),
+                #[cfg(any(feature = "pg14"))]
+                Some(f) => (f)(parse_state.as_ptr(), query.as_ptr(), jumble_state.unwrap().as_ptr()),

[workingjubilee]

Ooh, the extra parameter part I didn't notice.

[workingjubilee]

That might be hard to work around due to #794 but hopefully that will be fixed soon.

[workingjubilee]

#813 was merged, you might want to take a second crack at this?

[workingjubilee]

You probably will want to also support PostgreSQL 15.

[workingjubilee]

I kind of wanted #794 to be fully fixed before landing this, but I don't think it's actually worth it to keep it open. So, I will approve and merge this after #830 lands and this is updated accordingly.

[BradyBonnette]

@workingjubilee @jteplitz #830 has landed in develop

[workingjubilee]

Thank you!