Skip to content

update_lint: compare object definitions, not just identity #91

Description

@jnasbyupgrade

bin/update_lint (added in #90) compares object identity only. A changed function body or view definition is invisible to it, so of the two known real fresh-vs-updated divergences in this repo's history it catches one and is structurally blind to the other.

Why this matters more than the caveat suggests

bin/structural_diff's own header names the bug it was written for: cat_tools.trigger__parse, as produced by the pre-0.2.2 update scripts, hardcoded EXECUTE PROCEDURE and skipped an empty-args guard the fresh-install body had, breaking every trigger parse on PG11+. That is a definition divergence, and update_lint cannot see it.

The same blind spot covers the single most consequential update script in this repo's history. sql/cat_tools--0.2.2--0.2.3.sql.in is the _cat_tools.pg_class_v DROP+CREATE rebuild that strips relhasoids/relhaspkey — the repair documented in CLAUDE.md, README.asc and #42, and the reason the PG10 pg-upgrade-test leg has to bridge to 0.2.3 rather than 0.2.2. Its object set is unchanged, so:

$ bin/update_lint --versions 0.2.2 0.2.3
  0 object(s) added, 0 removed
OK: sql/cat_tools--0.2.2--0.2.3.sql.in accounts for all 0 added and 0 removed object(s)

Reduced to the general case — dropping a view column with an empty update script:

$ cat old.sql.in
CREATE SCHEMA s;
CREATE VIEW s.v AS SELECT 1 AS a, 2 AS b;
$ cat new.sql.in
CREATE SCHEMA s;
CREATE VIEW s.v AS SELECT 1 AS a;
$ bin/update_lint old.sql.in new.sql.in /dev/null
  0 object(s) added, 0 removed
OK: /dev/null accounts for all 0 added and 0 removed object(s)   # exit 0

bin/structural_diff does catch this class, at runtime, on every supported major. The point of update_lint is to move detection left off that seven-major job, and right now the class that most needs moving left is the one that stays there.

Proposed approach

A per-key normalized definition fingerprint, reusing the extraction machinery that already exists:

  • For each object key, hash the normalized defining text — comments stripped and whitespace collapsed, which the scanner already computes as its $code view.
  • Compare the hash old-vs-new. Report a gap when it changed and the update script never touches that key.
  • Stays database-free, so it keeps running in the cheap lint job with no new dependency.

The extraction already isolates each object's defining text: literal payloads through __cat_tools.create_function(), and statement text for plain CREATE. So this is an addition on top of what is built, not a redesign.

Worth deciding along the way

  • Whether a fingerprint mismatch should gate or warn. A body edit that is faithfully mirrored in the update script must not be a finding, so coverage matching has to key on the same object — which it already does.
  • How much normalization is enough. Too little and every whitespace change is a finding; too much and a real divergence hides. structural_diff sidesteps this by comparing pg_get_functiondef output, which PostgreSQL itself normalizes — a text-based approximation cannot match that exactly, and the risk of the two checks disagreeing is the main argument against doing this at all.
  • Frozen pairs. 0.2.20.2.3 and the pre-0.2.2 pairs would likely report fingerprint gaps that can never be fixed, so the existing "current pair only by default" scope matters even more here.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    update-safetyALTER EXTENSION UPDATE correctness: fresh-vs-updated equivalence

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions