Remove dead pre-PG16 shims from citus_prepare_pg_upgrade / citus_finish_pg_upgrade
- Dominant language
- C
- Stars
- 12.8k
- Forks
- 794
- Avg merge
- 2d 14h
- Merged PRs (30d)
- 31
Description
Split out of #8750 to keep the PG16 drop free of any SQL migration.
`citus_prepare_pg_upgrade` and `citus_finish_pg_upgrade` still carry two paths that only ever applied to PostgreSQL versions below 16. Both are already unreachable today — Citus 14.x requires PG16+, so a cluster running a current Citus binary cannot be on PG15 or older.
## Dead code
**1. The `any_value` shim** — `citus_prepare_pg_upgrade/latest.sql` L21–29
```sql
-- 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
...
END IF;
```
The `< 16` branch has been unreachable since PG15 support was dropped. Its own comment already flags it for removal; it was simply missed in that cycle.
**2. `pg_dist_partkeys_pre_16_upgrade`** — `citus_prepare_pg_upgrade/latest.sql` L90–106 and `citus_finish_pg_upgrade/latest.sql` L113–116
Captures partition keys that predate `varnullingrels`. Any cluster on PG17+ either had its partkeys created on PG16+ (so they already carry `varnullingrels`) or regenerated them during an earlier `pg_upgrade`.
Note this is **not** a standalone deletion: `citus_prepare_pg_upgrade/latest.sql` L103–106 de-duplicates `pg_dist_partkeys_pre_18_upgrade` against the pre-16 table. Removing one without the other leaves duplicates in `pre_18`.
## Why this is separate
The repo convention is that a UDF body change ships as a versioned snapshot plus an `#include` in the migration script — so this means new `15.0-1.sql` files for both UDFs, edits to both `latest.sql`, two includes in `citus--14.0-1--15.0-1.sql`, and a downgrade-path review. That is roughly seven files in the most failure-sensitive part of the tree in exchange for no functional change.
For comparison, the PG15 drop (`62a919066`) touched no files under `sql/udfs/` at all. Keeping #8750 migration-free follows that precedent and keeps the risk profile of the drop low.
## Scope
- Remove the `< 16` `any_value` branch
- Remove `pg_dist_partkeys_pre_16_upgrade` creation, the `pre_18` de-duplication that depends on it, and the restore/drop in `citus_finish_pg_upgrade`
- Add versioned snapshots and wire them into the migration script
- Leave frozen historical copies (`12.1-1`, `13.x`, `14.0-1`) untouched
- Exercise the `pg_upgrade` regression suite, particularly the `pg_dist_partkeys_pre_18_upgrade` path
Contributor guide
Research direction
Start with citus_prepare_pg_upgrade/latest.sql and citus_finish_pg_upgrade/latest.sql, then inspect the cited 14.0-1--15.0-1.sql migration and the frozen historical copies. Trace the versioned UDF snapshot and #include convention before making the matching changes. Done means the pre-PG16 paths and dependent de-duplication are removed, migration wiring is consistent, and the pg_upgrade regression suite covers the pre_18 path.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- postgresql, sql
- Domain
- databases
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 55/100