Cleanup from dropping PG16 support - #8764
Open
ibrahim halatci (ihalatci) wants to merge 6 commits into
Open
Conversation
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
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## ihalatci-drop-pg16-support #8764 +/- ##
==============================================================
+ Coverage 88.69% 88.70% +0.01%
==============================================================
Files 289 289
Lines 64808 64808
Branches 8178 8177 -1
==============================================================
+ Hits 57479 57487 +8
+ Misses 4964 4958 -6
+ Partials 2365 2363 -2 🚀 New features to boost your workflow:
|
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
The pg_inherits queries listing the partitions of partitioned_table and local_partitioned_table had no ORDER BY, so their row order was whatever the scan happened to return. This made the pg17 test fail intermittently under the flakyness gate, which reruns changed tests several times. The four equivalent queries for alt_test already order by partition_name, so apply the same ordering to the remaining nine. The expected output is unchanged apart from the echoed query text, because the recorded rows were already in alphabetical order. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7b92da71-e0c2-4baf-b47d-c81ec58da88b
ibrahim halatci (ihalatci)
marked this pull request as ready for review
August 22, 2026 08:15
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
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
2 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