feat(findings): replica_identity_missing — a published table that can't be updated

CHANGELOG.md

@@ -3,7 +3,7 @@
 All notable changes to pgbot are documented here. The format follows
 [Keep a Changelog](https://keepachangelog.com/), and the project aims for
 [Semantic Versioning](https://semver.org/). The `--json` contract is versioned
-separately by `model.SchemaVersion` (currently 1.3.0).
+separately by `model.SchemaVersion` (currently 1.5.0).
 
 ## [Unreleased]
 
@@ -21,6 +21,17 @@ separately by `model.SchemaVersion` (currently 1.3.0).
   `pgbot ask "why is it slow?"`.
 
 ### Added
+- **`replica_identity_missing` finding.** A table published for `UPDATE` or
+  `DELETE` with no usable replica identity — `DEFAULT` and no primary key,
+  `NOTHING`, or `USING INDEX` whose index is gone — makes Postgres reject the
+  write itself (`cannot update table … because it does not have a replica
+  identity and publishes updates`). Reads and INSERTs keep working, so the
+  failure lands on the first UPDATE after a migration rather than at deploy
+  time. Read from `pg_publication` and `pg_class`, so it is `schema` scope and
+  runs under `--profile=schema` / `pgbot lint` on an empty CI database.
+  Insert-only publications need no identity and are not reported. New
+  `replica_identity` section in `--json`; `SchemaVersion` → **1.5.0** (additive;
+  a 1.4.0 consumer parses it unchanged).
 - **`collation_version_mismatch` finding** (PG15+). The collation library
   (libc or ICU) that defines text sort order changed version under the data —
   an OS upgrade, a new base image, a restore onto a different host — so every

README.md

@@ -33,7 +33,7 @@
   <a href="docs/providers.md">Provider notes</a>
 </p>
 
-> **Status: beta.** The `--json` contract is versioned (currently `1.3.0`, JSON
+> **Status: beta.** The `--json` contract is versioned (currently `1.5.0`, JSON
 > Schema published in [`schema/`](schema/)) and breaking changes to it are
 > treated as breaking changes to the tool. The human-readable report is **not**
 > a stable interface — parse `--json`, not the terminal output.
@@ -821,21 +821,21 @@ All from SQL — connections, cache-hit ratio, TPS and rollback ratio, WAL and I
 rates, checkpoints, locks and blocking chains, replication lag, replication-slot
 WAL retention and logical-subscription health, top queries
 (`pg_stat_statements`), table/index sizes, dead tuples and vacuum activity,
-unused and missing indexes, non-default settings, and collation version drift
-(PG15+). Counters
+unused and missing indexes, non-default settings, collation version drift
+(PG15+), and published tables with no replica identity. Counters
 (`pg_stat_database`, `pg_stat_wal`, IO) are **double-sampled** to produce live
 rates; the rest are point-in-time reads trended against the baseline.
 
 ## The `--json` contract
 
 `--json` (and `--format=json`) is the interface to build on — a versioned,
-PII-free document (`schema_version`, currently `1.3.0`) whose machine-checkable
+PII-free document (`schema_version`, currently `1.5.0`) whose machine-checkable
 JSON Schema is published in [`schema/`](schema/). Every section carries an
 `exactness` label — `sampled`, `cumulative`, `scraped`, or `unavailable` — so a
 consumer never mistakes a cumulative total for a live rate.
 
 Versioning policy: additive fields bump the minor version and are not breaking —
-a `1.2.0` consumer parses `1.3.0` output unchanged; breaking changes to the
+a `1.4.0` consumer parses `1.5.0` output unchanged; breaking changes to the
 contract are treated as breaking changes to the tool. `pgbot advise --json` has
 its own schema
 ([`schema/pgbot-advise-1.0.0.json`](schema/pgbot-advise-1.0.0.json)).

docs/findings/README.md

@@ -21,6 +21,7 @@ Lost durability, corruption, wraparound, replication — things that end in an o
 - **[full_page_writes_off](full_page_writes_off.md)** · Critical — full_page_writes off — a crash can leave torn pages
 - **[ignore_checksum_failure_on](ignore_checksum_failure_on.md)** · Critical — ignore_checksum_failure is on — corrupt pages are returned, not caught
 - **[index_invalid](index_invalid.md)** · Critical — a failed CREATE INDEX CONCURRENTLY left an invalid index — critical if it's still maintained on writes, warn if it's failed-build debris
+- **[replica_identity_missing](replica_identity_missing.md)** · Critical — a published table has no replica identity, so UPDATE and DELETE on it fail
 - **[sync_rep_degraded](sync_rep_degraded.md)** · Critical — fewer synchronous standbys connected than the config requires
 - **[archiving_disabled](archiving_disabled.md)** · Warn — archive_mode is off — no continuous WAL archive for PITR
 - **[collation_version_mismatch](collation_version_mismatch.md)** · Warn — the collation library changed version under the data — text indexes may be silently out of order

docs/findings/replica_identity_missing.md

@@ -0,0 +1,134 @@
+---
+id: replica_identity_missing
+severity: critical
+critical_when: ""
+dimension: risk
+object: relation
+scope: schema
+requires: [a publication replicating UPDATE or DELETE]
+thresholds: []
+related: [subscription_worker_down, replication_slot_inactive]
+---
+
+# replica_identity_missing
+
+**Severity:** critical · **Dimension:** risk · **Object identity:** `schema.table` (see [configuration](../configuration.md)) · **Requires:** a publication that replicates `UPDATE` or `DELETE`
+
+## What pgbot observed
+
+A table belongs to a publication that replicates `UPDATE` or `DELETE`, but its
+replica identity cannot identify a row:
+
+- `DEFAULT` with no primary key — the default resolves to the primary key, and
+  there isn't one.
+- `NOTHING` — set explicitly.
+- `USING INDEX` whose nominated index is missing or invalid, which behaves like
+  `NOTHING`.
+
+Publications that replicate only `INSERT` need no identity and are not reported.
+Everything here comes from `pg_publication` and `pg_class`, so it is valid on a
+freshly migrated, never-queried database.
+
+## Why it matters
+
+Postgres accepts the table, the publication and the schema, then refuses the
+write at runtime:
+
+```
+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.
+```
+
+`SELECT` and `INSERT` keep working, so nothing fails at deploy time. The failure
+arrives with the first `UPDATE` or `DELETE` after the migration, in whatever code
+path happens to run it, and it is a hard error for that statement rather than a
+replication lag or a warning.
+
+## How to verify it yourself
+
+```sql
+SELECT n.nspname AS schema,
+       c.relname AS "table",
+       c.relreplident AS identity,
+       string_agg(DISTINCT p.pubname, ', ') AS publications
+FROM pg_publication p
+JOIN pg_publication_tables pt ON pt.pubname = p.pubname
+JOIN pg_namespace n ON n.nspname = pt.schemaname
+JOIN pg_class c ON c.relnamespace = n.oid AND c.relname = pt.tablename
+WHERE (p.pubupdate OR p.pubdelete)
+  AND c.relkind IN ('r', 'p')
+  AND (
+    (c.relreplident = 'd' AND NOT EXISTS (
+       SELECT 1 FROM pg_index i WHERE i.indrelid = c.oid AND i.indisprimary AND i.indisvalid))
+    OR c.relreplident = 'n'
+    OR (c.relreplident = 'i' AND NOT EXISTS (
+       SELECT 1 FROM pg_index i WHERE i.indrelid = c.oid AND i.indisreplident AND i.indisvalid))
+  )
+GROUP BY 1, 2, 3
+ORDER BY 1, 2;
+```
+
+## How to fix it
+
+Pick the cheapest identity the table can support.
+
+1. **A primary key**, if the table can have one. This is the normal answer and
+   needs no further configuration:
+
+   ```sql
+   ALTER TABLE public.events ADD PRIMARY KEY (id);
+   ```
+
+2. **An existing unique index** on `NOT NULL` columns, when a primary key is not
+   an option:
+
+   ```sql
+   ALTER TABLE public.events REPLICA IDENTITY USING INDEX events_uniq_idx;
+   ```
+
+3. **`FULL`**, when neither fits. Correct, but it writes every column into the WAL
+   record and makes the subscriber match rows without an index:
+
+   ```sql
+   ALTER TABLE public.events REPLICA IDENTITY FULL;
+   ```
+
+4. **Remove the table from the publication**, if it was never meant to replicate:
+
+   ```sql
+   ALTER PUBLICATION app_pub DROP TABLE public.events;
+   ```
+
+## When to ignore it
+
+When the table is genuinely insert-only and you are certain no `UPDATE` or
+`DELETE` will ever run against it. That is a claim about application behaviour
+that the catalog cannot confirm, and one stray `UPDATE` turns into an error, so
+scope the suppression to the table and give it an expiry:
+
+```toml
+[[ignore]]
+finding = "replica_identity_missing"
+object  = "public.events"
+reason  = "append-only event log; no UPDATE/DELETE in the writer (OPS-2291)"
+expires = "2027-01-01"
+```
+
+## What pgbot cannot see
+
+- Whether anything actually issues an `UPDATE` or `DELETE` against the table. The
+  finding reports that the write *would* fail, not that it has.
+- The subscriber side. A subscription may also need its own matching index for
+  `FULL` identity to perform acceptably.
+- Row filters and column lists on the publication, which narrow what replicates
+  but do not remove the identity requirement.
+- A partitioned table published with `publish_via_partition_root` replicates as
+  the root; pgbot names the relation the catalog lists, which may be a partition.
+
+## Related
+
+- [subscription_worker_down](subscription_worker_down.md) — the subscriber-side
+  symptom when replication stops.
+- [replication_slot_inactive](replication_slot_inactive.md) — a stalled logical
+  slot retains WAL while the publisher cannot make progress.

internal/collect/collector.go

@@ -40,6 +40,7 @@ var schemaCollectors = map[string]bool{
 	"indexes":   true, // index_invalid, redundant_indexes, fk_unindexed
 	"sequences": true, // int4_identity_column
 	"tables":    true, // autovacuum_disabled_on_table (reloptions)
+	"replident": true, // replica_identity_missing (pg_publication + pg_class)
 }
 
 func (o Options) interval() time.Duration {
@@ -106,6 +107,7 @@ var registry = []Collector{
 	checksumsCollector{},
 	collationCollector{},
 	standbyCollector{},
+	replidentCollector{},
 }
 
 func nowUTC() time.Time { return time.Now().UTC() }

internal/collect/replident.go

@@ -0,0 +1,47 @@
+package collect
+
+import (
+	"context"
+	_ "embed"
+	"time"
+
+	"github.com/pgrundev/pgbot/internal/conn"
+	"github.com/pgrundev/pgbot/internal/model"
+)
+
+//go:embed sql/replident.sql
+var sqlReplident string
+
+// replident = published tables whose replica identity can't identify a row, so
+// UPDATE/DELETE on them errors. Catalog-only, so it works on an empty database.
+type replidentCollector struct{}
+
+type replidentRow struct {
+	Schema       string `db:"schema"`
+	Table        string `db:"table"`
+	Identity     string `db:"identity"`
+	Publications string `db:"publications"`
+}
+
+func (replidentCollector) Name() string                     { return "replident" }
+func (replidentCollector) Kind() Kind                       { return KindGauge }
+func (replidentCollector) Available(conn.Capabilities) bool { return true }
+
+func (replidentCollector) Sample(ctx context.Context, t *conn.Target, _ conn.Capabilities) (any, error) {
+	return queryMany[replidentRow](ctx, t, sqlReplident)
+}
+
+func (replidentCollector) Assemble(c *model.Context, _ conn.Capabilities, s sampled, _ time.Duration, _ Options) {
+	rows, ok := s.A.([]replidentRow)
+	if s.Err != nil || !ok {
+		c.ReplicaIdentity = &model.ReplicaIdentity{Section: unavail(s.Err, "publication catalog unreadable")}
+		return
+	}
+	ri := &model.ReplicaIdentity{Section: model.Section{Exactness: model.ExactnessScraped}}
+	for _, r := range rows {
+		ri.Unidentifiable = append(ri.Unidentifiable, model.PublishedTable{
+			Schema: r.Schema, Name: r.Table, Identity: r.Identity, Publications: r.Publications,
+		})
+	}
+	c.ReplicaIdentity = ri
+}

internal/collect/replident_integration_test.go

@@ -0,0 +1,103 @@
+package collect_test
+
+import (
+	"context"
+	"os"
+	"testing"
+	"time"
+
+	"github.com/jackc/pgx/v5"
+	"github.com/pgrundev/pgbot/internal/collect"
+	"github.com/pgrundev/pgbot/internal/conn"
+	"github.com/pgrundev/pgbot/internal/findings"
+	"github.com/pgrundev/pgbot/internal/model"
+)
+
+// A publication over a table with no primary key is accepted by Postgres and only
+// rejects the first UPDATE. Build exactly that, prove the write really fails, then
+// run the real collector as the read-only role and check the finding. Adding a
+// primary key must clear it.
+func TestIntegration_replicaIdentityMissing(t *testing.T) {
+	su := os.Getenv("PGBOT_TEST_SUPERUSER_DSN")
+	if su == "" {
+		t.Skip("set PGBOT_TEST_SUPERUSER_DSN (a superuser DSN) to run the publication fixture")
+	}
+	ro := dsn(t)
+	ctx, cancel := context.WithTimeout(context.Background(), 60*time.Second)
+	defer cancel()
+	admin, err := pgx.Connect(ctx, su)
+	if err != nil {
+		t.Fatalf("admin connect: %v", err)
+	}
+	t.Cleanup(func() { admin.Close(context.Background()) })
+
+	cleanup := func() {
+		_, _ = admin.Exec(context.Background(), `DROP PUBLICATION IF EXISTS pgbot_it_pub`)
+		_, _ = admin.Exec(context.Background(), `DROP TABLE IF EXISTS public.pgbot_it_nopk`)
+	}
+	cleanup()
+	t.Cleanup(cleanup)
+	if _, err := admin.Exec(ctx, `
+		CREATE TABLE public.pgbot_it_nopk (id bigint, note text);
+		INSERT INTO public.pgbot_it_nopk VALUES (1, 'a');
+		CREATE PUBLICATION pgbot_it_pub FOR TABLE public.pgbot_it_nopk`); err != nil {
+		t.Fatalf("fixture: %v", err)
+	}
+	// The breakage this finding predicts, confirmed against the server.
+	if _, err := admin.Exec(ctx, `UPDATE public.pgbot_it_nopk SET note = 'b'`); err == nil {
+		t.Fatal("expected the UPDATE to fail without a replica identity")
+	}
+
+	target, err := conn.Connect(ctx, ro)
+	if err != nil {
+		t.Fatalf("connect: %v", err)
+	}
+	defer target.Close()
+	run := func() *model.Context {
+		c, err := collect.Run(ctx, target, collect.Options{Interval: 200 * time.Millisecond, ASHHz: 0})
+		if err != nil {
+			t.Fatalf("run: %v", err)
+		}
+		if c.ReplicaIdentity == nil || c.ReplicaIdentity.Exactness != model.ExactnessScraped {
+			t.Fatalf("the section must be collected by the read-only role, got %+v", c.ReplicaIdentity)
+		}
+		return c
+	}
+
+	c := run()
+	var row *model.PublishedTable
+	for i := range c.ReplicaIdentity.Unidentifiable {
+		if c.ReplicaIdentity.Unidentifiable[i].Name == "pgbot_it_nopk" {
+			row = &c.ReplicaIdentity.Unidentifiable[i]
+		}
+	}
+	if row == nil {
+		t.Fatalf("the published table must be collected, got %+v", c.ReplicaIdentity.Unidentifiable)
+	}
+	if row.Identity != "d" || row.Publications != "pgbot_it_pub" {
+		t.Errorf("collected row must mirror the catalog: %+v", *row)
+	}
+	var f *model.Finding
+	for _, x := range findings.Compute(c) {
+		if x.ID == "replica_identity_missing" {
+			f = &x
+			break
+		}
+	}
+	if f == nil || f.Severity != model.SeverityCritical {
+		t.Fatalf("expected critical replica_identity_missing, got %+v", f)
+	}
+
+	if _, err := admin.Exec(ctx, `ALTER TABLE public.pgbot_it_nopk ADD PRIMARY KEY (id)`); err != nil {
+		t.Fatalf("add pk: %v", err)
+	}
+	for _, r := range run().ReplicaIdentity.Unidentifiable {
+		if r.Name == "pgbot_it_nopk" {
+			t.Fatalf("a primary key must clear the finding, still reported: %+v", r)
+		}
+	}
+	// And the write the finding predicted now succeeds.
+	if _, err := admin.Exec(ctx, `UPDATE public.pgbot_it_nopk SET note = 'c'`); err != nil {
+		t.Errorf("UPDATE should succeed once a replica identity exists: %v", err)
+	}
+}