Cleanup from dropping PG16 support - #8764
Merged
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8764 +/- ##
=======================================
Coverage 88.73% 88.74%
=======================================
Files 289 289
Lines 64992 64992
Branches 8200 8200
=======================================
+ Hits 57672 57677 +5
+ Misses 4953 4952 -1
+ Partials 2367 2363 -4 🚀 New features to boost your workflow:
|
Merged
ibrahim halatci (ihalatci)
marked this pull request as ready for review
August 22, 2026 08:15
Onur Tirtir (onurctirtir)
approved these changes
Aug 24, 2026
Should have been part of #8757 This commit deletes some redundant alternative test outputs which were created for PG16, and collapses the now-always-true `server_version_ge_17` gates. Since the minimum supported version is PG17, `server_version_ge_17` is always true, so the `\else` branches guarded by it were dead code. Also: - removes the dead `15_16` arm from the `version_category` CASE in merge_unsupported - removes an always-true `PG_MAJOR_VERSION >= 16` branch in common.py - fixes a stale `#endif` comment in ruleutils_18.c which claimed the band was `>= 17 && < 18` while the `#if` actually guards `>= 18 && < 19` Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7b92da71-e0c2-4baf-b47d-c81ec58da88b
The PG16 cleanup touched 9 test .sql files, which enrolled them in the test-flakyness job for the first time. That surfaced two latent defects that were already present on the base commit and are not regressions: merge_unsupported creates pgmerge_schema but never drops it, unlike its sibling pgmerge which the test was adapted from. Its leading DROP SCHEMA IF EXISTS therefore only reports "skipping" on the first run, so a repeat run fails. Add the missing teardown. The notice is suppressed because DROP SCHEMA ... CASCADE reports shard IDs in its DETAIL output, and the expected output is otherwise free of shard IDs so that it does not depend on the test's position in a schedule. pg17_json shares the schedule line "test: pg17 pg17_json" but had no DEPS entry, so running it standalone left pg17 without multi_behavioral_analytics_create_table and failed with relation "users_table" does not exist. Give it the same deps as pg17. Both were verified against the flakyness job's own command at --repeat 8, and both were reproduced on the unmodified base commit beforehand to confirm they predate this branch. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7b92da71-e0c2-4baf-b47d-c81ec58da88b
Now that PG17 is the minimum supported version, three alternative expected-output files only ever matched PG16 and earlier, so they are dead goldfiles: - isolation_multiuser_locking_0.out: the spec header said verbatim "isolation_multiuser_locking_0.out for PG16 and before". REINDEX TABLE requires table ownership as of PostgreSQL commit ecb0fd337, which changed the error from "must be owner of table" to "permission denied for table". - multi_multiuser_master_protocol_0.out: added by 5924162 for the MAINTAIN privilege, which exists only in PG17+ (it was added during PG16 development and reverted before PG16 shipped). - columnar_paths_0.out: added by 808626e, whose commit message states "The fix is to have an alternative goldfile for pre-PG17". PostgreSQL commit f7816aec2 improved CTE scan size estimation, flipping a merge join to a hash join. Also removes the now-dangling comments that pointed at the deleted files, including the copy echoed into columnar_paths.out. Verified by running columnar_paths, multi_multiuser_master_protocol and isolation_multiuser_locking against both PG17.10 and PG18.4. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7b92da71-e0c2-4baf-b47d-c81ec58da88b
Follow-up to dropping PG16 support. With PG17 as the minimum supported
version, three regression-test helpers that existed solely to normalize
EXPLAIN output across PG16/PG17 are now dead or pure pass-throughs:
- initplan_references_to_pg17
- explain_with_pg17_initplan_format (no-op on PG17+)
- explain_with_pg16_subplan_format (defined identically in both
multi_test_helpers.sql and columnar_test_helpers.sql)
Removes the definitions and unwraps their 41 call sites across 16 files.
All but one call site is output-neutral; the columnar one now shows the
native PG17+ SubPlan rendering instead of the down-converted PG16 form.
Also removes the multi_extension.extension_basic_types filter and the
matching type-exclusion block in upgrade_list_citus_objects. Both were
added to hide the PG17 change that makes auto-generated array,
multirange and rowtypes dependent objects (PG commit e5bc9454e). The
regenerated expected output now lists those types.
Verified against PG17.10 and PG18.4.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 7b92da71-e0c2-4baf-b47d-c81ec58da88b
Unwrapping the retired pre-PG17 EXPLAIN helpers brings four more test
files into the flakyness gate's detection set, which repeat-runs each
changed test on top of its default base schedule. All four then fail --
not because of the unwrap, but because they never declared the setup
they rely on. The failures reproduce byte-identically on the parent
commit, so they are pre-existing gaps that were simply never exercised.
ch_bench_having / having_subquery
Need public.supplier and public.nation, and share their schedule
lines with tests that query lineitem. minimal_schedule loads no
data at all, so use base_schedule.
multi_subquery
Shares its schedule line with cross_join. Neither test drops the
tables it creates, so a second pass fails with "relation ...
already exists" and a duplicate pg_dist_shard key. Mark the line
non-repeatable rather than rewriting two unrelated tests.
ch_bench_having_mx
Queries supplier_mx and nation_mx, which multi_mx_copy_data loads.
Verified with the exact command CI runs, on PG18.4:
run_test.py <test> --repeat 2 --use-whole-schedule-line
All four now pass.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 7b92da71-e0c2-4baf-b47d-c81ec58da88b
citus.enable_outer_joins_with_pseudoconstant_quals_pre_pg17 is inert now that PG16 is unsupported, but it is still accepted so existing configurations keep loading, and setting it emits a deprecation warning. Dropping PG16 support removed the three blocks in pg17.sql that used to set this GUC, which left the warning path with no coverage at all. Add a single SET/RESET pair where those blocks used to live. Only the SET produces output: RESET takes the reset_val branch in set_config_option_ext and does not re-run the check hook. Verified locally against PG17.10 and PG18.4. Relates to #8751 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7b92da71-e0c2-4baf-b47d-c81ec58da88b
PG17 is the oldest supported major and PG19 the newest, but the check-pg-upgrade matrix only covered adjacent pairs, so the two-major jump was never exercised. This needs no image work. Unlike citusupgradetester, which takes a singular PG_VERSION and is built once per version, pgupgradetester takes PG_VERSIONS and installs every major into a single image, and build_and_test.yml pins that one image for all matrix rows. The upgrade harness itself only plumbs --old-bindir/--new-bindir and has no hardcoded version pairs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7b92da71-e0c2-4baf-b47d-c81ec58da88b
ibrahim halatci (ihalatci)
force-pushed
the
ihalatci-drop-pg16-cleanup
branch
from
September 1, 2026 12:01
3ecd75b to
11abe26
Compare
Closed
ibrahim halatci (ihalatci)
added a commit
that referenced
this pull request
Sep 2, 2026
DESCRIPTION: Quote EXTRACT fields and support latest PostgreSQL minors Fixes #8803. ## Changes - quote EXTRACT field identifiers in the copied PG17 and PG18 ruleutils implementations; `ruleutils_19.c` already carries the equivalent fix from #8753; - run one discriminating, repeat-safe regression across supported PostgreSQL versions from the N-1-excluded create-Citus schedule; PG19 retains its dedicated #8753 coverage; - bump CI coverage from 17.10 / 18.4 to 17.11 / 18.6; - forward-port the already-landed release-14 fallout for the new PostgreSQL security minors, including `output_plugin_libraries` handling and modern psql COPY behavior; - add PG18 to the `test-citus-upgrade` matrix. ### PG18 citus upgrade coverage `citusupgradetester` is built and published for PG18 (`CITUS_UPGRADE_VERSIONS_18=v14.2.0`), but the matrix listed only PG17, so that image was built and never exercised. This adds it, matching the convention on `release-14.0`, which lists every supported major except the newest. PG19 stays out deliberately. The image compiles released Citus from source, and no released Citus supports PG19 yet, which is why the-process filters it out of `CITUS_UPGRADE_PG_VERSIONS`. PG19 upgrade paths are still covered by `test-pg-upgrade`, whose matrix already includes 17→18, 18→19, and 17→19. ## Cross-repo dependency The test images are built by [the-process#244](citusdata/the-process#244), stacked above [the-process#240](citusdata/the-process#240). Both have merged, along with #8757 and #8764, so this PR is last in that sequence and now pins the released `-vdba9cbb` suffix. ## Validation - `-Werror` builds: PostgreSQL 17.11, 18.6, and 19beta3 - shared `extract_deparse`: 8/8 repetitions on each of PostgreSQL 17.11, 18.6, and 19beta3 - focused PG17, PG18, and PG19 version regressions - focused `split_output_plugin_denied` regression on PG18.6 - `citus_indent --check` - SQL snapshot and test-schedule checks - `pg_regress_multi.pl` syntax check - all 13 image tags referenced by the workflow resolved against GHCR at the `-vdba9cbb` suffix before pinning ## Known limitation carried over from the release branches The `output_plugin_libraries` preflight splits the GUC on commas, so a plugin name containing a comma inside a quoted element (`"a,b"`) is not parsed the way PostgreSQL's `SplitGUCList` would parse it. This is unreachable for the name `citus` and matches what already shipped on release-14.0, release-13.2, and release-12.1. --------- Co-authored-by: Ibrahim Halatci <ihalatci@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: ihalatci <10450368+ihalatci@users.noreply.github.com> Co-authored-by: ihalatci <ihalatci@users.noreply.github.com> Copilot-Session: 7c6370b2-06fd-4491-bf92-ecb811d34518
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part 2 of 2 for #8750.
Stacked on #8757 — this PR targets that branch, not
main. Please merge #8757 first.Mirrors the equivalent PG14 cleanup commit
529b303b6that followed the PG15 drop.What this does
Now that PG17 is the minimum supported version,
server_version_ge_17is always true. This removes the resulting dead code.Deleted 6 dead alternative expected outputs (-4000 lines)
expected/pg17_0.outexpected/pg17_json_0.out\qexpected/subquery_in_where_0.outexpected/grant_on_table_propagation_0.outfforserver_version_ge_17expected/merge_unsupported_0.outversion_category=15_16expected/multi_alter_table_add_constraints_1.outge_17block produced no outputCollapsed 8
server_version_ge_17gatesmerge_unsupported,grant_on_table_propagation,multi_alter_table_add_constraints,multi_mx_create_table,multi_mx_hide_shard_names,multi_schema_support,pg15,pg17,pg17_json—.sqland.outedited in lockstep.Other
merge_unsupported: dropped the deadWHEN ... IN (15, 16) THEN '15_16'arm (the 17/18 arms stay)citus_tests/common.py: removed an always-trueif PG_MAJOR_VERSION >= 16:ruleutils_18.c: fixed a stale#endifcomment that said>= 17 && < 18while the#ifguards>= 18 && < 19Deliberately NOT changed
sql/pg16.sql/expected/pg16.out— ungated feature test, same aspg15.sqlwas kept after the PG15 dropmulti_test_helpers.sqlpg_ge_16_options— catalog introspection (attname = 'daticurules'), not a version gateexpected/multi_mx_hide_shard_names_0.outandexpected/pg18_0.out— still-live alternatives (plan shape /<18branch)Verification
ci/*.shstyle scripts pass;black/isort/flake8cleansubquery_in_where,pg15,pg17,pg17_json,merge_unsupported,grant_on_table_propagation,multi_alter_table_add_constraints,multi_schema_support,multi_mx_create_table,multi_mx_hide_shard_names