[tianzhou]

Summary

A default privilege only fires for objects created by its FOR ROLE owner, but the diff treated every default as if it would fire for the tables a migration creates. With ALTER DEFAULT PRIVILEGES FOR ROLE other_role ... GRANT SELECT ON TABLES TO reader:

• dump omitted the explicit GRANT SELECT ON TABLE t TO reader on every covered table,
• apply as a role other than other_role then produced tables without the grant (and emitted a REVOKE for the grant it assumed would be auto-created),
• a re-plan reported No changes detected while reader had lost access.

The plan cannot know which role will run it: apply may run as a different role than plan, a saved plan may be executed by anyone, and a dump has no apply role at all. So the diff is now role-independent in both directions:

• Explicit grants on new tables are always emitted, even when an active default would also grant them. A redundant GRANT is a no-op when the default did fire; a missing one was a silent privilege loss. This is the only behavior change from main.
• The conservative REVOKEs for privileges a default would auto-grant on a new table but the desired state does not include (Default privileges + selective REVOKE requires two apply cycles to converge #253) are computed against every active default, as before. The REVOKE is a no-op when the default did not fire and withholds the access when it did, so a plan or dump stays correct under whichever role restores it.

The "don't REVOKE an existing grant that a desired default covers" rule from #250 is unchanged.

Fixes #696

Test plan

• New case testdata/diff/default_privilege/issue_696_for_role_other_owner: a FOR ROLE app_owner default, plus a new table with an explicit grant and one without. Before: no GRANT and a bogus-looking REVOKE. Now: GRANT SELECT ON TABLE accounts TO app_reader plus the conservative REVOKE SELECT ON TABLE audit_log FROM app_reader; apply as the test role and re-plan are idempotent.
• New unit test TestDumpKeepsExplicitGrantsAndWithheldDefaults (internal/diff): a dump keeps both the explicit grant and the REVOKE for a withheld grant. Fails on main (the GRANT is missing).
• default_privilege/add_table_privilege and privilege/revoke_default_privilege now also list the explicit grants on their new tables; all other cases in both categories are unchanged.
• Verified against the reproduction in Default privileges FOR ROLE drop explicit table grants: apply as another role revokes them and plan reports no changes #696 end to end: dump keeps the grant, apply as postgres grants it, has_table_privilege is true, re-plan is clean.

PGSCHEMA_TEST_FILTER="default_privilege/" go test ./cmd -run TestPlanAndApply
PGSCHEMA_TEST_FILTER="privilege/" go test ./cmd -run TestPlanAndApply
go test ./internal/diff

🤖 Generated with Claude Code

[greptile-apps]

Confidence Score: 1/5

[High impact] The change affects privilege plans for tables created by pgschema.

Fix the two paths that can grant unintended table access before merging.

Findings

1.   Dumps restore withheld access ▶
2.   Saved plans grant unexpected access ▶

Summary

Records current_user in Metadata.CurrentRole and limits assumed automatic table grants to defaults owned by that role.

• Migration plans match default grants to the role creating each table.

_{Reviews (1) · Last reviewed commit: "fix: only treat default privileges FOR R..." · Reviewed by Greptile}

[Copilot]

🟡 Changes recommended

Unresolved critical privilege-handling issues can omit required grants or revokes when saved plans execute under a different role and restore explicitly revoked access when dumps are reloaded.

2 open issues

• Reject saved plans applied under a different effective role · New
• Preserve revoked grants in dump output · New

What changed in this PR

This PR addresses #696 by making pgschema’s default-privilege handling depend on the inspected role, preserving explicit grants when another role owns the defaults.

Changes:

• Records the connected role in IR metadata.
• Filters automatic-grant assumptions by role and retains explicit grants in dumps.
• Adds regression fixtures covering new tables with and without explicit grants under another role’s defaults.

File	Description
testdata/​diff/​default_privilege/​issue_696_for_role_other_owner/​plan.txt	Expected human-readable migration plan.
testdata/​diff/​default_privilege/​issue_696_for_role_other_owner/​plan.sql	Expected executable plan SQL.
testdata/​diff/​default_privilege/​issue_696_for_role_other_owner/​plan.json	Expected serialized migration plan.
testdata/​diff/​default_privilege/​issue_696_for_role_other_owner/​old.sql	Establishes another role’s default privileges.
testdata/​diff/​default_privilege/​issue_696_for_role_other_owner/​new.sql	Adds tables with and without explicit SELECT grants.
testdata/​diff/​default_privilege/​issue_696_for_role_other_owner/​diff.sql	Expects the explicit GRANT without an unnecessary REVOKE.
ir/​ir.go	Adds current-role metadata.
ir/​inspector.go	Captures current_user during inspection.
internal/​diff/​diff.go	Restricts active defaults to the inspected role.

_{Review effort: Balanced}

[Copilot]

🟢 Approval recommended

No active review issues are identified for the changes preserving explicit table grants while retaining conservative REVOKEs.

No open issues 🎉

2 resolved since last review

• Preserve revoked grants in dump output
• Reject saved plans applied under a different effective role

_{Review effort: Balanced}