Skip to content
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

Update fcinfo fn toward actual usage #1375

Merged

Conversation

workingjubilee
Copy link
Member

@workingjubilee workingjubilee commented Nov 3, 2023

Functions that had no downstream users, some odd misdeclarations, and in one case a function we can expose finally as public because it's actually what we want people to be using, going forward (probably).

This function seems to have no callers, even in downstreams. Drop it.
A little consistency is a hobgoblin of etc. etc.
It's important safety obligations appear where they are important,
and not where they do not actually bind.
Now that we support pg12 as a baseline, it's a better public API.
Another case with no public users of it.
Comment on lines -244 to +247
/// return unsafe { pg_return_void() };
/// pg_return_void()
/// }
///```
///
/// # Safety
///
/// This function is unsafe for symmetry with the other related functions that deal with
/// `PG_FUNCTION_INFO_V1` functions. It has no specific safety invariants that must be met.
#[inline]
pub unsafe fn pg_return_void() -> pg_sys::Datum {
pub fn pg_return_void() -> pg_sys::Datum {
Copy link
Member Author

Choose a reason for hiding this comment

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

I should note, despite making this change, it's always better to be able to relax an unsafety than apply one, as it's (mostly) non-breaking to relax it.

@workingjubilee workingjubilee changed the title Cleanup fcinfo fn Update fcinfo fn toward actual usage Nov 7, 2023
Copy link
Contributor

@thomcc thomcc left a comment

Choose a reason for hiding this comment

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

SGTM, I'm totally fine cutting things if it reduces the number of things we need to worry about while figuring out the soundness design changes.

@eeeebbbbrrrr
Copy link
Contributor

Go for it

@workingjubilee workingjubilee merged commit 7cef89c into pgcentralfoundation:develop Nov 8, 2023
8 checks passed
@workingjubilee workingjubilee deleted the cleanup-fcinfo-fn branch November 8, 2023 00:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants