-
-
Notifications
You must be signed in to change notification settings - Fork 296
Manually bind MyProcPort #2162
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
base: develop
Are you sure you want to change the base?
Manually bind MyProcPort #2162
Conversation
a3856b9 to
1477ed3
Compare
1477ed3 to
9e613a3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's one more field that needs an annotation for pg13, but everything else LGTM.
It was tedious to check, but I suppose we'll only need to compare when adding a major version.
Thank you for working on this!
| @@ -0,0 +1,98 @@ | |||
| /** Manually-constructed bindings to libpq | |||
|
|
|||
| Because pgrx is for extensions which run in the Postgres server, it rarely needs access to libpq. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏻
|
|
||
| /// #define SCRAM_MAX_KEY_LEN PG_SHA256_DIGEST_LENGTH | ||
| /// #define PG_SHA256_DIGEST_LENGTH 32 | ||
| const SCRAM_MAX_KEY_LEN: usize = 32; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏻
| #[cfg(any(feature = "pg13", feature = "pg14", feature = "pg15", feature = "pg16"))] | ||
| pub canAcceptConnections: core::ffi::c_uint, | ||
| #[cfg(feature = "pg18")] | ||
| pub local_host: [core::ffi::c_char; 64], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏻
| HbaLine: *mut core::ffi::c_void, | ||
|
|
||
| #[cfg(any(feature = "pg14", feature = "pg15"))] | ||
| authn_id: *const core::ffi::c_char, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏻
| ssl_in_use: bool, | ||
| peer_cn: *mut core::ffi::c_char, | ||
| peer_dn: *mut core::ffi::c_char, | ||
| peer_cert_valid: bool, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔧 pg13 lacks peer_dn here. https://github.com/postgres/postgres/blob/REL_13_0/src/include/libpq/libpq-be.h#L187
| ssl_in_use: bool, | |
| peer_cn: *mut core::ffi::c_char, | |
| peer_dn: *mut core::ffi::c_char, | |
| peer_cert_valid: bool, | |
| ssl_in_use: bool, | |
| peer_cn: *mut core::ffi::c_char, | |
| #[cfg(any(feature = "pg14", feature = "pg15", feature = "pg16", feature = "pg17", feature = "pg18"))] | |
| peer_dn: *mut core::ffi::c_char, | |
| peer_cert_valid: bool, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, thank you. Missed that one.
| #[cfg(any(feature = "pg18", feature = "pg17"))] | ||
| alpn_used: bool, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⛏️ nit
| #[cfg(any(feature = "pg18", feature = "pg17"))] | |
| alpn_used: bool, | |
| #[cfg(any(feature = "pg17", feature = "pg18"))] | |
| alpn_used: bool, |
| #[cfg(any(feature = "pg18", feature = "pg17"))] | ||
| alpn_used: bool, | ||
| #[cfg(feature = "pg18")] | ||
| last_read_was_eof: bool, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏻
| // as if USE_OPENSSL == false && ENABLE_SSPI == false | ||
| #[cfg(feature = "pg18")] | ||
| ssl: *mut core::ffi::c_void, | ||
| #[cfg(feature = "pg18")] | ||
| peer: *mut core::ffi::c_void, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔 I see only USE_OPENSSL affecting these. https://github.com/postgres/postgres/blob/REL_18_0/src/include/libpq/libpq-be.h#L215
| // as if USE_OPENSSL == false && ENABLE_SSPI == false | |
| #[cfg(feature = "pg18")] | |
| ssl: *mut core::ffi::c_void, | |
| #[cfg(feature = "pg18")] | |
| peer: *mut core::ffi::c_void, | |
| // as if USE_OPENSSL == false | |
| #[cfg(feature = "pg18")] | |
| ssl: *mut core::ffi::c_void, | |
| #[cfg(feature = "pg18")] | |
| peer: *mut core::ffi::c_void, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
think that was a copypaste error, yeah.
| #[cfg(feature = "pg18")] | ||
| raw_buf: *mut core::ffi::c_char, | ||
| #[cfg(feature = "pg18")] | ||
| raw_buf_consumed: isize, | ||
| #[cfg(feature = "pg18")] | ||
| raw_buf_remaining: isize, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏻
| last_read_was_eof: bool, | ||
|
|
||
| // NOTE: 5 fields remain on PG17, but two are `#ifdef USE_OPENSSL` in PG17, so treat all | ||
| // as conditioned on PG18, even if that is not strictly accurate for PG17 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔 So we're too small in pg17. I'm not sure what to do about that or how to better explain it to the reader. 👍🏻
This is effectively my proposed alternative to #2117, building on @usamoi's suggestion.
Fixes #2157
cc @cbandy