From a383ef6831298d6c0aac70aea6cefab0937b90e6 Mon Sep 17 00:00:00 2001 From: Naisila Puka <37271756+naisila@users.noreply.github.com> Date: Tue, 24 Dec 2024 17:56:51 +0300 Subject: [PATCH] Adds PG17.1 support - Regression tests sanity (#7661) 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 https://github.com/citusdata/citus/issues/7653 See the list below: Compilation PR: https://github.com/citusdata/citus/pull/7699 Ruleutils PR: https://github.com/citusdata/citus/pull/7725 Sister PR for tests: https://github.com/citusdata/the-process/pull/159 Helpful smaller PRs: - https://github.com/citusdata/citus/pull/7714 - https://github.com/citusdata/citus/pull/7726 - https://github.com/citusdata/citus/pull/7731 - https://github.com/citusdata/citus/pull/7732 - https://github.com/citusdata/citus/pull/7733 - https://github.com/citusdata/citus/pull/7738 - https://github.com/citusdata/citus/pull/7745 - https://github.com/citusdata/citus/pull/7747 - https://github.com/citusdata/citus/pull/7748 - https://github.com/citusdata/citus/pull/7749 - https://github.com/citusdata/citus/pull/7752 - https://github.com/citusdata/citus/pull/7755 - https://github.com/citusdata/citus/pull/7757 - https://github.com/citusdata/citus/pull/7759 - https://github.com/citusdata/citus/pull/7760 - https://github.com/citusdata/citus/pull/7761 - https://github.com/citusdata/citus/pull/7762 - https://github.com/citusdata/citus/pull/7765 - https://github.com/citusdata/citus/pull/7766 - https://github.com/citusdata/citus/pull/7768 - https://github.com/citusdata/citus/pull/7769 - https://github.com/citusdata/citus/pull/7771 - https://github.com/citusdata/citus/pull/7774 - https://github.com/citusdata/citus/pull/7776 - https://github.com/citusdata/citus/pull/7780 - https://github.com/citusdata/citus/pull/7781 - https://github.com/citusdata/citus/pull/7785 - https://github.com/citusdata/citus/pull/7788 - https://github.com/citusdata/citus/pull/7793 - https://github.com/citusdata/citus/pull/7796 --------- Co-authored-by: Colm --- .devcontainer/Dockerfile | 14 ++- .github/workflows/build_and_test.yml | 46 ++++++-- configure | 2 +- configure.ac | 2 +- .../distributed/sql/citus--12.1-1--13.0-1.sql | 1 + .../udfs/citus_prepare_pg_upgrade/13.0-1.sql | 100 ++++++++++++++++++ .../udfs/citus_prepare_pg_upgrade/latest.sql | 14 +-- src/test/regress/citus_tests/common.py | 1 + 8 files changed, 163 insertions(+), 17 deletions(-) create mode 100644 src/backend/distributed/sql/udfs/citus_prepare_pg_upgrade/13.0-1.sql diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile index 9c0b011f0..44424067a 100644 --- a/.devcontainer/Dockerfile +++ b/.devcontainer/Dockerfile @@ -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 @@ -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.6 +RUN pgenv switch 17.1 # make connecting to the coordinator easy ENV PGPORT=9700 diff --git a/.github/workflows/build_and_test.yml b/.github/workflows/build_and_test.yml index 594834a1c..5afa98831 100644 --- a/.github/workflows/build_and_test.yml +++ b/.github/workflows/build_and_test.yml @@ -31,12 +31,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.6" - image_suffix: "-v5779674" - pg14_version: '{ "major": "14", "full": "14.15" }' - pg15_version: '{ "major": "15", "full": "15.10" }' - pg16_version: '{ "major": "16", "full": "16.6" }' - upgrade_pg_versions: "14.15-15.10-16.6" + 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" }' + 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 @@ -113,6 +114,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 }}" @@ -144,6 +146,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 @@ -173,6 +176,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 @@ -185,6 +192,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 @@ -197,6 +208,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 }} @@ -205,6 +220,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 @@ -217,6 +236,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 }}" @@ -260,6 +283,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@v4 @@ -310,6 +334,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 }} @@ -397,7 +427,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 @@ -509,7 +539,7 @@ jobs: name: Test flakyness runs-on: ubuntu-20.04 container: - image: ${{ needs.params.outputs.fail_test_image_name }}:${{ fromJson(needs.params.outputs.pg16_version).full }}${{ needs.params.outputs.image_suffix }} + image: ${{ needs.params.outputs.fail_test_image_name }}:${{ fromJson(needs.params.outputs.pg17_version).full }}${{ needs.params.outputs.image_suffix }} options: --user root env: runs: 8 diff --git a/configure b/configure index 4bda6f37f..5240df4db 100755 --- a/configure +++ b/configure @@ -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 diff --git a/configure.ac b/configure.ac index 6ecb13760..c7fde02de 100644 --- a/configure.ac +++ b/configure.ac @@ -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]) diff --git a/src/backend/distributed/sql/citus--12.1-1--13.0-1.sql b/src/backend/distributed/sql/citus--12.1-1--13.0-1.sql index 3a342a0fe..216171664 100644 --- a/src/backend/distributed/sql/citus--12.1-1--13.0-1.sql +++ b/src/backend/distributed/sql/citus--12.1-1--13.0-1.sql @@ -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" diff --git a/src/backend/distributed/sql/udfs/citus_prepare_pg_upgrade/13.0-1.sql b/src/backend/distributed/sql/udfs/citus_prepare_pg_upgrade/13.0-1.sql new file mode 100644 index 000000000..4f07ce5c4 --- /dev/null +++ b/src/backend/distributed/sql/udfs/citus_prepare_pg_upgrade/13.0-1.sql @@ -0,0 +1,100 @@ +CREATE OR REPLACE FUNCTION pg_catalog.citus_prepare_pg_upgrade() + RETURNS void + LANGUAGE plpgsql + SET search_path = pg_catalog + AS $cppu$ +BEGIN + + DELETE FROM pg_depend WHERE + objid IN (SELECT oid FROM pg_proc WHERE proname = 'array_cat_agg') AND + refobjid IN (select oid from pg_extension where extname = 'citus'); + -- + -- We are dropping the aggregates because postgres 14 changed + -- array_cat type from anyarray to anycompatiblearray. When + -- upgrading to pg14, specifically when running pg_restore on + -- array_cat_agg we would get an error. So we drop the aggregate + -- and create the right one on citus_finish_pg_upgrade. + + 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 can remove this part when we drop support for PG16 + 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 + -- + DROP TABLE IF EXISTS public.pg_dist_partition; + DROP TABLE IF EXISTS public.pg_dist_shard; + DROP TABLE IF EXISTS public.pg_dist_placement; + DROP TABLE IF EXISTS public.pg_dist_node_metadata; + DROP TABLE IF EXISTS public.pg_dist_node; + DROP TABLE IF EXISTS public.pg_dist_local_group; + DROP TABLE IF EXISTS public.pg_dist_transaction; + DROP TABLE IF EXISTS public.pg_dist_colocation; + DROP TABLE IF EXISTS public.pg_dist_authinfo; + DROP TABLE IF EXISTS public.pg_dist_poolinfo; + DROP TABLE IF EXISTS public.pg_dist_rebalance_strategy; + DROP TABLE IF EXISTS public.pg_dist_object; + DROP TABLE IF EXISTS public.pg_dist_cleanup; + DROP TABLE IF EXISTS public.pg_dist_schema; + DROP TABLE IF EXISTS public.pg_dist_clock_logical_seq; + + -- + -- backup citus catalog tables + -- + CREATE TABLE public.pg_dist_partition AS SELECT * FROM pg_catalog.pg_dist_partition; + CREATE TABLE public.pg_dist_shard AS SELECT * FROM pg_catalog.pg_dist_shard; + CREATE TABLE public.pg_dist_placement AS SELECT * FROM pg_catalog.pg_dist_placement; + CREATE TABLE public.pg_dist_node_metadata AS SELECT * FROM pg_catalog.pg_dist_node_metadata; + CREATE TABLE public.pg_dist_node AS SELECT * FROM pg_catalog.pg_dist_node; + CREATE TABLE public.pg_dist_local_group AS SELECT * FROM pg_catalog.pg_dist_local_group; + CREATE TABLE public.pg_dist_transaction AS SELECT * FROM pg_catalog.pg_dist_transaction; + CREATE TABLE public.pg_dist_colocation AS SELECT * FROM pg_catalog.pg_dist_colocation; + CREATE TABLE public.pg_dist_cleanup AS SELECT * FROM pg_catalog.pg_dist_cleanup; + -- save names of the tenant schemas instead of their oids because the oids might change after pg upgrade + CREATE TABLE public.pg_dist_schema AS SELECT schemaid::regnamespace::text AS schemaname, colocationid FROM pg_catalog.pg_dist_schema; + -- enterprise catalog tables + CREATE TABLE public.pg_dist_authinfo AS SELECT * FROM pg_catalog.pg_dist_authinfo; + CREATE TABLE public.pg_dist_poolinfo AS SELECT * FROM pg_catalog.pg_dist_poolinfo; + -- sequences + CREATE TABLE public.pg_dist_clock_logical_seq AS SELECT last_value FROM pg_catalog.pg_dist_clock_logical_seq; + CREATE TABLE public.pg_dist_rebalance_strategy AS SELECT + name, + default_strategy, + shard_cost_function::regprocedure::text, + node_capacity_function::regprocedure::text, + shard_allowed_on_node_function::regprocedure::text, + default_threshold, + minimum_threshold, + improvement_threshold + FROM pg_catalog.pg_dist_rebalance_strategy; + + -- store upgrade stable identifiers on pg_dist_object catalog + CREATE TABLE public.pg_dist_object AS SELECT + address.type, + address.object_names, + address.object_args, + objects.distribution_argument_index, + objects.colocationid + FROM pg_catalog.pg_dist_object objects, + pg_catalog.pg_identify_object_as_address(objects.classid, objects.objid, objects.objsubid) address; + + -- if we are upgrading from PG14/PG15 to PG16+, + -- we will need to regenerate the partkeys because they will include varnullingrels as well. + -- so we save the partkeys as column names here + CREATE TABLE IF NOT EXISTS public.pg_dist_partkeys_pre_16_upgrade AS + SELECT logicalrelid, column_to_column_name(logicalrelid, partkey) as col_name + FROM pg_catalog.pg_dist_partition WHERE partkey IS NOT NULL AND partkey NOT ILIKE '%varnullingrels%'; +END; +$cppu$; + +COMMENT ON FUNCTION pg_catalog.citus_prepare_pg_upgrade() + IS 'perform tasks to copy citus settings to a location that could later be restored after pg_upgrade is done'; diff --git a/src/backend/distributed/sql/udfs/citus_prepare_pg_upgrade/latest.sql b/src/backend/distributed/sql/udfs/citus_prepare_pg_upgrade/latest.sql index b4bc653f2..4f07ce5c4 100644 --- a/src/backend/distributed/sql/udfs/citus_prepare_pg_upgrade/latest.sql +++ b/src/backend/distributed/sql/udfs/citus_prepare_pg_upgrade/latest.sql @@ -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 diff --git a/src/test/regress/citus_tests/common.py b/src/test/regress/citus_tests/common.py index 232b39067..246db36f4 100644 --- a/src/test/regress/citus_tests/common.py +++ b/src/test/regress/citus_tests/common.py @@ -93,6 +93,7 @@ OLDEST_SUPPORTED_CITUS_VERSION_MATRIX = { 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]