Skip to content

Demand Ord for PostgresOrd #1262

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

Jump to bottom
Merged
merged 3 commits into from
Aug 17, 2023
Merged

Demand Ord for PostgresOrd #1262

merged 3 commits into from
Aug 17, 2023

Conversation

workingjubilee
Copy link
Member

Previously, the cmp invocation could be misinterpreted as Iterator.
Code exploiting this bug is unlikely, but it could hit other errors,
so this way the error is much nicer.

Also makes the previous trybuild test for PostgresEq more realistic.

@workingjubilee
Demand Ord for PostgresOrd up-front
be5c181 
Previously, the cmp invocation could be misinterpreted as Iterator.
Code exploiting this bug is unlikely, but it could hit other errors,
so this way the error is much nicer.
@workingjubilee
More realistic ui test for PostgresEq
057bfed 
I had written the test after making the change, previously.
That meant the test had other ways to fail which I missed.
This one is guaranteed to work if not for recent changes.
workingjubilee
workingjubilee commented Aug 17, 2023
View reviewed changes
pgrx-tests/tests/ui/total_ord_for_postgres_ord.rs Outdated
Comment on lines 10 to 43
impl Iterator for BrokenType {
type Item = i32;

fn next(&mut self) -> Option<Self::Item> {
todo!()
}

// Previously, this fn could win over Ord::cmp in trait resolution
fn cmp<I>(self, _rhs: I) -> Ordering
where
I: IntoIterator<Item = Self::Item>,
{
todo!()
}
}

impl IntoIterator for &BrokenType {
type Item = i32;
type IntoIter = BrokenIter;

fn into_iter(self) -> BrokenIter {
todo!()
}
}

pub struct BrokenIter {}

