Upgrade to pgx 0.5.0 #547

syvb · 2022-09-28T15:16:25Z

Upgrades to pgx 0.5.0-beta.1 (0.5.0 will hopefully be released in 0-7 days). I originally tested on another branch with a lot of code commented out; this branch cherry picks the commits that fix errors but not the ones that comment code out.

To do:

Fix the three failing tests in datum_utils.rs (Include type when getting argument pgcentralfoundation/pgrx#740)
Remove workaround for lack of Copy on some types
Make update tester work in CI (Binary update testing in CI #552)
Change pgx version from 0.5.0-beta.1 to 0.5.0
Publish new CI docker image with pgx 0.5.0 right before merging (Update Docker image for pgx 0.5.0 #571)

Future work:

use pgx::prelude::* instead of pgx::* #586
Use native pgx types instead of crate::raw
Use #[pg_aggregate] instead of our own aggregate builder macro
Misuse of SpiClient::select in some tests #529

extension/src/time_vector/pipeline.rs

syvb · 2022-09-29T18:15:45Z

extension/src/lib.rs

@@ -4,6 +4,10 @@
 #![allow(clippy::extra_unused_lifetimes)]
 // every function calling in_aggregate_context should be unsafe
 #![allow(clippy::not_unsafe_ptr_arg_deref)]
+// since 0.5 pgx requires non-elided lifetimes on extern functions: https://github.com/tcdi/pgx/issues/721
+#![allow(clippy::needless_lifetimes)]


pgx 0.5.0 now requires there to be explicit lifetimes in some cases, despite Clippy claiming they can be removed. I added explicit lifetimes to all of the (many) functions that require them now.

syvb · 2022-09-29T18:15:58Z

Current status:

All tests pass except for these three in datum_utils
- All three tests error with type with OID 0 does not exist
- I'm not very familiar with DatumStore so I'm not sure what the issue is there
The CI fails since the Docker image hasn't been updated to have a newer cargo-pgx yet
update-tester doesn't work with pgx 0.5; hopefully we can move to binary-based testing before merging this to avoid the need to add support a third pgx version to the update tester

559: Basic test for space saving aggregate serialization r=Smittyvb a=Smittyvb I noticed while working on #547 that we don't have any tests for `SpaceSavingAggregate`s. This PR adds a simple test to verify that the `raw_freq_agg` function will at least run without failing. This test currently fails on #547, which is good since that PR causes a bug that leads to `SpaceSavingAggregate` not working at all. Co-authored-by: Smitty <smitty@timescale.com>

epgts · 2022-10-07T23:10:33Z

@JLockerman can we get your eyes on this next week?

Thanks!

__CARGO_FIX_YOLO=1 cargo fix --broken-code

syvb · 2022-10-11T21:46:13Z

The CI now passes when I manually run it using the testing Docker image from #571: https://github.com/timescale/timescaledb-toolkit/actions/runs/3230387310/jobs/5288778595

syvb · 2022-10-11T21:48:27Z

extension/src/state_aggregate.rs

-    TableIterator::new(agg.durations.clone().into_iter().map(move |record| {
-        let beg = record.state_beg as usize;
-        let end = record.state_end as usize;
-        (states[beg..end].to_owned(), record.duration)
-    }))
+    TableIterator::new(
+        agg.durations
+            .clone()
+            .into_iter()
+            .map(move |record| {
+                let beg = record.state_beg as usize;
+                let end = record.state_end as usize;
+                (states[beg..end].to_owned(), record.duration)
+            })
+            .collect::<Vec<_>>()
+            .into_iter(),
+    )


pgx requires that a TableIterator/SetOfIterator only see values allocated from Rust (otherwise you get use-after-frees). To comply with this requirement I collect the iterator into a Vec then turn it back into an iterator.

WireBaron

Really nice work.

syvb · 2022-10-14T19:28:24Z

Note that pgx ~~0.5.1~~ 0.5.2 will be released soon. All ~~0.5.1~~ 0.5.2 does it fix a bug with pg_sys::Point. We don't use Point anywhere so we don't need to update to ~~0.5.1~~ 0.5.2. We can wait until whatever release adds pg15 support (probably ~~0.5.2~~ 0.5.3) to update pgx again.

571: Update Docker image for pgx 0.5.0 r=Smittyvb a=Smittyvb pgx 0.5.0 just released! This PR updates the CI to have pgx 0.5.0 and adds pgx 0.5.0 to the `$PATH` instead of pgx 0.4.5. This PR should be merged right before merging #547 so that the CI can pass on that PR before merging it. Co-authored-by: Smitty <smitty@timescale.com>

syvb · 2022-10-17T17:18:37Z

bors r+

bors · 2022-10-17T17:35:52Z

Build succeeded:

syvb requested review from JLockerman and removed request for JLockerman September 28, 2022 16:15

syvb changed the title ~~WIP: pgx 0.5~~ Upgrade to pgx 0.5 Sep 29, 2022

syvb force-pushed the sv/pgx-0.5 branch from 02b9b74 to b6b78af Compare September 29, 2022 17:56

syvb commented Sep 29, 2022

View reviewed changes

extension/src/time_vector/pipeline.rs Outdated Show resolved Hide resolved

syvb commented Sep 29, 2022

View reviewed changes

syvb requested review from thatzopoulos, WireBaron, epgts and rtwalker September 29, 2022 18:25

syvb changed the title ~~Upgrade to pgx 0.5~~ Upgrade to pgx 0.5.0 Sep 29, 2022

syvb mentioned this pull request Oct 4, 2022

Basic test for space saving aggregate serialization #559

Merged

syvb force-pushed the sv/pgx-0.5 branch from 5a6fa33 to e3d5dc6 Compare October 5, 2022 15:18

syvb force-pushed the sv/pgx-0.5 branch 2 times, most recently from eb6c2e9 to f81179a Compare October 7, 2022 19:20

syvb mentioned this pull request Oct 7, 2022

Update Docker image for pgx 0.5.0 #571

Merged

epgts requested a review from JLockerman October 7, 2022 23:10

syvb marked this pull request as ready for review October 11, 2022 13:08

syvb added 9 commits October 11, 2022 13:52

Bump pgx to beta release of 0.5.0

a111014

Autofix some errors

86fb388

__CARGO_FIX_YOLO=1 cargo fix --broken-code

Fix some simple errors

ee54291

Datum fixes

5ce7248

Implement SqlTranslatable for raw types

15e3c04

more datum fixes

6b7a85b

Fix even more pgx errors

53998de

more fixes

f02b5c8

Wrap datums for usage with hyperloglog

53dbc6f

syvb added 11 commits October 11, 2022 13:52

Fix all the lifetime errors

c16809b

Hacky fix for copying a Const

4b2238a

Fix errors in tests

318b507

Fix accidentally commented out HLL ops

85f9141

fmt

d207757

Fix warning

25e929c

Fix clippy lints

4e0ff3e

Fix how datums get pointerified

6ec2224

Support pgx develop branch

d31fd23

Match upstream pgx

5bf0743

Use pgx 0.5.0 now that it's released

fa28ae5

syvb force-pushed the sv/pgx-0.5 branch from a984f98 to fa28ae5 Compare October 11, 2022 17:52

syvb commented Oct 11, 2022

View reviewed changes

Collect to vec to prevent use after free

fe8b386

syvb force-pushed the sv/pgx-0.5 branch from bedbc6c to fe8b386 Compare October 11, 2022 22:27

WireBaron approved these changes Oct 12, 2022

View reviewed changes

jerryxwu mentioned this pull request Oct 14, 2022

Postgres 15 support #579

Closed

3 tasks

thatzopoulos approved these changes Oct 14, 2022

View reviewed changes

bors bot merged commit fa7b32f into main Oct 17, 2022

bors bot deleted the sv/pgx-0.5 branch October 17, 2022 17:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade to pgx 0.5.0 #547

Upgrade to pgx 0.5.0 #547

syvb commented Sep 28, 2022 •

edited

Loading

syvb Sep 29, 2022 •

edited

Loading

syvb commented Sep 29, 2022 •

edited

Loading

epgts commented Oct 7, 2022

syvb commented Oct 11, 2022 •

edited

Loading

syvb Oct 11, 2022

WireBaron left a comment

syvb commented Oct 14, 2022 •

edited

Loading

syvb commented Oct 17, 2022

bors bot commented Oct 17, 2022

Upgrade to pgx 0.5.0 #547

Upgrade to pgx 0.5.0 #547

Conversation

syvb commented Sep 28, 2022 • edited Loading

syvb Sep 29, 2022 • edited Loading

Choose a reason for hiding this comment

syvb commented Sep 29, 2022 • edited Loading

epgts commented Oct 7, 2022

syvb commented Oct 11, 2022 • edited Loading

syvb Oct 11, 2022

Choose a reason for hiding this comment

WireBaron left a comment

Choose a reason for hiding this comment

syvb commented Oct 14, 2022 • edited Loading

syvb commented Oct 17, 2022

bors bot commented Oct 17, 2022

syvb commented Sep 28, 2022 •

edited

Loading

syvb Sep 29, 2022 •

edited

Loading

syvb commented Sep 29, 2022 •

edited

Loading

syvb commented Oct 11, 2022 •

edited

Loading

syvb commented Oct 14, 2022 •

edited

Loading