diff --git a/README.md b/README.md index aa679a8..705ddb7 100644 --- a/README.md +++ b/README.md @@ -77,6 +77,7 @@ Exit codes: `0` = clean, `1` = issues found, `2` = error. | [`not_valid_constraints`](docs/rules/not_valid_constraints.md) | FK and CHECK constraints stuck at `NOT VALID` (`convalidated = false`). | | [`sequence_drift`](docs/rules/sequence_drift.md) | Sequences whose `nextval` would collide with rows already in the table. | | [`three_state_boolean`](docs/rules/three_state_boolean.md) | Boolean columns without `NOT NULL` (true / false / null is rarely intended). | +| [`json_over_jsonb`](docs/rules/json_over_jsonb.md) | Columns typed as `json`; `jsonb` supports indexing and is faster on read. | | [`varchar_length`](docs/rules/varchar_length.md) | `varchar(N)` columns where `text` is equivalent in Postgres and avoids the cap. | Every rule has a dedicated page covering rationale, examples, fix SQL, and "when to ignore." Each reported `Issue` carries a `docs_url` pointing at the rule's page, plus a severity (`info` / `warning` / `error`), a fully-qualified object name, a human-readable message, and a suggested fix. diff --git a/docs/rules/json_over_jsonb.md b/docs/rules/json_over_jsonb.md new file mode 100644 index 0000000..44e54f8 --- /dev/null +++ b/docs/rules/json_over_jsonb.md @@ -0,0 +1,86 @@ +# json_over_jsonb + +> **Severity:** warning +> Columns typed as `json` (instead of `jsonb`). + +## What it catches + +Any column whose type is `json`. The `jsonb` type — almost always what you actually want — is **not** flagged. + +```sql +CREATE TABLE events ( + id bigserial PRIMARY KEY, + payload json, -- ⚠️ flagged + meta jsonb -- ok +); +``` + +## Why it matters + +`json` and `jsonb` look similar but have very different storage and read characteristics: + +| | `json` | `jsonb` | +| --- | --- | --- | +| Storage | text, exactly as written | parsed binary, normalized | +| Read | reparses every time | parses once at write | +| Whitespace, key order, duplicates | preserved | normalized away | +| GIN index | not supported | supported | +| Containment operators (`@>`, `<@`, `?`, `?&`, `?|`) | not supported | supported | +| Speed on read | slower | faster | + +The practical impact: + +### 1. No indexing + +`json` columns can't be indexed in any useful way. Once your table has more than a few thousand rows, every `WHERE payload->>'status' = 'pending'` becomes a sequential scan. `jsonb` lets you build a GIN index that turns the same query into a quick lookup: + +```sql +CREATE INDEX events_payload_gin ON events USING gin (payload jsonb_path_ops); +``` + +### 2. Reparsing on every read + +Every time you read a `json` column, Postgres parses the text again from scratch. For a heavily-read table, this is a measurable per-query cost — and it's silent, because the parsing is fast on small payloads but compounds at scale. + +### 3. No containment / existence operators + +`payload @> '{"status": "pending"}'` or `payload ? 'user_id'` are `jsonb`-only. With `json` you fall back to text comparison or extracting individual fields, which is both slower and more code. + +### 4. Duplicate keys silently kept + +`{"a": 1, "a": 2}` is preserved as-is in `json`. `jsonb` keeps the last occurrence. Most clients (Python, JS, Go) take the same "last wins" approach when reading, but the database and the client now disagree about what the value is. + +The only thing `json` is *better* at is preserving the literal input bytes, including whitespace and key order. Almost no application needs this — and the few that do (some signature-verification flows) usually want the bytes stored as plain `text`, not `json`. + +## How to fix + +Convert in place: + +```sql +ALTER TABLE events + ALTER COLUMN payload TYPE jsonb + USING payload::jsonb; +``` + +The cast is straightforward — Postgres reparses each value into the binary representation. For very large tables, the `ALTER COLUMN ... TYPE` rewrite takes a full ACCESS EXCLUSIVE lock for the duration; copy-table-and-swap is usually safer than an in-place rewrite on a hot table. + +## When to ignore + +Suppress the rule for a specific column if you genuinely need byte-for-byte preservation (rare): + +```toml +[pgsleuth] +exclude_tables = ["^webhook_signatures$"] +``` + +Or, project-wide: + +```toml +[pgsleuth.checkers.json_over_jsonb] +enabled = false +``` + +## See also + +- [PostgreSQL — JSON Types](https://www.postgresql.org/docs/current/datatype-json.html) +- [PostgreSQL — JSON Functions and Operators](https://www.postgresql.org/docs/current/functions-json.html) diff --git a/src/pgsleuth/checkers/__init__.py b/src/pgsleuth/checkers/__init__.py index 04d3af6..a122b8d 100644 --- a/src/pgsleuth/checkers/__init__.py +++ b/src/pgsleuth/checkers/__init__.py @@ -4,6 +4,7 @@ from pgsleuth.checkers import ( # noqa: F401 column_value_at_risk, fk_type_mismatch, + json_over_jsonb, missing_fk_index, missing_primary_key, not_valid_constraints, diff --git a/src/pgsleuth/checkers/json_over_jsonb.py b/src/pgsleuth/checkers/json_over_jsonb.py new file mode 100644 index 0000000..3d3ea0e --- /dev/null +++ b/src/pgsleuth/checkers/json_over_jsonb.py @@ -0,0 +1,58 @@ +"""Columns typed as `json` instead of `jsonb`. + +`json` stores the literal text of the input — including whitespace, key +order, and duplicate keys — and reparses it on every read. `jsonb` parses +once at write time, normalizes the structure, and supports indexing +(GIN), containment operators (`@>`, `<@`), and is materially faster on +read. The only thing `json` preserves that `jsonb` doesn't is the exact +input bytes; almost no application cares about that. +""" + +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, + c.relname AS table, + a.attname AS column +FROM pg_attribute a +JOIN pg_class c ON c.oid = a.attrelid +JOIN pg_namespace n ON n.oid = c.relnamespace +WHERE c.relkind IN ('r', 'p') + AND a.attnum > 0 + AND NOT a.attisdropped + AND a.atttypid = 'json'::regtype + {schema_filter} +ORDER BY n.nspname, c.relname, a.attname; +""" + + +class JsonOverJsonb(RowChecker): + name: ClassVar[str] = "json_over_jsonb" + description: ClassVar[str] = "Columns typed as `json`; `jsonb` is almost always preferable." + default_severity: ClassVar[Severity] = Severity.WARNING + sql: ClassVar[str] = _SQL + + def check_row(self, ctx: CheckerContext, row: dict) -> Issue | None: + obj = f"{row['schema']}.{row['table']}.{row['column']}" + return self.issue( + ctx, + object_type="column", + object_name=obj, + message=( + f"Column {obj} is `json`; `jsonb` supports indexing, containment " + f"operators, and is faster on read." + ), + suggestion=( + f"ALTER TABLE {row['schema']}.{row['table']} " + f"ALTER COLUMN {row['column']} TYPE jsonb USING {row['column']}::jsonb;" + ), + ) + + +register(JsonOverJsonb) diff --git a/tests/checkers/test_json_over_jsonb.py b/tests/checkers/test_json_over_jsonb.py new file mode 100644 index 0000000..e83f68b --- /dev/null +++ b/tests/checkers/test_json_over_jsonb.py @@ -0,0 +1,43 @@ +from pgsleuth.checkers.json_over_jsonb import JsonOverJsonb + + +def test_clean_when_jsonb(ctx, conn, schema): + with conn.cursor() as cur: + cur.execute("CREATE TABLE t (id bigserial PRIMARY KEY, payload jsonb)") + + issues = [i for i in JsonOverJsonb().run(ctx) if i.object_name.startswith(schema)] + assert issues == [] + + +def test_flags_json(ctx, conn, schema): + with conn.cursor() as cur: + cur.execute("CREATE TABLE t (id bigserial PRIMARY KEY, payload json)") + + issues = [i for i in JsonOverJsonb().run(ctx) if i.object_name.startswith(schema)] + assert len(issues) == 1 + assert issues[0].object_name == f"{schema}.t.payload" + + +def test_flags_only_json_columns(ctx, conn, schema): + with conn.cursor() as cur: + cur.execute( + "CREATE TABLE t (" + " id bigserial PRIMARY KEY," + " payload json," + " meta jsonb," + " notes text" + ")" + ) + + issues = [i for i in JsonOverJsonb().run(ctx) if i.object_name.startswith(schema)] + assert len(issues) == 1 + assert issues[0].object_name == f"{schema}.t.payload" + + +def test_dropped_column_is_ignored(ctx, conn, schema): + with conn.cursor() as cur: + cur.execute("CREATE TABLE t (id bigserial PRIMARY KEY, payload json)") + cur.execute("ALTER TABLE t DROP COLUMN payload") + + issues = [i for i in JsonOverJsonb().run(ctx) if i.object_name.startswith(schema)] + assert issues == []