You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Tracking issue for one question: does ALTER EXTENSION UPDATE reliably produce the same database as a fresh CREATE EXTENSION?
All work carries the update-safety label. Sub-issues are linked below.
Scope: this is forward-looking
Released update scripts are locked and will not be touched barring something serious. The pair that matters is the current development cycle — sql/cat_tools--<last-released>.sql.in → sql/cat_tools.sql.in, with sql/cat_tools--<last-released>--stable.sql.in as the accumulator every SQL-touching PR extends. That accumulator exists and is tracked. Checks are run against historical pairs to validate the checks themselves, which is useful, but protecting those pairs is not the goal and findings against them are not blockers.
Corollary for tooling: false positives are cheap. Future update scripts can be required to satisfy whatever rule we adopt, so a check that occasionally needs a one-line waiver is fine. Silent failure — reporting green while having understood nothing — is not.
Separation of concerns
Changes that add or alter cat_tools features get their own PRs, separate from work that only improves CI and checking. Where a check would be more useful as a user-facing function than as test scaffolding, prefer the function — but weigh ongoing maintenance and testing cost before committing to that.
Choosing the right mechanism per case
The governing principle for everything below: think hard about the right way to verify each individual case. Use a parser where parsing is genuinely easy. Where it is not, the answer is a different mechanism — not a cleverer parser. A case that is awkward to check by text comparison is positive evidence that it belongs somewhere else, and declining to handle it in a static checker is the correct outcome rather than a coverage gap to apologize for.
Worked example: enum contents. The install script states a full label list in CREATE TYPE … AS ENUM (…); an update script says ALTER TYPE … ADD VALUE, one label at a time. Teaching a static checker that those two statement forms mean the same change is exactly the kind of special-casing that makes such a tool grow without bound. A test asserting each enum's contents is both simpler and stronger — the suite already runs identical expected output against fresh, updated and pg_upgraded databases, so a disagreement fails on its own, in whichever mode diverges. Enum support was therefore dropped from #92 and replaced by #97.
Status
D1 — privilege divergence between fresh and updated installs: fixed by Converge privileges between a fresh install and an updated one #96. pg_init_privs was 319 rows fresh vs 324 updated; it is now 336 in both, and pg_dump output differs only by its per-dump \restrict nonce.
A fresh install and an updated install can have identical runtime behaviour while differing in catalog state that pg_dump cares about. D1 was exactly this: typacl byte-identical and has_type_privilege true on both sides, yet five pg_init_privs rows differed, because ALTER DEFAULT PRIVILEGES grants implicitly and implicit grants are not snapshotted.
That means our checks are imperfect in a specific, nameable way, and that should be written down rather than discovered again. bin/structural_diff compares per-object ACL columns and reported these databases identical. A text-level check cannot see it at all. Only a pg_init_privs comparison or a pg_dump diff catches it.
Plan
Privilege correctness
ALTER DEFAULT PRIVILEGES stays. It is not merely documentation — it actively enforces the default, and removing it would break restore for already-updated databases whose dumps carry no explicit GRANT. Explicit REVOKE/GRANT normalization (landed in #96) is the second half of a belt-and-suspenders arrangement, not a replacement: ADP enforces the default, normalization guarantees the explicit end state.
Remaining: detect privilege drift directly (#95), so normalization is defence-in-depth rather than the only thing standing between us and drift. Deferred for now — the immediate priority was getting the two paths in sync, which #96 did. A pg_init_privs comparison must land after #96 or it fires on the divergence #96 fixed.
The ADP invariant check — for every pg_default_acl entry, assert every existing object of that class/schema/owner already carries the privilege — is worth building as a real cat_tools feature rather than test scaffolding, since it is useful against any database including production, and needs no superuser. Feature PR, separate from CI work. Weigh maintenance cost first.
Roles — cat_tools__usage is created inside a DO block; CREATE ROLE fires no event trigger and a role is not an extension member, so nothing sees it. Approach: compare global objects via pg_dumpall, which needs brand-new clusters per side.
Leaked objects — created by a script and left behind as non-members. Detectable via event-trigger DDL capture during CREATE EXTENSION; nothing else covers it. Note this needs superuser, so it is a testing tool only and cannot be part of a non-superuser install path.
Object inventory — a test/sql/ file dumping extension members via pg_describe_object with expected output from make results. Rides the existing suite in fresh, update and post-pg_upgrade modes for free, and makes object additions visible in the PR diff for human review.
Release-time gate
The only moment this truly matters is a release, since that is when a version's contents are locked. A heavier, partly interpretive check is acceptable there and need not be cheap enough for every PR: pg_dump comparison of fresh vs updated across every supported update origin, pg_init_privs included, plus review of the update script against the inter-release diff. A dump comparison alone would have caught D1 with no new tooling.
Static early-warning check
#92, kept deliberately simple. Its value is catching small, subtle diffs — a changed function body, a changed grant, a tweaked view definition. Whole new objects are comparatively easy to spot unaided in a diff of the install script, so the check does not need to chase them and should not grow machinery to do so. Enum labels are explicitly out of scope; see the principle above and #97.
Related work, tracked separately
#88 (non-superuser install, superuser = off) constrains the permission model — see the comment below for the constraints adopted here.
Tracking issue for one question: does
ALTER EXTENSION UPDATEreliably produce the same database as a freshCREATE EXTENSION?All work carries the
update-safetylabel. Sub-issues are linked below.Scope: this is forward-looking
Released update scripts are locked and will not be touched barring something serious. The pair that matters is the current development cycle —
sql/cat_tools--<last-released>.sql.in→sql/cat_tools.sql.in, withsql/cat_tools--<last-released>--stable.sql.inas the accumulator every SQL-touching PR extends. That accumulator exists and is tracked. Checks are run against historical pairs to validate the checks themselves, which is useful, but protecting those pairs is not the goal and findings against them are not blockers.Corollary for tooling: false positives are cheap. Future update scripts can be required to satisfy whatever rule we adopt, so a check that occasionally needs a one-line waiver is fine. Silent failure — reporting green while having understood nothing — is not.
Separation of concerns
Changes that add or alter cat_tools features get their own PRs, separate from work that only improves CI and checking. Where a check would be more useful as a user-facing function than as test scaffolding, prefer the function — but weigh ongoing maintenance and testing cost before committing to that.
Choosing the right mechanism per case
The governing principle for everything below: think hard about the right way to verify each individual case. Use a parser where parsing is genuinely easy. Where it is not, the answer is a different mechanism — not a cleverer parser. A case that is awkward to check by text comparison is positive evidence that it belongs somewhere else, and declining to handle it in a static checker is the correct outcome rather than a coverage gap to apologize for.
Worked example: enum contents. The install script states a full label list in
CREATE TYPE … AS ENUM (…); an update script saysALTER TYPE … ADD VALUE, one label at a time. Teaching a static checker that those two statement forms mean the same change is exactly the kind of special-casing that makes such a tool grow without bound. A test asserting each enum's contents is both simpler and stronger — the suite already runs identical expected output against fresh, updated and pg_upgraded databases, so a disagreement fails on its own, in whichever mode diverges. Enum support was therefore dropped from #92 and replaced by #97.Status
pg_init_privswas 319 rows fresh vs 324 updated; it is now 336 in both, andpg_dumpoutput differs only by its per-dump\restrictnonce.bin/structural_diffnever compares the extension's own schema: open, see structural_diff: cover the extension's own schema #94.Known limitation to document
A fresh install and an updated install can have identical runtime behaviour while differing in catalog state that
pg_dumpcares about. D1 was exactly this:typaclbyte-identical andhas_type_privilegetrue on both sides, yet fivepg_init_privsrows differed, becauseALTER DEFAULT PRIVILEGESgrants implicitly and implicit grants are not snapshotted.That means our checks are imperfect in a specific, nameable way, and that should be written down rather than discovered again.
bin/structural_diffcompares per-object ACL columns and reported these databases identical. A text-level check cannot see it at all. Only apg_init_privscomparison or apg_dumpdiff catches it.Plan
Privilege correctness
ALTER DEFAULT PRIVILEGESstays. It is not merely documentation — it actively enforces the default, and removing it would break restore for already-updated databases whose dumps carry no explicitGRANT. ExplicitREVOKE/GRANTnormalization (landed in #96) is the second half of a belt-and-suspenders arrangement, not a replacement: ADP enforces the default, normalization guarantees the explicit end state.Remaining: detect privilege drift directly (#95), so normalization is defence-in-depth rather than the only thing standing between us and drift. Deferred for now — the immediate priority was getting the two paths in sync, which #96 did. A
pg_init_privscomparison must land after #96 or it fires on the divergence #96 fixed.The ADP invariant check — for every
pg_default_aclentry, assert every existing object of that class/schema/owner already carries the privilege — is worth building as a real cat_tools feature rather than test scaffolding, since it is useful against any database including production, and needs no superuser. Feature PR, separate from CI work. Weigh maintenance cost first.Detection gaps
bin/structural_diffmisses the extension's own schema. Small, independent, actionable now.pg_init_privs,pg_default_aclstate, column-level ACLs).cat_tools__usageis created inside aDOblock;CREATE ROLEfires no event trigger and a role is not an extension member, so nothing sees it. Approach: compare global objects viapg_dumpall, which needs brand-new clusters per side.CREATE EXTENSION; nothing else covers it. Note this needs superuser, so it is a testing tool only and cannot be part of a non-superuser install path.test/sql/file dumping extension members viapg_describe_objectwith expected output frommake results. Rides the existing suite in fresh, update and post-pg_upgrademodes for free, and makes object additions visible in the PR diff for human review.Release-time gate
The only moment this truly matters is a release, since that is when a version's contents are locked. A heavier, partly interpretive check is acceptable there and need not be cheap enough for every PR:
pg_dumpcomparison of fresh vs updated across every supported update origin,pg_init_privsincluded, plus review of the update script against the inter-release diff. A dump comparison alone would have caught D1 with no new tooling.Static early-warning check
#92, kept deliberately simple. Its value is catching small, subtle diffs — a changed function body, a changed grant, a tweaked view definition. Whole new objects are comparatively easy to spot unaided in a diff of the install script, so the check does not need to chase them and should not grow machinery to do so. Enum labels are explicitly out of scope; see the principle above and #97.
Related work, tracked separately
#88 (non-superuser install,
superuser = off) constrains the permission model — see the comment below for the constraints adopted here.