Skip to content

Commit

Permalink
Adds PG17.1 support - Regression tests sanity (#7661)
Browse files Browse the repository at this point in the history
This is the final commit that adds
PG17 compatibility with Citus's current capabilities.

You can use Citus community, release-13.0 branch, with PG17.1.

---------

Specifically, this commit:

- Enables PG17 in the configure script.

- Adds PG17 tests to CI using test images that have 17.1

- Fixes an upgrade test: see below for details
In `citus_prepare_upgrade()`, don't drop any_value when upgrading from
PG16+, because PG16+ has its own any_value function. Attempting to do so
results in the error seen in [pg16-pg17
upgrade](https://github.com/citusdata/citus/actions/runs/11768444117/job/32778340003?pr=7661):
```
ERROR:  cannot drop function any_value(anyelement) because it is required by the database system
CONTEXT:  SQL statement "DROP AGGREGATE IF EXISTS pg_catalog.any_value(anyelement)"
```
When 16 becomes the minimum supported Postgres version, the drop
statements can be removed.

---------

Several PG17 Compatibility commits have been merged before this final one.
All these subtasks are done #7653

See the list below:

Compilation PR: #7699
Ruleutils PR: #7725
Sister PR for tests: citusdata/the-process#159

Helpful smaller PRs:
- #7714
- #7726
- #7731
- #7732
- #7733
- #7738
- #7745
- #7747
- #7748
- #7749
- #7752
- #7755
- #7757
- #7759
- #7760
- #7761
- #7762
- #7765
- #7766
- #7768
- #7769
- #7771
- #7774
- #7776
- #7780
- #7781
- #7785
- #7788
- #7793
- #7796

---------

Co-authored-by: Colm <colmmchugh@microsoft.com>
  • Loading branch information
naisila and colm-mchugh authored Dec 24, 2024
1 parent 743f0eb commit 6fed917
Show file tree
Hide file tree
Showing 8 changed files with 160 additions and 14 deletions.
14 changes: 13 additions & 1 deletion .devcontainer/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,18 @@ RUN mkdir .pgenv-staging/
RUN cp -r .pgenv/src .pgenv/pgsql-* .pgenv/config .pgenv-staging/
RUN rm .pgenv-staging/config/default.conf

FROM base AS pg17
RUN MAKEFLAGS="-j $(nproc)" pgenv build 17.1
RUN rm .pgenv/src/*.tar*
RUN make -C .pgenv/src/postgresql-*/ clean
RUN make -C .pgenv/src/postgresql-*/src/include install

# create a staging directory with all files we want to copy from our pgenv build
# we will copy the contents of the staged folder into the final image at once
RUN mkdir .pgenv-staging/
RUN cp -r .pgenv/src .pgenv/pgsql-* .pgenv/config .pgenv-staging/
RUN rm .pgenv-staging/config/default.conf

FROM base AS uncrustify-builder

RUN sudo apt update && sudo apt install -y cmake tree
Expand Down Expand Up @@ -211,7 +223,7 @@ COPY --chown=citus:citus .psqlrc .
RUN sudo chown --from=root:root citus:citus -R ~

# sets default pg version
RUN pgenv switch 16.5
RUN pgenv switch 17.1

# make connecting to the coordinator easy
ENV PGPORT=9700
40 changes: 35 additions & 5 deletions .github/workflows/build_and_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,13 @@ jobs:
pgupgrade_image_name: "ghcr.io/citusdata/pgupgradetester"
style_checker_image_name: "ghcr.io/citusdata/stylechecker"
style_checker_tools_version: "0.8.18"
sql_snapshot_pg_version: "16.5"
image_suffix: "-v1d9d7d7"
sql_snapshot_pg_version: "17.1"
image_suffix: "-v84c0cf8"
pg14_version: '{ "major": "14", "full": "14.14" }'
pg15_version: '{ "major": "15", "full": "15.9" }'
pg16_version: '{ "major": "16", "full": "16.5" }'
upgrade_pg_versions: "14.14-15.9-16.5"
pg17_version: '{ "major": "17", "full": "17.1" }'
upgrade_pg_versions: "14.14-15.9-16.5-17.1"
steps:
# Since GHA jobs need at least one step we use a noop step here.
- name: Set up parameters
Expand Down Expand Up @@ -108,6 +109,7 @@ jobs:
- ${{ needs.params.outputs.pg14_version }}
- ${{ needs.params.outputs.pg15_version }}
- ${{ needs.params.outputs.pg16_version }}
- ${{ needs.params.outputs.pg17_version }}
runs-on: ubuntu-20.04
container:
image: "${{ matrix.image_name }}:${{ fromJson(matrix.pg_version).full }}${{ matrix.image_suffix }}"
Expand Down Expand Up @@ -139,6 +141,7 @@ jobs:
- ${{ needs.params.outputs.pg14_version }}
- ${{ needs.params.outputs.pg15_version }}
- ${{ needs.params.outputs.pg16_version }}
- ${{ needs.params.outputs.pg17_version }}
make:
- check-split
- check-multi
Expand Down Expand Up @@ -168,6 +171,10 @@ jobs:
pg_version: ${{ needs.params.outputs.pg16_version }}
suite: regress
image_name: ${{ needs.params.outputs.fail_test_image_name }}
- make: check-failure
pg_version: ${{ needs.params.outputs.pg17_version }}
suite: regress
image_name: ${{ needs.params.outputs.fail_test_image_name }}
- make: check-enterprise-failure
pg_version: ${{ needs.params.outputs.pg14_version }}
suite: regress
Expand All @@ -180,6 +187,10 @@ jobs:
pg_version: ${{ needs.params.outputs.pg16_version }}
suite: regress
image_name: ${{ needs.params.outputs.fail_test_image_name }}
- make: check-enterprise-failure
pg_version: ${{ needs.params.outputs.pg17_version }}
suite: regress
image_name: ${{ needs.params.outputs.fail_test_image_name }}
- make: check-pytest
pg_version: ${{ needs.params.outputs.pg14_version }}
suite: regress
Expand All @@ -192,6 +203,10 @@ jobs:
pg_version: ${{ needs.params.outputs.pg16_version }}
suite: regress
image_name: ${{ needs.params.outputs.fail_test_image_name }}
- make: check-pytest
pg_version: ${{ needs.params.outputs.pg17_version }}
suite: regress
image_name: ${{ needs.params.outputs.fail_test_image_name }}
- make: installcheck
suite: cdc
image_name: ${{ needs.params.outputs.test_image_name }}
Expand All @@ -200,6 +215,10 @@ jobs:
suite: cdc
image_name: ${{ needs.params.outputs.test_image_name }}
pg_version: ${{ needs.params.outputs.pg16_version }}
- make: installcheck
suite: cdc
image_name: ${{ needs.params.outputs.test_image_name }}
pg_version: ${{ needs.params.outputs.pg17_version }}
- make: check-query-generator
pg_version: ${{ needs.params.outputs.pg14_version }}
suite: regress
Expand All @@ -212,6 +231,10 @@ jobs:
pg_version: ${{ needs.params.outputs.pg16_version }}
suite: regress
image_name: ${{ needs.params.outputs.fail_test_image_name }}
- make: check-query-generator
pg_version: ${{ needs.params.outputs.pg17_version }}
suite: regress
image_name: ${{ needs.params.outputs.fail_test_image_name }}
runs-on: ubuntu-20.04
container:
image: "${{ matrix.image_name }}:${{ fromJson(matrix.pg_version).full }}${{ needs.params.outputs.image_suffix }}"
Expand Down Expand Up @@ -255,6 +278,7 @@ jobs:
- ${{ needs.params.outputs.pg14_version }}
- ${{ needs.params.outputs.pg15_version }}
- ${{ needs.params.outputs.pg16_version }}
- ${{ needs.params.outputs.pg17_version }}
parallel: [0,1,2,3,4,5] # workaround for running 6 parallel jobs
steps:
- uses: actions/checkout@v3.5.0
Expand Down Expand Up @@ -303,6 +327,12 @@ jobs:
new_pg_major: 16
- old_pg_major: 14
new_pg_major: 16
- old_pg_major: 16
new_pg_major: 17
- old_pg_major: 15
new_pg_major: 17
- old_pg_major: 14
new_pg_major: 17
env:
old_pg_major: ${{ matrix.old_pg_major }}
new_pg_major: ${{ matrix.new_pg_major }}
Expand Down Expand Up @@ -386,7 +416,7 @@ jobs:
CC_TEST_REPORTER_ID: ${{ secrets.CC_TEST_REPORTER_ID }}
runs-on: ubuntu-20.04
container:
image: ${{ needs.params.outputs.test_image_name }}:${{ fromJson(needs.params.outputs.pg16_version).full }}${{ needs.params.outputs.image_suffix }}
image: ${{ needs.params.outputs.test_image_name }}:${{ fromJson(needs.params.outputs.pg17_version).full }}${{ needs.params.outputs.image_suffix }}
needs:
- params
- test-citus
Expand Down Expand Up @@ -481,7 +511,7 @@ jobs:
name: Test flakyness
runs-on: ubuntu-20.04
container:
image: ${{ needs.params.outputs.fail_test_image_name }}:${{ needs.params.outputs.pg16_version }}${{ needs.params.outputs.image_suffix }}
image: ${{ needs.params.outputs.fail_test_image_name }}:${{ needs.params.outputs.pg17_version }}${{ needs.params.outputs.image_suffix }}
options: --user root
env:
runs: 8
Expand Down
2 changes: 1 addition & 1 deletion configure
Original file line number Diff line number Diff line change
Expand Up @@ -2588,7 +2588,7 @@ fi
if test "$with_pg_version_check" = no; then
{ $as_echo "$as_me:${as_lineno-$LINENO}: building against PostgreSQL $version_num (skipped compatibility check)" >&5
$as_echo "$as_me: building against PostgreSQL $version_num (skipped compatibility check)" >&6;}
elif test "$version_num" != '14' -a "$version_num" != '15' -a "$version_num" != '16'; then
elif test "$version_num" != '14' -a "$version_num" != '15' -a "$version_num" != '16' -a "$version_num" != '17'; then
as_fn_error $? "Citus is not compatible with the detected PostgreSQL version ${version_num}." "$LINENO" 5
else
{ $as_echo "$as_me:${as_lineno-$LINENO}: building against PostgreSQL $version_num" >&5
Expand Down
2 changes: 1 addition & 1 deletion configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ AC_SUBST(with_pg_version_check)

if test "$with_pg_version_check" = no; then
AC_MSG_NOTICE([building against PostgreSQL $version_num (skipped compatibility check)])
elif test "$version_num" != '14' -a "$version_num" != '15' -a "$version_num" != '16'; then
elif test "$version_num" != '14' -a "$version_num" != '15' -a "$version_num" != '16' -a "$version_num" != '17'; then
AC_MSG_ERROR([Citus is not compatible with the detected PostgreSQL version ${version_num}.])
else
AC_MSG_NOTICE([building against PostgreSQL $version_num])
Expand Down
1 change: 1 addition & 0 deletions src/backend/distributed/sql/citus--12.1-1--13.0-1.sql
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
-- citus--12.1-1--13.0-1.sql

-- bump version to 13.0-1
#include "udfs/citus_prepare_pg_upgrade/13.0-1.sql"
100 changes: 100 additions & 0 deletions src/backend/distributed/sql/udfs/citus_prepare_pg_upgrade/13.0-1.sql

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,15 @@ BEGIN
DROP AGGREGATE IF EXISTS array_cat_agg(anyarray);
DROP AGGREGATE IF EXISTS array_cat_agg(anycompatiblearray);

-- We should drop any_value because PG16 has its own any_value function
-- We should drop any_value because PG16+ has its own any_value function
-- We can remove this part when we drop support for PG16
DELETE FROM pg_depend WHERE
objid IN (SELECT oid FROM pg_proc WHERE proname = 'any_value' OR proname = 'any_value_agg') AND
refobjid IN (select oid from pg_extension where extname = 'citus');
DROP AGGREGATE IF EXISTS pg_catalog.any_value(anyelement);
DROP FUNCTION IF EXISTS pg_catalog.any_value_agg(anyelement, anyelement);
IF substring(current_Setting('server_version'), '\d+')::int < 16 THEN
DELETE FROM pg_depend WHERE
objid IN (SELECT oid FROM pg_proc WHERE proname = 'any_value' OR proname = 'any_value_agg') AND
refobjid IN (select oid from pg_extension where extname = 'citus');
DROP AGGREGATE IF EXISTS pg_catalog.any_value(anyelement);
DROP FUNCTION IF EXISTS pg_catalog.any_value_agg(anyelement, anyelement);
END IF;

--
-- Drop existing backup tables
Expand Down
1 change: 1 addition & 0 deletions src/test/regress/citus_tests/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ def get_pg_major_version():
14: "10.2.0",
15: "11.1.5",
16: "12.1.5",
17: "13.0.0",
}

OLDEST_SUPPORTED_CITUS_VERSION = OLDEST_SUPPORTED_CITUS_VERSION_MATRIX[PG_MAJOR_VERSION]
Expand Down

0 comments on commit 6fed917

Please sign in to comment.