Include libpq/libpq-be.h

[cbandy]

I want to read database_name and user_name from MyProcPort. These fields inform the %u and %d escapes of log_line_prefix. Without this, the binding for Port is:

pub struct Port {
    _unused: [u8; 0],
}

[eeeebbbbrrrr]

I'm sorry for the delay in approving CI, but now that it is, it looks like this has introduced a number of build failures.

I don't know if libpq-be.h is going to require some extra ./configure options or what.

[cbandy]

Thanks! I'll take a look.

[eeeebbbbrrrr]

I don't know if libpq-be.h is going to require some extra ./configure options or what.

I guess I should say that my preference is that we find a way to make this work without needing to change how we expect postgres to be ./configure'd. Doing so would be a gigantic pain for existing users.

Truthfully, now that I've looked at the contents of libpq-be.h in more detail, I don't really think it belongs in pgrx anyways.

[cbandy]

Is there an alternative to MyProcPort? I would like to know the database name inside a log hook.

[cbandy]

The CI pipelines use PGDG packages which are built using '--with-gssapi' . ¹ Ubuntu keeps GSS headers in the libkrb5-dev package.

I'm not familiar with developing on Windows. 🙇🏻

Footnotes

1. https://github.com/pgcentralfoundation/pgrx/actions/runs/16614280060/job/47202306964?pr=2117#step:7:34 ↩

[usamoi]

Is there an alternative to MyProcPort? I would like to know the database name inside a log hook.

What about https://docs.rs/pgrx/latest/pgrx/pg_sys/fn.get_database_name.html? You may get the current database name by get_database_name(MyDatabaseId).

[usamoi]

Windows CI fails because crypto and ssl are not linked.

Putting

println!("cargo::rustc-link-lib=ssl");
println!("cargo::rustc-link-lib=crypto");

to pgrx-bindgen/src/build.rs should work.

[cbandy]

Windows CI fails because crypto and ssl are not linked.

Putting

println!("cargo::rustc-link-lib=ssl");
println!("cargo::rustc-link-lib=crypto");

to pgrx-bindgen/src/build.rs should work.

From my research, I agree!

Where should these go in build.rs? I see very little logic for Windows or MSVC, and I'm out of my depth.

[usamoi]

Where should these go in build.rs?

Simply put this code after the ending of pgrx-pg-sys/src/lib.rs:

#[cfg(target_os = "windows")]
#[link(name = "ssl")]
#[link(name = "crypto")]
unsafe extern "C" {}

[cbandy]

Simply put this code after the ending of pgrx-pg-sys/src/lib.rs:

Done!

[eeeebbbbrrrr]

I'm concerned about this new requirement.

If this is now required for pgrx extensions it will most likely break everyone's CI. This isn't something I want to do.

[eeeebbbbrrrr]

Can we do this such that kerberos isn't a required system dependency?

[cbandy]

This is not a new dependency. The Postgres installed in the GitHub action is compiled with --with-gssapi, so bindgen is loading a pg_config.h containing ENABLE_GSS 1 and HAVE_GSSAPI_GSSAPI_H 1.

The GSS headers are in the libkrb5-dev package.

[cbandy]

The Postgres installed in the action is compiled --with-openssl so libssl-dev is installed on the line below this one.

[workingjubilee]

I don't see a response to usamoi's suggestion to use https://docs.rs/pgrx/latest/pgrx/pg_sys/fn.get_database_name.html

Eric reviewed the header and thinks libpq-be is a bridge too far for pgrx, and it requires additional linkages such as these that cause complications.

I don't think this should be merged without at the very least an answer as to why usamoi's alternative route is not acceptable.

[cbandy]

I'll take a look later this week. My goal is to implement a log hook that writes in a new format, so I'm interested in all the fields included in the current formats. The ones I see from MyProcPort in text prefix, CSV, JSON are:

• user name
• database name
• remote host
• remote port

If you have any suggestions for alternatives, I'd appreciate them.

[usamoi]

If pgrx does not generate bindings for this file, you can also write them manually, like this:

struct Port {
    sock: pgrx::pg_sys::pgsocket,
    noblock: bool,
    proto: pgrx::pg_sys::ProtocolVersion,
    laddr: pgrx::pg_sys::SockAddr,
    raddr: pgrx::pg_sys::SockAddr,
    remote_host: *mut core::ffi::c_char,
    remote_hostname: *mut core::ffi::c_char,
    remote_hostname_resolv: core::ffi::c_int,
    remote_hostname_errcode: core::ffi::c_int,
    remote_port: *mut core::ffi::c_char,
    #[cfg(any(feature = "pg13", feature = "pg14", feature = "pg15", feature = "pg16"))]
    canAcceptConnections: core::ffi::c_uint,
    #[cfg(feature = "pg18")]
    local_host: [core::ffi::c_char; 64],
    database_name: *mut core::ffi::c_char,
    user_name: *mut core::ffi::c_char,
    cmdline_options: *mut core::ffi::c_char,
    guc_options: *mut core::ffi::c_char,
    application_name: *mut core::ffi::c_char,
}

You just need to check whether struct Proc has changed before supporting each PostgreSQL major version.

[workingjubilee]

This seems like, even though it has gotten working in CI, it will risk adding too much friction to dev with pgrx. We probably should just handbind everything we want instead. I think we're happy to host those hand-bindings upstream (i.e. here) and try to keep them consistent on version updates, as long as they are what we need and no more and we document what we are binding them from. This is an unfortunate flaw of bindgen, because it tries too much to be a C compiler, even when it doesn't need to be.

[cbandy]

We probably should just handbind everything we want instead. I think we're happy to host those hand-bindings upstream (i.e. here) and try to keep them consistent on version updates, as long as they are what we need and no more and we document what we are binding them from.

Thanks, I appreciate that! 🙇🏻

I haven't had the opportunity to try a manual binding locally. The latter fields (database_name, user_name, application_name) seem reasonably testable. I should be able to help on that front.

I'm going to close this PR and open a proper feature request. Thank you for your patience with this!