From 91b9bf807383478a5e526b602513507a8895a682 Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Thu, 9 Nov 2023 20:37:29 +0100 Subject: [PATCH] Ensure PostgreSQL 16 compatibility (#776) * Ensure PostgreSQL 16 compatibility DateTimeParseError: PostgreSQL source code documents that we can pass null pointers here: > extra can be NULL if not needed for the particular dterr value. > [...] > If escontext points to an ErrorSaveContext node, that is filled instead > of throwing an error DecodeDateTime however does not allow us to pass a null pointer, reading the source code, it is expected that extra is not null. Therefore, in order for us to get this thing to compile, we create an empty DateTimeErrorExtra, which gets passed along. * Ensure PostgreSQL 16 is tested To achieve this, we have had to change quite a few things: Explicitly list our Docker Image builds The logic in the json was pretty complex, by using more yamls, and lists we hope to achieve a workflow that is easier to inspect, reason with and adapt. We also need to reintroduce the difference between $PG_VERSIONS and $TSDB_PG_VERSIONS, as PostgreSQL 16 support is not yet available for TimescaleDB. * Print debugging output on failure to start PG * Explictly set materialized_only=false for test hypertable creations * Skip some unsupported tests on PG16 * Skip PostgreSQL 16 doc tests --------- Co-authored-by: syvb --- .github/workflows/ci.yml | 83 +++++++++++++++++--------------- Readme.md | 2 +- docker/ci/setup.sh | 16 ++++-- docs/counter_agg.md | 2 +- docs/percentile_approximation.md | 4 +- docs/state_agg.md | 2 +- docs/tdigest.md | 2 +- docs/test_caggs.md | 2 +- docs/test_candlestick_agg.md | 2 +- docs/time_weighted_average.md | 2 +- docs/two-step_aggregation.md | 2 +- docs/uddsketch.md | 2 +- extension/Cargo.toml | 3 +- extension/src/serialization.rs | 44 ++++++++++++++++- tools/build | 6 +-- tools/dependencies.sh | 6 +-- tools/testbin | 3 +- 17 files changed, 120 insertions(+), 63 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3f85cb9c..f783bd98 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -39,43 +39,47 @@ jobs: fail-fast: false max-parallel: 12 matrix: - pgversion: [12, 13, 14, 15] - container: "${{ \ - ( \ - inputs.all-platforms || \ - ( \ - github.event_name == 'schedule' && \ - github.event.schedule == '0 6 * * 1-4' \ - ) \ - ) && \ - fromJSON('[ \ - { \"os\": \"debian\", \"version\": \"10\", \"image\": \"debian-10-amd64\" }, \ - { \"os\": \"debian\", \"version\": \"11\", \"image\": \"debian-11-amd64\" }, \ - { \"os\": \"rockylinux\", \"version\": \"8\", \"image\": \"rockylinux-8-x86_64\" }, \ - { \"os\": \"rockylinux\", \"version\": \"9\", \"image\": \"rockylinux-9-x86_64\" }, \ - { \"os\": \"centos\", \"version\": \"7\", \"image\": \"centos-7-x86_64\" }, \ - { \"os\": \"ubuntu\", \"version\": \"20.04\", \"image\": \"ubuntu-20.04-amd64\" }, \ - { \"os\": \"ubuntu\", \"version\": \"22.04\", \"image\": \"ubuntu-22.04-amd64\" } \ - ]') \ - || fromJSON('[ \ - { \"os\": \"debian\", \"version\": \"11\", \"image\": \"debian-11-amd64\" }, \ - { \"os\": \"rockylinux\", \"version\": \"9\", \"image\": \"rockylinux-9-x86_64\" }, \ - { \"os\": \"centos\", \"version\": \"7\", \"image\": \"centos-7-x86_64\" } \ - ]') \ - }}" - # Building TSDB in CI is only supported on Debian/Ubuntu - exclude: "${{ \ - ( \ - (github.event_name == 'schedule' && github.event.schedule == '0 8 * * 1-4') \ - || inputs.tsdb-commit != '' \ - ) && \ - fromJSON('[ \ - {\"container\": {\"os\": \"centos\"}}, \ - {\"container\": {\"os\": \"rockylinux\"}} \ - ]') || \ - fromJSON('[]') \ - }}" + pgversion: [12, 13, 14, 15, 16] + container: + - os: rockylinux + version: "9" + image: rockylinux-9-x86_64 + # Building TSDB in CI is only supported on Debian/Ubuntu + skip: ${{ inputs.tsdb-commit != '' }} + schedule: ${{ !(github.event_name == 'schedule' && github.event.schedule == '0 8 * * 1-4') }} + - os: centos + version: "7" + image: centos-7-x86_64 + # Building TSDB in CI is only supported on Debian/Ubuntu + skip: ${{ inputs.tsdb-commit != '' }} + schedule: ${{ !(github.event_name == 'schedule' && github.event.schedule == '0 8 * * 1-4') }} + - os: debian + version: "11" + image: debian-11-amd64 + schedule: true + - os: debian + version: "10" + image: debian-10-amd64 + schedule: ${{ inputs.all-platforms || ( github.event_name == 'schedule' && github.event.schedule == '0 6 * * 1-4' ) }} + - os: ubuntu + version: "20.04" + image: ubuntu-20.04-amd64 + schedule: ${{ inputs.all-platforms || ( github.event_name == 'schedule' && github.event.schedule == '0 6 * * 1-4' ) }} + - os: ubuntu + version: "22.04" + image: ubuntu-22.04-amd64 + schedule: ${{ inputs.all-platforms || ( github.event_name == 'schedule' && github.event.schedule == '0 6 * * 1-4' ) }} + exclude: + - container: + skip: true + - container: + schedule: false + # CentOS 7 does not have PostgreSQL 16 packages + - container: + os: centos + version: "7" + pgversion: 16 env: # TODO Why? Cargo default is to pass `-C incremental` to rustc; why don't we want that? # https://doc.rust-lang.org/rustc/codegen-options/index.html#incremental @@ -133,14 +137,17 @@ jobs: su postgres -c 'sh tools/build -pg${{ matrix.pgversion }} test-extension 2>&1' - name: Run doc tests + # depends on TSDB, which requires PG >=13 + if: ${{ matrix.pgversion >= 13 && (matrix.pgversion >= 13 && (matrix.pgversion <= 15 || ((github.event_name == 'schedule' && github.event.schedule == '0 8 * * 1-4') || inputs.tsdb-commit != ''))) }} run: su postgres -c 'sh tools/build -pg${{ matrix.pgversion }} test-doc 2>&1' - name: Run binary update tests (deb) - if: ${{ matrix.container.os == 'debian' || matrix.container.os == 'ubuntu' }} + # depends on TSDB, which requires PG >=13 + if: ${{ (matrix.container.os == 'debian' || matrix.container.os == 'ubuntu') && (matrix.pgversion >= 13 && (matrix.pgversion <= 16 || ((github.event_name == 'schedule' && github.event.schedule == '0 8 * * 1-4') || inputs.tsdb-commit != ''))) }} run: | su postgres -c 'OS_NAME=${{ matrix.container.os }} OS_VERSION=${{ matrix.container.version }} tools/testbin -version no -bindir / -pgversions ${{ matrix.pgversion }} ci 2>&1' - name: Run binary update tests (EL) - if: ${{ (matrix.container.os == 'rockylinux' || matrix.container.os == 'centos') && matrix.container.version != '9' }} + if: ${{ (matrix.container.os == 'rockylinux' || matrix.container.os == 'centos') && matrix.container.version != '9' && (matrix.pgversion >= 13 && (matrix.pgversion <= 16 || ((github.event_name == 'schedule' && github.event.schedule == '0 8 * * 1-4') || inputs.tsdb-commit != ''))) }} run: | su postgres -c 'OS_NAME=${{ matrix.container.os }} OS_VERSION=${{ matrix.container.version }} tools/testbin -version no -bindir / -pgversions ${{ matrix.pgversion }} rpm_ci 2>&1' diff --git a/Readme.md b/Readme.md index 97a39dc5..451315df 100644 --- a/Readme.md +++ b/Readme.md @@ -96,7 +96,7 @@ hop on the [Discussions forum](https://github.com/timescale/timescaledb-toolkit/ See above for prerequisites and installation instructions. -You can run tests against a postgres version `pg12`, `pg13`, `pg14`, or `pg15` using +You can run tests against a postgres version `pg12`, `pg13`, `pg14`, `pg15` or `pg16` using ``` cargo pgrx test ${postgres_version} diff --git a/docker/ci/setup.sh b/docker/ci/setup.sh index 651c0074..250c2e4f 100755 --- a/docker/ci/setup.sh +++ b/docker/ci/setup.sh @@ -133,10 +133,13 @@ EOF for pg in $PG_VERSIONS; do yum -q -y install \ postgresql$pg-devel \ - postgresql$pg-server \ - timescaledb-2-postgresql-$pg + postgresql$pg-server # We install as user postgres, so that needs write access to these. chown $BUILDER_USERNAME $PG_BASE$pg/lib $PG_BASE$pg/share/extension + done; + + for pg in $TSDB_PG_VERSIONS; do + yum -q -y install timescaledb-2-postgresql-$pg done gem install fpm -v $FPM_VERSION -N @@ -198,12 +201,15 @@ EOF apt-get -qq install \ postgresql-$pg \ postgresql-server-dev-$pg - # timescaledb packages Recommend toolkit, which we don't want here. - apt-get -qq install --no-install-recommends timescaledb-2-postgresql-$pg # We install as user postgres, so that needs write access to these. chown $BUILDER_USERNAME $PG_BASE$pg/lib /usr/share/postgresql/$pg/extension done + for pg in $TSDB_PG_VERSIONS; do + # timescaledb packages Recommend toolkit, which we don't want here. + apt-get -qq install --no-install-recommends timescaledb-2-postgresql-$pg + done + # Ubuntu is the only system we want an image for that sticks an extra # copy of the default PATH into PAM's /etc/environment and we su or sudo # to $BUILDER_USERNAME thereby picking up that PATH and clobbering the @@ -244,7 +250,7 @@ for pg in $PG_VERSIONS; do done cargo pgrx init $init_flags ## Initialize pgrx-managed databases so we can add the timescaledb load. -for pg in $PG_VERSIONS; do +for pg in $TSDB_PG_VERSIONS; do echo "shared_preload_libraries = 'timescaledb'" >> ~/.pgrx/data-$pg/postgresql.conf done diff --git a/docs/counter_agg.md b/docs/counter_agg.md index df6f96ad..337e29f2 100644 --- a/docs/counter_agg.md +++ b/docs/counter_agg.md @@ -79,7 +79,7 @@ Now we can make our continuous aggregate: ```SQL ,ignore CREATE MATERIALIZED VIEW foo_15 -WITH (timescaledb.continuous) +WITH (timescaledb.continuous, timescaledb.materialized_only=false) AS SELECT measure_id, time_bucket('15 min'::interval, ts) as bucket, counter_agg(ts, val, bounds => time_bucket_range('15 min'::interval, ts)) diff --git a/docs/percentile_approximation.md b/docs/percentile_approximation.md index d812e939..7433b396 100644 --- a/docs/percentile_approximation.md +++ b/docs/percentile_approximation.md @@ -200,7 +200,7 @@ Let's do this with our example, we can't use `percentile_disc` anymore as ordere ```SQL , non-transactional, ignore-output CREATE MATERIALIZED VIEW response_times_hourly -WITH (timescaledb.continuous) +WITH (timescaledb.continuous, timescaledb.materialized_only=false) AS SELECT time_bucket('1 h'::interval, ts) as bucket, api_id, @@ -379,7 +379,7 @@ They are often used to create [continuous aggregates]() after which we can use m ```SQL ,ignore CREATE MATERIALIZED VIEW foo_hourly -WITH (timescaledb.continuous) +WITH (timescaledb.continuous, timescaledb.materialized_only=false) AS SELECT time_bucket('1 h'::interval, ts) as bucket, percentile_agg(value) as pct_agg diff --git a/docs/state_agg.md b/docs/state_agg.md index 1aa9ee8c..f5afc403 100644 --- a/docs/state_agg.md +++ b/docs/state_agg.md @@ -749,7 +749,7 @@ VALUES ```SQL ,non-transactional,ignore-output CREATE MATERIALIZED VIEW sa -WITH (timescaledb.continuous) AS +WITH (timescaledb.continuous, timescaledb.materialized_only=false) AS SELECT time_bucket('1 minute'::interval, ts) AS bucket, id, state_agg(ts, status) AS agg diff --git a/docs/tdigest.md b/docs/tdigest.md index f34b3752..3f10aac2 100644 --- a/docs/tdigest.md +++ b/docs/tdigest.md @@ -108,7 +108,7 @@ Next a materialized view with the timescaledb.continuous property is added. Thi including the tdigest in this case, up to date as data is added to the table. ```SQL ,non-transactional,ignore-output CREATE MATERIALIZED VIEW weekly_sketch -WITH (timescaledb.continuous) +WITH (timescaledb.continuous, timescaledb.materialized_only=false) AS SELECT time_bucket('7 day'::interval, time) as week, tdigest(100, value) as digest diff --git a/docs/test_caggs.md b/docs/test_caggs.md index f00358d0..ccc7e52c 100644 --- a/docs/test_caggs.md +++ b/docs/test_caggs.md @@ -17,7 +17,7 @@ SELECT create_hypertable('test', 'time'); ## Setup continuous aggs ```SQL ,non-transactional,ignore-output CREATE MATERIALIZED VIEW weekly_aggs -WITH (timescaledb.continuous) +WITH (timescaledb.continuous, timescaledb.materialized_only=false) AS SELECT time_bucket('7 day'::interval, time) as week, hyperloglog(64, value1) as hll, diff --git a/docs/test_candlestick_agg.md b/docs/test_candlestick_agg.md index a5128fc2..b95c0d4a 100644 --- a/docs/test_candlestick_agg.md +++ b/docs/test_candlestick_agg.md @@ -15,7 +15,7 @@ SELECT create_hypertable('stocks_real_time','time'); ## Setup Continuous Aggs ```SQL,non-transactional,ignore-output CREATE MATERIALIZED VIEW cs -WITH (timescaledb.continuous) AS +WITH (timescaledb.continuous, timescaledb.materialized_only=false) AS SELECT time_bucket('1 minute'::interval, "time") AS ts, symbol, candlestick_agg("time", price, day_volume) AS candlestick diff --git a/docs/time_weighted_average.md b/docs/time_weighted_average.md index d15c3859..d1020f03 100644 --- a/docs/time_weighted_average.md +++ b/docs/time_weighted_average.md @@ -102,7 +102,7 @@ Now we can make our continuous aggregate: ```SQL ,non-transactional, ignore-output CREATE MATERIALIZED VIEW foo_5 -WITH (timescaledb.continuous) +WITH (timescaledb.continuous, timescaledb.materialized_only=false) AS SELECT measure_id, time_bucket('5 min'::interval, ts) as bucket, time_weight('LOCF', ts, val) diff --git a/docs/two-step_aggregation.md b/docs/two-step_aggregation.md index 5927954e..ea166109 100644 --- a/docs/two-step_aggregation.md +++ b/docs/two-step_aggregation.md @@ -127,7 +127,7 @@ Two-step aggregates expose the internal, re-aggregateable form to the user so th ```SQL , ignore CREATE MATERIALIZED VIEW foo_15 -WITH (timescaledb.continuous) +WITH (timescaledb.continuous, timescaledb.materialized_only=false) AS SELECT id, time_bucket('15 min'::interval, ts) as bucket, sum(val), diff --git a/docs/uddsketch.md b/docs/uddsketch.md index 2c06e41d..cf80cd4d 100644 --- a/docs/uddsketch.md +++ b/docs/uddsketch.md @@ -126,7 +126,7 @@ SELECT create_hypertable('test', 'time'); Now we'll create a continuous aggregate which will group all the points for each week into a UddSketch: ```SQL ,non-transactional,ignore-output CREATE MATERIALIZED VIEW weekly_sketch -WITH (timescaledb.continuous) +WITH (timescaledb.continuous, timescaledb.materialized_only=false) AS SELECT time_bucket('7 day'::interval, time) as week, uddsketch(100, 0.005, value) as sketch diff --git a/extension/Cargo.toml b/extension/Cargo.toml index d4102fc1..c283b479 100644 --- a/extension/Cargo.toml +++ b/extension/Cargo.toml @@ -7,11 +7,12 @@ edition = "2021" crate-type = ["cdylib"] [features] -default = ["pg14"] +default = ["pg15"] pg12 = ["pgrx/pg12", "pgrx-tests/pg12"] pg13 = ["pgrx/pg13", "pgrx-tests/pg13"] pg14 = ["pgrx/pg14", "pgrx-tests/pg14"] pg15 = ["pgrx/pg15", "pgrx-tests/pg15"] +pg16 = ["pgrx/pg16", "pgrx-tests/pg16"] pg_test = ["approx"] [dependencies] diff --git a/extension/src/serialization.rs b/extension/src/serialization.rs index 41d2da4a..53f58cfd 100644 --- a/extension/src/serialization.rs +++ b/extension/src/serialization.rs @@ -6,7 +6,10 @@ use std::{ os::raw::{c_char, c_int}, }; -use pgrx::pg_sys; +#[cfg(feature = "pg16")] +use pgrx::pg_sys::DateTimeErrorExtra; +use pgrx::pg_sys::{self}; + use std::ffi::CStr; pub(crate) mod collations; @@ -75,7 +78,27 @@ pub extern "C" fn _ts_toolkit_decode_timestamptz(text: &str) -> i64 { pg_sys::MAXDATEFIELDS as i32, &mut nf, ); + #[cfg(any( + feature = "pg11", + feature = "pg12", + feature = "pg13", + feature = "pg14", + feature = "pg15" + ))] + if dterr == 0 { + dterr = pg_sys::DecodeDateTime( + field.as_mut_ptr(), + ftype.as_mut_ptr(), + nf, + &mut dtype, + tm, + &mut fsec, + &mut tz, + ) + } + #[cfg(feature = "pg16")] if dterr == 0 { + let mut extra = DateTimeErrorExtra::default(); dterr = pg_sys::DecodeDateTime( field.as_mut_ptr(), ftype.as_mut_ptr(), @@ -84,13 +107,32 @@ pub extern "C" fn _ts_toolkit_decode_timestamptz(text: &str) -> i64 { tm, &mut fsec, &mut tz, + &mut extra as *mut DateTimeErrorExtra, ) } + + #[cfg(any( + feature = "pg11", + feature = "pg12", + feature = "pg13", + feature = "pg14", + feature = "pg15" + ))] + if dterr != 0 { + pg_sys::DateTimeParseError( + dterr, + str.as_ptr(), + b"timestamptz\0".as_ptr().cast::(), + ); + } + #[cfg(feature = "pg16")] if dterr != 0 { pg_sys::DateTimeParseError( dterr, + core::ptr::null_mut(), str.as_ptr(), b"timestamptz\0".as_ptr().cast::(), + core::ptr::null_mut(), ); } diff --git a/tools/build b/tools/build index 31ebd02a..1c302b05 100755 --- a/tools/build +++ b/tools/build @@ -20,7 +20,7 @@ usage() { } require_pg_version() { - [ -n "$pg_version" ] || die 'specify one of -pg12 | -pg13 | -pg14 | -pg15' + [ -n "$pg_version" ] || die 'specify one of -pg12 | -pg13 | -pg14 | -pg15 | -pg16' } find_pg_config() { @@ -116,7 +116,7 @@ while [ $# -gt 0 ]; do test-doc) find_profile require_pg_version - $nop cargo pgrx start --package timescaledb_toolkit $pg + $nop cargo pgrx start --package timescaledb_toolkit $pg || (cat /home/postgres/.pgrx/${pg_version}.log; false) $nop cargo run --profile $profile -p sql-doctester -- \ -h localhost \ -p $pg_port \ @@ -130,7 +130,7 @@ while [ $# -gt 0 ]; do find_pg_config require_cargo_pgrx require_cargo_pgrx_old - $nop cargo pgrx start --package timescaledb_toolkit $pg + $nop cargo pgrx start --package timescaledb_toolkit $pg || (cat /home/postgres/.pgrx/${pg_version}.log; false) $nop cargo run --profile $profile --manifest-path tools/update-tester/Cargo.toml -- full-update-test-source \ -h localhost \ -p $pg_port \ diff --git a/tools/dependencies.sh b/tools/dependencies.sh index 22964905..98fe986c 100644 --- a/tools/dependencies.sh +++ b/tools/dependencies.sh @@ -7,9 +7,9 @@ # All our automation scripts read this, so at least we're not duplicating this # information across all those. -PG_VERSIONS='12 13 14 15' -# TODO: remove this once TimescaleDB supports PostgreSQL 15: issue #648 -TSDB_PG_VERSIONS='12 13 14' +PG_VERSIONS='12 13 14 15 16' +# TODO: extend this with 16 this once TimescaleDB supports PostgreSQL 16: issue #5752 +TSDB_PG_VERSIONS='12 13 14 15' CARGO_EDIT=0.11.2 diff --git a/tools/testbin b/tools/testbin index ce4268e5..abeda55c 100755 --- a/tools/testbin +++ b/tools/testbin @@ -103,8 +103,9 @@ skip_from_version() { # - FROM_VERSION # - PG_VERSION skip_from_version_pg_version() { - # skip versions without PG15 binaries + # skip versions without binaries for this PostgreSQL version [ $PG_VERSION -gt 14 ] && [ `cmp_version $FROM_VERSION` -lt 011301 ] && return + [ $PG_VERSION -gt 15 ] && [ `cmp_version $FROM_VERSION` -lt 011801 ] && return } # Requires: