citusdata / citusdata/citus

Remove dead pre-PG16 shims from citus_prepare_pg_upgrade / citus_finish_pg_upgrade

Open
#8,755 0 comments 0 reactions 0 assignees View on GitHub
technical debt
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.