From 2cb22c45f2db20cab1723e60218c976dc51e0d45 Mon Sep 17 00:00:00 2001 From: Aleksey Smaga Date: Tue, 5 May 2026 19:00:11 +0200 Subject: [PATCH] feat(checkers): add missing_replica_identity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Flags tables whose effective REPLICA IDENTITY is empty — UPDATE/DELETE will not replicate via logical decoding. Two cases match: - relreplident = 'd' (default) but no PRIMARY KEY exists, or - relreplident = 'n' (explicit NOTHING). Strictly narrower than missing_primary_key: tables with explicit REPLICA IDENTITY USING INDEX or FULL are not flagged here, even if they lack a PK. Skips temp / unlogged tables (never replicated) and partition children (identity is inherited from the parent). Severity warning, matching missing_primary_key. The rule doc walks through fixes (ADD PRIMARY KEY, USING INDEX, FULL) and notes when to disable for teams not using logical replication. Updates README.md / docs/rules/README.md / docs/severity.md to list the new rule. Drops it from the ROADMAP medium-term list. --- README.md | 1 + ROADMAP.md | 1 - docs/rules/README.md | 1 + docs/rules/missing_replica_identity.md | 105 ++++++++++++++++++ docs/severity.md | 1 + src/pgsleuth/checkers/__init__.py | 1 + .../checkers/missing_replica_identity.py | 89 +++++++++++++++ .../checkers/test_missing_replica_identity.py | 76 +++++++++++++ 8 files changed, 274 insertions(+), 1 deletion(-) create mode 100644 docs/rules/missing_replica_identity.md create mode 100644 src/pgsleuth/checkers/missing_replica_identity.py create mode 100644 tests/checkers/test_missing_replica_identity.py diff --git a/README.md b/README.md index 593e892..b05d4b6 100644 --- a/README.md +++ b/README.md @@ -69,6 +69,7 @@ Exit codes: `0` = clean, `1` = issues found, `2` = error. | Name | Catches | | --- | --- | | [`missing_primary_key`](docs/rules/missing_primary_key.md) | Ordinary tables without a PK (breaks logical replication, ambiguous rows). | +| [`missing_replica_identity`](docs/rules/missing_replica_identity.md) | Tables whose effective `REPLICA IDENTITY` is empty — UPDATE/DELETE won't replicate logically. | | [`primary_key_type`](docs/rules/primary_key_type.md) | Primary keys whose type can run out of values (e.g. `int4` on a hot table). | | [`column_value_at_risk`](docs/rules/column_value_at_risk.md) | Sequence-backed columns whose sequence is past 70% of its type's max value. | | [`missing_fk_index`](docs/rules/missing_fk_index.md) | Foreign-key columns without a covering index — slow joins, slow `ON DELETE`. | diff --git a/ROADMAP.md b/ROADMAP.md index c62914b..ccb0f47 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -78,7 +78,6 @@ When a checker hits `statement_timeout` or is version-gated, the skip goes to st Get to ~30 rules so the tool is taken seriously next to schemacrawler. High-leverage adds, in rough order: -- **`missing_replica_identity`** — tables with no `REPLICA IDENTITY` (extends `missing_primary_key` to cover tables with explicit replica identity but no PK). - **`partition_without_pk`** — partitioned tables where the partition key isn't part of the PK. - **`default_now_text`** — `default 'now()'` (string literal!) instead of `default now()`. diff --git a/docs/rules/README.md b/docs/rules/README.md index 8b9cda9..936a180 100644 --- a/docs/rules/README.md +++ b/docs/rules/README.md @@ -10,6 +10,7 @@ Each rule has a dedicated page covering what it catches, why it matters, how to | [`json_over_jsonb`](json_over_jsonb.md) | info | Columns typed `json`; `jsonb` supports indexing and is faster on read. | | [`missing_fk_index`](missing_fk_index.md) | warning | Foreign-key columns not covered by a leading index — slow cascades and joins. | | [`missing_primary_key`](missing_primary_key.md) | warning | Ordinary tables without a primary key. | +| [`missing_replica_identity`](missing_replica_identity.md) | warning | Tables whose effective `REPLICA IDENTITY` is empty — logical UPDATE/DELETE replication breaks. | | [`not_valid_constraints`](not_valid_constraints.md) | error | FK and CHECK constraints stuck at `NOT VALID`. | | [`primary_key_type`](primary_key_type.md) | warning | Primary keys typed as `integer` / `smallint` will eventually overflow. | | [`redundant_index`](redundant_index.md) | info | An index whose column list is a strict prefix of another on the same table. | diff --git a/docs/rules/missing_replica_identity.md b/docs/rules/missing_replica_identity.md new file mode 100644 index 0000000..f86b044 --- /dev/null +++ b/docs/rules/missing_replica_identity.md @@ -0,0 +1,105 @@ +# missing_replica_identity + +> **Severity:** warning +> Tables whose `REPLICA IDENTITY` is effectively empty — `UPDATE` and `DELETE` will not replicate via logical decoding. + +## What it catches + +Postgres needs a way to identify a row when an `UPDATE` or `DELETE` is replicated logically. `pg_class.relreplident` carries one of four values per table: + +| Value | Meaning | Replicates UPDATE/DELETE? | +| --- | --- | --- | +| `'d'` (default) | Uses the table's `PRIMARY KEY`. | Only if a PK exists. | +| `'i'` (index) | Uses a specific `UNIQUE NOT NULL` non-deferrable index. | Yes. | +| `'f'` (full) | Logs the entire pre-image. | Yes (expensive). | +| `'n'` (nothing) | No identity recorded. | **No.** | + +A table fails this check when its **effective** replica identity is empty: + +```sql +-- ❌ flagged: default identity, but no primary key +CREATE TABLE events (user_id bigint, payload jsonb); + +-- ❌ flagged: explicit "nothing" +CREATE TABLE audit_log (...); +ALTER TABLE audit_log REPLICA IDENTITY NOTHING; + +-- ✅ not flagged: index-based identity covers it +CREATE TABLE events (event_uuid uuid NOT NULL, payload jsonb); +CREATE UNIQUE INDEX events_uuid_idx ON events (event_uuid); +ALTER TABLE events REPLICA IDENTITY USING INDEX events_uuid_idx; + +-- ✅ not flagged: FULL works (pricey, but works) +ALTER TABLE events REPLICA IDENTITY FULL; +``` + +Excluded by design: + +- **Partitions** (`relispartition`) — replica identity is set on the partitioned root and inherited. +- **Temp and unlogged tables** (`relpersistence != 'p'`) — never replicated, so the column doesn't matter. + +## Why it matters + +The first time a team adopts logical replication — a CDC pipeline (Debezium, pglogical), a logical replication slot for blue/green migration, a downstream warehouse using `pg_logical_slot_get_changes` — every table flagged by this rule becomes a silent landmine. + +Two failure shapes you actually see: + +```text +-- on the publisher +ERROR: cannot update table "events" because it does not have a replica identity + and publishes updates +HINT: To enable updating the table, set REPLICA IDENTITY using ALTER TABLE. +``` + +Or, more insidiously, on the consumer side: the row event arrives with `before = null`, the consumer can't identify which downstream row to update, and either errors out or silently drops the event. You discover the divergence weeks later. + +This is **warning severity** because the cost only materializes when logical replication enters the picture. Teams on physical replication only can downgrade — see *When to ignore*. + +## How to fix + +The right answer depends on why the table has no identity: + +### Add a primary key (preferred) + +```sql +ALTER TABLE events ADD COLUMN id bigserial PRIMARY KEY; +-- or, when an existing column is already unique + non-null +ALTER TABLE events ADD PRIMARY KEY (event_uuid); +``` + +A PK fixes this rule *and* `missing_primary_key`, *and* unlocks `pg_repack` / online schema-change tooling. Almost always the right move. + +### Or point at an existing unique index + +```sql +ALTER TABLE events REPLICA IDENTITY USING INDEX events_unique_idx; +``` + +Requires a `UNIQUE` index on `NOT NULL` columns, non-deferrable, non-partial. Useful when adding a PK is genuinely not desirable (e.g., the existing unique key is composite and already serves the role). + +### Or use FULL when no useful key exists + +```sql +ALTER TABLE events REPLICA IDENTITY FULL; +``` + +Logs the full row image with every change. Works on any table, but bloats WAL by 2–10× depending on row width — only choose this when nothing else fits. + +## When to ignore + +- **Append-only log tables** never updated or deleted, and never consumed by logical replication. The replica-identity gap is theoretical. +- **No logical replication anywhere on the roadmap.** Some teams run on physical replication only and never plan to add CDC. Reasonable to disable or downgrade: + + ```toml + [pgsleuth.checkers.missing_replica_identity] + enabled = false + # or: + severity = "info" + ``` + +When you *do* eventually adopt logical rep, re-enable and run prune to catch what slipped in. + +## See also + +- [PostgreSQL — `ALTER TABLE … REPLICA IDENTITY`](https://www.postgresql.org/docs/current/sql-altertable.html#SQL-ALTERTABLE-REPLICA-IDENTITY). +- Related rule: [`missing_primary_key`](missing_primary_key.md) — broader scope; this rule is the strictly-correct subset for the replica-identity concern. diff --git a/docs/severity.md b/docs/severity.md index 96b48b0..eaa97be 100644 --- a/docs/severity.md +++ b/docs/severity.md @@ -35,6 +35,7 @@ By default, `pgsleuth check` runs at `--min-severity info`, which means warnings | Rule | Why it's a warning | | --- | --- | | `missing_primary_key` | Logical replication breaks, ORM identity becomes ambiguous, common tooling refuses to operate. | +| `missing_replica_identity` | Strictly-correct subset of the replica-identity concern — fires when the table's effective REPLICA IDENTITY is empty (default + no PK, or explicit NOTHING) so logical UPDATE/DELETE replication will silently lose row-identity. | | `primary_key_type` | `integer` PK overflow is months or years away on most tables; the migration to fix it is the painful part. | | `column_value_at_risk` | Sequence-backed columns past 70% of `max_value` will overflow soon. Reactive companion to `primary_key_type` — fires before the failure but after the runway is short. | | `missing_fk_index` | Slow cascades and slow joins. Often invisible until a parent-side `DELETE` migration or a 10× traffic spike. | diff --git a/src/pgsleuth/checkers/__init__.py b/src/pgsleuth/checkers/__init__.py index c546718..8ca640b 100644 --- a/src/pgsleuth/checkers/__init__.py +++ b/src/pgsleuth/checkers/__init__.py @@ -8,6 +8,7 @@ json_over_jsonb, missing_fk_index, missing_primary_key, + missing_replica_identity, not_valid_constraints, primary_key_type, redundant_index, diff --git a/src/pgsleuth/checkers/missing_replica_identity.py b/src/pgsleuth/checkers/missing_replica_identity.py new file mode 100644 index 0000000..ca0e3a5 --- /dev/null +++ b/src/pgsleuth/checkers/missing_replica_identity.py @@ -0,0 +1,89 @@ +"""Tables whose REPLICA IDENTITY would silently break logical replication. + +Postgres needs a way to identify a row in WAL when an UPDATE or DELETE is +replicated logically. The four `pg_class.relreplident` values: + + 'd' default → uses the table's PRIMARY KEY + 'i' index → uses a specific UNIQUE NOT-NULL non-deferrable index + 'f' full → the entire pre-image is logged (works, but expensive) + 'n' nothing → no identity recorded (UPDATE/DELETE cannot replicate) + +A table fails the check when its effective replica identity is empty: + + - relreplident = 'd' AND no PRIMARY KEY exists, or + - relreplident = 'n' (explicitly disabled) + +Compared with `missing_primary_key` this rule is strictly narrower: a table +with `REPLICA IDENTITY USING INDEX` or `REPLICA IDENTITY FULL` set explicitly +has a working replica identity even without a PK and is *not* flagged here. +""" + +from __future__ import annotations + +from typing import ClassVar + +from pgsleuth.checkers.base import Issue, RowChecker, Severity, register +from pgsleuth.context import CheckerContext + +_SQL = """ +SELECT + n.nspname AS schema, + t.relname AS table, + t.relreplident AS replident +FROM pg_class t +JOIN pg_namespace n ON n.oid = t.relnamespace +WHERE t.relkind IN ('r', 'p') + AND t.relpersistence = 'p' -- skip temp + unlogged: not replicated + AND NOT t.relispartition -- partitions inherit identity from the parent + AND ( + (t.relreplident = 'd' AND NOT EXISTS ( + SELECT 1 FROM pg_index i + WHERE i.indrelid = t.oid AND i.indisprimary + )) + OR t.relreplident = 'n' + ) + {schema_filter} +ORDER BY n.nspname, t.relname; +""" + + +class MissingReplicaIdentity(RowChecker): + name: ClassVar[str] = "missing_replica_identity" + description: ClassVar[str] = ( + "Tables whose REPLICA IDENTITY is empty — UPDATE/DELETE will not replicate logically." + ) + default_severity: ClassVar[Severity] = Severity.WARNING + # PG 10's pg_class.relispartition is the earliest catalog column we touch. + min_version: ClassVar[int] = 100000 + sql: ClassVar[str] = _SQL + + def check_row(self, ctx: CheckerContext, row: dict) -> Issue | None: + obj = f"{row['schema']}.{row['table']}" + if row["replident"] == "n": + message = ( + f"Table {obj} has REPLICA IDENTITY NOTHING — " + f"UPDATE/DELETE will not replicate via logical decoding." + ) + suggestion = ( + f"ALTER TABLE {obj} REPLICA IDENTITY DEFAULT; " + f"-- or USING INDEX , or FULL" + ) + else: + message = ( + f"Table {obj} has REPLICA IDENTITY DEFAULT but no primary key — " + f"UPDATE/DELETE will not replicate via logical decoding." + ) + suggestion = ( + f"ALTER TABLE {obj} ADD PRIMARY KEY (...); " + f"-- or REPLICA IDENTITY USING INDEX , or FULL" + ) + return self.issue( + ctx, + object_type="table", + object_name=obj, + message=message, + suggestion=suggestion, + ) + + +register(MissingReplicaIdentity) diff --git a/tests/checkers/test_missing_replica_identity.py b/tests/checkers/test_missing_replica_identity.py new file mode 100644 index 0000000..4fc0cca --- /dev/null +++ b/tests/checkers/test_missing_replica_identity.py @@ -0,0 +1,76 @@ +from pgsleuth.checkers.missing_replica_identity import MissingReplicaIdentity + + +def test_clean_when_table_has_pk(ctx, conn, schema): + """Default replica identity + PK → working identity, not flagged.""" + with conn.cursor() as cur: + cur.execute("CREATE TABLE t (id bigserial PRIMARY KEY, v int)") + + issues = [i for i in MissingReplicaIdentity().run(ctx) if i.object_name.startswith(schema)] + assert issues == [] + + +def test_flags_table_with_default_identity_and_no_pk(ctx, conn, schema): + """Default identity but no PK → effective identity is empty, flagged.""" + with conn.cursor() as cur: + cur.execute("CREATE TABLE t (a int, b int)") + + issues = [i for i in MissingReplicaIdentity().run(ctx) if i.object_name.startswith(schema)] + assert len(issues) == 1 + assert issues[0].object_name.endswith(".t") + assert "no primary key" in issues[0].message + assert "ADD PRIMARY KEY" in issues[0].suggestion + + +def test_clean_with_replica_identity_full(ctx, conn, schema): + """FULL works without a PK — not flagged.""" + with conn.cursor() as cur: + cur.execute("CREATE TABLE t (a int, b int)") + cur.execute("ALTER TABLE t REPLICA IDENTITY FULL") + + issues = [i for i in MissingReplicaIdentity().run(ctx) if i.object_name.startswith(schema)] + assert issues == [] + + +def test_clean_with_replica_identity_using_index(ctx, conn, schema): + """USING INDEX on a unique non-null index works without a PK — not flagged.""" + with conn.cursor() as cur: + cur.execute("CREATE TABLE t (k int NOT NULL, v int)") + cur.execute("CREATE UNIQUE INDEX t_k_idx ON t (k)") + cur.execute("ALTER TABLE t REPLICA IDENTITY USING INDEX t_k_idx") + + issues = [i for i in MissingReplicaIdentity().run(ctx) if i.object_name.startswith(schema)] + assert issues == [] + + +def test_flags_explicit_replica_identity_nothing(ctx, conn, schema): + """Even with a PK, explicit NOTHING disables logical-rep identity — flagged.""" + with conn.cursor() as cur: + cur.execute("CREATE TABLE t (id bigserial PRIMARY KEY, v int)") + cur.execute("ALTER TABLE t REPLICA IDENTITY NOTHING") + + issues = [i for i in MissingReplicaIdentity().run(ctx) if i.object_name.startswith(schema)] + assert len(issues) == 1 + assert "NOTHING" in issues[0].message + assert "REPLICA IDENTITY DEFAULT" in issues[0].suggestion + + +def test_clean_for_unlogged_table_without_pk(ctx, conn, schema): + """Unlogged tables aren't replicated — not flagged.""" + with conn.cursor() as cur: + cur.execute("CREATE UNLOGGED TABLE t (a int, b int)") + + issues = [i for i in MissingReplicaIdentity().run(ctx) if i.object_name.startswith(schema)] + assert issues == [] + + +def test_does_not_double_flag_partitions(ctx, conn, schema): + """A partitioned table without PK is flagged once at the parent — not per partition.""" + with conn.cursor() as cur: + cur.execute("CREATE TABLE t (a int, b int) PARTITION BY RANGE (a)") + cur.execute("CREATE TABLE t_p1 PARTITION OF t FOR VALUES FROM (0) TO (100)") + cur.execute("CREATE TABLE t_p2 PARTITION OF t FOR VALUES FROM (100) TO (200)") + + issues = [i for i in MissingReplicaIdentity().run(ctx) if i.object_name.startswith(schema)] + assert len(issues) == 1 + assert issues[0].object_name.endswith(".t")