Skip to content

Include libpq/libpq-be.h #2117

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

Include libpq/libpq-be.h #2117

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fbc2bc7
Install development packages for PGDG packages
cbandy Aug 2, 2025
3ad5ce3
Include libpq/libpq-be.h
cbandy Jul 30, 2025
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
4 changes: 4 additions & 0 deletions .github/workflows/runas.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ jobs:
build-essential \
llvm-14-dev libclang-14-dev clang-14 \
gcc \
libkrb5-dev \
Copy link
Contributor

@eeeebbbbrrrr eeeebbbbrrrr Aug 21, 2025

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

@eeeebbbbrrrr eeeebbbbrrrr Aug 21, 2025

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

@cbandy cbandy Aug 21, 2025

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@cbandy cbandy Aug 21, 2025

Choose a reason for hiding this comment

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

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

libssl-dev \
libz-dev \
make \
Expand Down Expand Up @@ -86,6 +87,9 @@ jobs:
cargo --version
pg_config --version

- name: pg_config details
run: pg_config

- name: Install cargo pgrx
run: cd cargo-pgrx && cargo install --path . --debug

Expand Down
1 change: 1 addition & 0 deletions .github/workflows/tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ jobs:
build-essential \
llvm-14-dev libclang-14-dev clang-14 \
gcc \
libkrb5-dev \
libssl-dev \
libz-dev \
make \
Expand Down
1 change: 1 addition & 0 deletions pgrx-pg-sys/include/pg13.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@
#include "foreign/foreign.h"
#include "jit/jit.h"
#include "lib/stringinfo.h"
#include "libpq/libpq-be.h"
#include "libpq/pqformat.h"
#include "mb/pg_wchar.h"
#include "nodes/execnodes.h"
Expand Down
1 change: 1 addition & 0 deletions pgrx-pg-sys/include/pg14.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@
#include "foreign/foreign.h"
#include "jit/jit.h"
#include "lib/stringinfo.h"
#include "libpq/libpq-be.h"
#include "libpq/pqformat.h"
#include "mb/pg_wchar.h"
#include "nodes/execnodes.h"
Expand Down
1 change: 1 addition & 0 deletions pgrx-pg-sys/include/pg15.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@
#include "foreign/foreign.h"
#include "jit/jit.h"
#include "lib/stringinfo.h"
#include "libpq/libpq-be.h"
#include "libpq/pqformat.h"
#include "mb/pg_wchar.h"
#include "nodes/execnodes.h"
Expand Down
1 change: 1 addition & 0 deletions pgrx-pg-sys/include/pg16.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@
#include "foreign/foreign.h"
#include "jit/jit.h"
#include "lib/stringinfo.h"
#include "libpq/libpq-be.h"
#include "libpq/pqformat.h"
#include "mb/pg_wchar.h"
#include "nodes/execnodes.h"
Expand Down
1 change: 1 addition & 0 deletions pgrx-pg-sys/include/pg17.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@
#include "foreign/foreign.h"
#include "jit/jit.h"
#include "lib/stringinfo.h"
#include "libpq/libpq-be.h"
#include "libpq/pqformat.h"
#include "mb/pg_wchar.h"
#include "nodes/execnodes.h"
Expand Down
1 change: 1 addition & 0 deletions pgrx-pg-sys/include/pg18.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@
#include "foreign/foreign.h"
#include "jit/jit.h"
#include "lib/stringinfo.h"
#include "libpq/libpq-be.h"
#include "libpq/pqformat.h"
#include "mb/pg_wchar.h"
#include "nodes/execnodes.h"
Expand Down
9 changes: 9 additions & 0 deletions pgrx-pg-sys/src/lib.rs
View file
Open in desktop
Copy link
Member

@workingjubilee workingjubilee Sep 24, 2025

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@cbandy cbandy Sep 25, 2025

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

@usamoi usamoi Sep 25, 2025

Choose a reason for hiding this comment

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

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.

Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,12 @@ mod seal {
#[cfg(target_os = "linux")]
#[link(name = "resolv")]
unsafe extern "C" {}

// Link to the builtin crypto libraries on Windows.
// This is necessary only when Postgres is built --with-openssl, but what Windows distribution isn't?
//
// See: https://github.com/postgres/postgres/blob/REL_17_0/meson.build#L1321
#[cfg(target_os = "windows")]
#[link(name = "ssl")]
#[link(name = "crypto")]
unsafe extern "C" {}