impl Iterator for BrokenIter {
type Item = i32;

fn next(&mut self) -> Option<Self::Item> {
todo!()
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This entire repro could be greatly simplified by simply implementing fn cmp.

@thomcc
Copy link
Contributor

thomcc commented Aug 17, 2023

I think it could probably happen without any traits, just with an inherent fn cmp(&self, o: &Self), no?

@workingjubilee
Copy link
Member Author

workingjubilee commented Aug 17, 2023 •
edited
Loading

The LHS didn't even need to be &self, so this could have been a mutating comparison hypothetically!

@workingjubilee workingjubilee merged commit a8a09a0 into pgcentralfoundation:develop Aug 17, 2023
@workingjubilee workingjubilee deleted the order-better-errors branch August 17, 2023 05:18
@workingjubilee workingjubilee mentioned this pull request Aug 23, 2023
Merged
workingjubilee added a commit that referenced this pull request Aug 23, 2023
@workingjubilee
Update version to 0.10.0-beta.4 (#1267)
1592952 
The fifth beta release of 0.10.0!

* `#[pg_test]`-compatible proptest support in
#1258 courtesy of moi
* PostgresEq (#1261) and
PostgresOrd (#1262) now
explicitly require their "base" Rust traits, which may hypothetically
break some impls, but in exchange your equality implementations are now
presumed to be commutative.
* Work on improving/simplifying/optimizing our SPI continues, with
@vrmiguel contributing some polish to Datum preparation in
#1256
* We have been finding some extra-spicy edge cases in how PGRX interacts
with a continuously-operational Postgres installation which might
feature extensions being installed and reinstalled lately! @Sasasu fixed
at least one core-dump-causing case from `cargo pgrx install` in
#1263

## New Contributors
* @vrmiguel made their first contribution in
#1256

**Full Changelog**:
v0.10.0-beta.3...v0.10.0-beta.4
eeeebbbbrrrr pushed a commit to eeeebbbbrrrr/pgrx that referenced this pull request Sep 5, 2023
@workingjubilee @eeeebbbbrrrr
Update version to 0.10.0-beta.4 (pgcentralfoundation#1267)
4b84263 
The fifth beta release of 0.10.0!

* `#[pg_test]`-compatible proptest support in
pgcentralfoundation#1258 courtesy of moi
* PostgresEq (pgcentralfoundation#1261) and
PostgresOrd (pgcentralfoundation#1262) now
explicitly require their "base" Rust traits, which may hypothetically
break some impls, but in exchange your equality implementations are now
presumed to be commutative.
* Work on improving/simplifying/optimizing our SPI continues, with
@vrmiguel contributing some polish to Datum preparation in
pgcentralfoundation#1256
* We have been finding some extra-spicy edge cases in how PGRX interacts
with a continuously-operational Postgres installation which might
feature extensions being installed and reinstalled lately! @Sasasu fixed
at least one core-dump-causing case from `cargo pgrx install` in
pgcentralfoundation#1263

## New Contributors
* @vrmiguel made their first contribution in
pgcentralfoundation#1256

**Full Changelog**:
pgcentralfoundation/pgrx@v0.10.0-beta.3...v0.10.0-beta.4
@eeeebbbbrrrr eeeebbbbrrrr mentioned this pull request Sep 5, 2023
Merged
eeeebbbbrrrr added a commit that referenced this pull request Sep 5, 2023
@eeeebbbbrrrr
Update version to 0.10.0 (#1280)
3d1c8e3 
This is the final release of v0.10.0. Thanks everyone for the beta
testing, pull requests, issues, and patience.

As always, install `cargo-pgrx` with `cargo install cargo-pgrx --locked`
and update your extension Cargo.toml files to use the `0.10.0` pgrx
dependencies.

This release includes support for Postgres 16RC1. Support for the
previous betas has been removed. As such, a fresh `cargo pgrx init` is
required.

## What's Changed Since v0.10.0-beta.4

* Fix `GetMemoryChunkContext` port by @workingjubilee in
#1273
* Better error messages when `pg_config` isn't found. by @eeeebbbbrrrr
in #1271
* Make `PostgresHash` also need `Eq` by @workingjubilee in
#1264
* Memoize git hash and extension metadata by @levkk in
#1274
* move to pg16rc1 by @eeeebbbbrrrr in
#1276
* Fix bgworker template up to 0.10.0-beta.4 by @workingjubilee in
#1270

## New Contributors
* @levkk made their first contribution in
#1274

**Changelog**:
v0.10.0-beta.4...v0.10.0

---

v0.10.0's full set of changes throughout the entire beta period are:

* Postgres 16beta1 Support by @eeeebbbbrrrr in
#1169
* Support building against macOS universal binaries by @clowder in
#1166
* list specific versions in feature gates by @eeeebbbbrrrr in
#1175
* Fix bug with converting a `pg_sys::Datum` into a `pgrx::Date` by
@eeeebbbbrrrr in #1177
* Fix Arrays with leading nulls by @eeeebbbbrrrr in
#1180
* Disable hello_versioned_so test by @workingjubilee in
#1192
* doc: fix link broken by @yihong0618 in
#1181
* fcinfo: fix incorrect length set in unsafe code by @Sasasu in
#1190
* update to pg16beta2 support by @eeeebbbbrrrr in
#1188
* Array-walking is aligned by @workingjubilee in
#1191
* Implement PGRXSharedMemory for Deque by @feikesteenbergen in
#1170
* Include security labels header by @daamien in
#1189
* Fixes macos-11 tests by @BradyBonnette in
#1197
* Pgcentralfoundation updates again by @eeeebbbbrrrr in
#1200
* Update version to 0.10.0-beta.0 by @eeeebbbbrrrr in
#1201
* Testing help by @eeeebbbbrrrr in
#1203
* Type testability cleanup by @eeeebbbbrrrr in
#1204
* Try to smartly propagate fs errors by @workingjubilee in
#1186
* Fix issue #1209 by @eeeebbbbrrrr in
#1210
* Type roundtrip tests by @eeeebbbbrrrr in
#1185
* Update version to 0.10.0-beta.1 by @eeeebbbbrrrr in
#1213
* Add a workaround for the pg16/homebrew/icu4c situation by @thomcc in
#1206
* Add security policy by @johnrballard in
#1207
* `AnyNumeric` is no longer backed by Postgres-allocated memory by
@eeeebbbbrrrr in #1216
* Modularize pgrx::spi by @workingjubilee in
#1219
* Stop SpiClient soundness from regressing by @workingjubilee in
#1214
* Add foreign table headers by @workingjubilee in
#1226
* Modularize the interior of pgrx-pg-sys by @workingjubilee in
#1227
* Initial valgrind support by @thomcc in
#1218
* Add support for handling SIGINT and SIGCHLD from bgworker by @JelteF
in #1229
* Ignores UI tests for MUSL environments by @BradyBonnette in
#1235
* Add a env flag that can be set to skip `#[pg_test]`-generated tests.
by @thomcc in #1239
* Fix issue #1076: Properly handle dependency graph of `Result<T, _>` by
@eeeebbbbrrrr in #1241
* Cleanup the error when cargo-pgrx version doesn't match Cargo.toml by
@eeeebbbbrrrr in #1240
* Add operator and cache related api by @VoVAllen in
#1242
* Addresses cargo-pgrx error reporting by @BradyBonnette in
#1238
* Update version to 0.10.0-beta.2 by @eeeebbbbrrrr in
#1244
* Bump cargo-metadata and clap-cargo by @thomcc in
#1246
* Derive Clone for Inet by @JelteF in
#1251
* Correct docs for datetime `From` impls by @workingjubilee in
#1253
* Only enable line tables for profile.dev by @thomcc in
#1249
* Remove references to master branch by @thomcc in
#1243
* Ensure bindgen gets all the `cppflags` it needs (on macOS, anyway) by
@thomcc in #1247
* update for pg16beta3 support by @eeeebbbbrrrr in
#1254
* Update version to 0.10.0-beta.3 by @eeeebbbbrrrr in
#1255
* Add proptest support by @workingjubilee in
#1258
* Misc reformatting and typo fixes by @workingjubilee in
#1260
* spi: simplify (optimize?) Datum preparation by @vrmiguel in
#1256
* Assume commutation when deriving PostgresEq by @workingjubilee in
#1261
* Demand Ord for PostgresOrd by @workingjubilee in
#1262
* Fix pgrx install causing postgresql coredump by @Sasasu in
#1263
* Update version to 0.10.0-beta.4 by @workingjubilee in
#1267

## New Contributors
* @clowder made their first contribution in
#1166
* @yihong0618 made their first contribution in
#1181
* @Sasasu made their first contribution in
#1190
* @daamien made their first contribution in
#1189
* @johnrballard made their first contribution in
#1207
* @VoVAllen made their first contribution in
#1242
* @vrmiguel made their first contribution in
#1256


**Full Changelog**:
v0.9.8...v0.10.0
@bayandin bayandin mentioned this pull request Sep 6, 2023
Closed
usamoi pushed a commit to tensorchord/pgrx that referenced this pull request Mar 6, 2025
@workingjubilee
Demand Ord for PostgresOrd (pgcentralfoundation#1262)
9b609df 
Previously, the `left.cmp(&right)` call could resolve to `Iterator::cmp`.
A malicious impl could even force it to use `fn(&mut self, rhs: &Self)`!
Code exploiting this bug is unlikely, but it could hit other errors,
so this way the error emitted is much nicer.

Also makes the previous trybuild test for PostgresEq more realistic.
usamoi pushed a commit to tensorchord/pgrx that referenced this pull request Mar 6, 2025
@workingjubilee
Update version to 0.10.0-beta.4 (pgcentralfoundation#1267)
6ca9e37 
The fifth beta release of 0.10.0!

* `#[pg_test]`-compatible proptest support in
pgcentralfoundation#1258 courtesy of moi
* PostgresEq (pgcentralfoundation#1261) and
PostgresOrd (pgcentralfoundation#1262) now
explicitly require their "base" Rust traits, which may hypothetically
break some impls, but in exchange your equality implementations are now
presumed to be commutative.
* Work on improving/simplifying/optimizing our SPI continues, with
@vrmiguel contributing some polish to Datum preparation in
pgcentralfoundation#1256
* We have been finding some extra-spicy edge cases in how PGRX interacts
with a continuously-operational Postgres installation which might
feature extensions being installed and reinstalled lately! @Sasasu fixed
at least one core-dump-causing case from `cargo pgrx install` in
pgcentralfoundation#1263

## New Contributors
* @vrmiguel made their first contribution in
pgcentralfoundation#1256

**Full Changelog**:
pgcentralfoundation/pgrx@v0.10.0-beta.3...v0.10.0-beta.4
usamoi pushed a commit to tensorchord/pgrx that referenced this pull request Mar 6, 2025
@eeeebbbbrrrr
Update version to 0.10.0 (pgcentralfoundation#1280)
d063ce9 
This is the final release of v0.10.0. Thanks everyone for the beta
testing, pull requests, issues, and patience.

As always, install `cargo-pgrx` with `cargo install cargo-pgrx --locked`
and update your extension Cargo.toml files to use the `0.10.0` pgrx
dependencies.

This release includes support for Postgres 16RC1. Support for the
previous betas has been removed. As such, a fresh `cargo pgrx init` is
required.

## What's Changed Since v0.10.0-beta.4

* Fix `GetMemoryChunkContext` port by @workingjubilee in
pgcentralfoundation#1273
* Better error messages when `pg_config` isn't found. by @eeeebbbbrrrr
in pgcentralfoundation#1271
* Make `PostgresHash` also need `Eq` by @workingjubilee in
pgcentralfoundation#1264
* Memoize git hash and extension metadata by @levkk in
pgcentralfoundation#1274
* move to pg16rc1 by @eeeebbbbrrrr in
pgcentralfoundation#1276
* Fix bgworker template up to 0.10.0-beta.4 by @workingjubilee in
pgcentralfoundation#1270

## New Contributors
* @levkk made their first contribution in
pgcentralfoundation#1274

**Changelog**:
pgcentralfoundation/pgrx@v0.10.0-beta.4...v0.10.0

---

v0.10.0's full set of changes throughout the entire beta period are:

* Postgres 16beta1 Support by @eeeebbbbrrrr in
pgcentralfoundation#1169
* Support building against macOS universal binaries by @clowder in
pgcentralfoundation#1166
* list specific versions in feature gates by @eeeebbbbrrrr in
pgcentralfoundation#1175
* Fix bug with converting a `pg_sys::Datum` into a `pgrx::Date` by
@eeeebbbbrrrr in pgcentralfoundation#1177
* Fix Arrays with leading nulls by @eeeebbbbrrrr in
pgcentralfoundation#1180
* Disable hello_versioned_so test by @workingjubilee in
pgcentralfoundation#1192
* doc: fix link broken by @yihong0618 in
pgcentralfoundation#1181
* fcinfo: fix incorrect length set in unsafe code by @Sasasu in
pgcentralfoundation#1190
* update to pg16beta2 support by @eeeebbbbrrrr in
pgcentralfoundation#1188
* Array-walking is aligned by @workingjubilee in
pgcentralfoundation#1191
* Implement PGRXSharedMemory for Deque by @feikesteenbergen in
pgcentralfoundation#1170
* Include security labels header by @daamien in
pgcentralfoundation#1189
* Fixes macos-11 tests by @BradyBonnette in
pgcentralfoundation#1197
* Pgcentralfoundation updates again by @eeeebbbbrrrr in
pgcentralfoundation#1200
* Update version to 0.10.0-beta.0 by @eeeebbbbrrrr in
pgcentralfoundation#1201
* Testing help by @eeeebbbbrrrr in
pgcentralfoundation#1203
* Type testability cleanup by @eeeebbbbrrrr in
pgcentralfoundation#1204
* Try to smartly propagate fs errors by @workingjubilee in
pgcentralfoundation#1186
* Fix issue pgcentralfoundation#1209 by @eeeebbbbrrrr in
pgcentralfoundation#1210
* Type roundtrip tests by @eeeebbbbrrrr in
pgcentralfoundation#1185
* Update version to 0.10.0-beta.1 by @eeeebbbbrrrr in
pgcentralfoundation#1213
* Add a workaround for the pg16/homebrew/icu4c situation by @thomcc in
pgcentralfoundation#1206
* Add security policy by @johnrballard in
pgcentralfoundation#1207
* `AnyNumeric` is no longer backed by Postgres-allocated memory by
@eeeebbbbrrrr in pgcentralfoundation#1216
* Modularize pgrx::spi by @workingjubilee in
pgcentralfoundation#1219
* Stop SpiClient soundness from regressing by @workingjubilee in
pgcentralfoundation#1214
* Add foreign table headers by @workingjubilee in
pgcentralfoundation#1226
* Modularize the interior of pgrx-pg-sys by @workingjubilee in
pgcentralfoundation#1227
* Initial valgrind support by @thomcc in
pgcentralfoundation#1218
* Add support for handling SIGINT and SIGCHLD from bgworker by @JelteF
in pgcentralfoundation#1229
* Ignores UI tests for MUSL environments by @BradyBonnette in
pgcentralfoundation#1235
* Add a env flag that can be set to skip `#[pg_test]`-generated tests.
by @thomcc in pgcentralfoundation#1239
* Fix issue pgcentralfoundation#1076: Properly handle dependency graph of `Result<T, _>` by
@eeeebbbbrrrr in pgcentralfoundation#1241
* Cleanup the error when cargo-pgrx version doesn't match Cargo.toml by
@eeeebbbbrrrr in pgcentralfoundation#1240
* Add operator and cache related api by @VoVAllen in
pgcentralfoundation#1242
* Addresses cargo-pgrx error reporting by @BradyBonnette in
pgcentralfoundation#1238
* Update version to 0.10.0-beta.2 by @eeeebbbbrrrr in
pgcentralfoundation#1244
* Bump cargo-metadata and clap-cargo by @thomcc in
pgcentralfoundation#1246
* Derive Clone for Inet by @JelteF in
pgcentralfoundation#1251
* Correct docs for datetime `From` impls by @workingjubilee in
pgcentralfoundation#1253
* Only enable line tables for profile.dev by @thomcc in
pgcentralfoundation#1249
* Remove references to master branch by @thomcc in
pgcentralfoundation#1243
* Ensure bindgen gets all the `cppflags` it needs (on macOS, anyway) by
@thomcc in pgcentralfoundation#1247
* update for pg16beta3 support by @eeeebbbbrrrr in
pgcentralfoundation#1254
* Update version to 0.10.0-beta.3 by @eeeebbbbrrrr in
pgcentralfoundation#1255
* Add proptest support by @workingjubilee in
pgcentralfoundation#1258
* Misc reformatting and typo fixes by @workingjubilee in
pgcentralfoundation#1260
* spi: simplify (optimize?) Datum preparation by @vrmiguel in
pgcentralfoundation#1256
* Assume commutation when deriving PostgresEq by @workingjubilee in
pgcentralfoundation#1261
* Demand Ord for PostgresOrd by @workingjubilee in
pgcentralfoundation#1262
* Fix pgrx install causing postgresql coredump by @Sasasu in
pgcentralfoundation#1263
* Update version to 0.10.0-beta.4 by @workingjubilee in
pgcentralfoundation#1267

## New Contributors
* @clowder made their first contribution in
pgcentralfoundation#1166
* @yihong0618 made their first contribution in
pgcentralfoundation#1181
* @Sasasu made their first contribution in
pgcentralfoundation#1190
* @daamien made their first contribution in
pgcentralfoundation#1189
* @johnrballard made their first contribution in
pgcentralfoundation#1207
* @VoVAllen made their first contribution in
pgcentralfoundation#1242
* @vrmiguel made their first contribution in
pgcentralfoundation#1256


**Full Changelog**:
pgcentralfoundation/pgrx@v0.9.8...v0.10.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thomcc thomcc thomcc approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@workingjubilee @thomcc