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

Allow safe u32 -> Oid conversions #1374

Conversation

workingjubilee
Copy link
Member

A while back Thom discovered this, but we didn't open the PR at the time.

Simply cast u32 as i32 and pass it through SPI to do this by other means. While it is an incredibly bad idea to do so, unfortunately that means we cannot rely on the conversion being unsafe, as there is no way we would make SPI unsafe. That leaves no persuasive argument for not providing From.

Simply cast `u32 as i32` and pass it through SPI to do this by other means.
While it is an incredibly bad idea to do so, unfortunately that means we
cannot rely on the conversion being `unsafe`, as there is no way we would
make SPI unsafe. That leaves no persuasive argument for not providing From.
@workingjubilee workingjubilee requested a review from thomcc November 3, 2023 17:40
@workingjubilee
Copy link
Member Author

wait, no it doesn't break. what was I thinking?

@workingjubilee workingjubilee merged commit 273dcdc into pgcentralfoundation:develop Nov 8, 2023
8 checks passed
@workingjubilee workingjubilee deleted the allow-safe-oid-conversions branch December 5, 2023 21:04
workingjubilee added a commit that referenced this pull request Dec 5, 2023
A while back Thom discovered this, but we didn't open the PR at the
time.

Simply cast `u32 as i32` and pass it through SPI to do this by other
means. While it is an incredibly bad idea to do so, unfortunately that
means we cannot rely on the conversion being `unsafe`, as there is no
way we would make SPI unsafe. That leaves no persuasive argument for not
providing From.
workingjubilee added a commit that referenced this pull request Dec 5, 2023
pgrx v0.11.2 is a minor release which

- makes available the storage-related API, thanks to @silver-ymz in
#1409
- deprecates the `Oid::from_u32_unchecked` API... because it is actually
possible to do via casting in a *query*, which makes it effectively
impossible for us to not wind up providing it via some other safe API,
since the source is also an easily-Copied type. Thus you can now simply
use `From::from` for it. Thanks to @thomcc for this discovery,
implemented by @workingjubilee in
#1374

As usual, `cargo install cargo-pgrx --version 0.11.2 --locked` and be on
your merry way!

Thanks everyone!
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.

1 participant