Move DSL operators to df schema and auto-manage search_path in grant_usage (#202)#226
Open
crprashant wants to merge 4 commits into
Open
Move DSL operators to df schema and auto-manage search_path in grant_usage (#202)#226crprashant wants to merge 4 commits into
crprashant wants to merge 4 commits into
Conversation
pgspot flagged the 7 DSL operators (~>, |=>, &, |, ?>, !>, @>) as created in the public schema (PS017). Move them into the df schema to avoid polluting public.
Operators in df.start('a' ~> 'b') resolve in the caller's session before df.start()/df.explain() run, so df must be on the session search_path for the unqualified operator syntax. This is a documented, pre-1.0 behavior change.
- Qualify CREATE OPERATOR as df.<op> in src/lib.rs (fresh install)
- Move operators in the 0.2.2->0.2.3 upgrade script (DROP public, CREATE df)
- Add df to database search_path in E2E runners (local + docker)
- Add search_path GUC to unit-test config (postgresql_conf_options)
- Remove obsolete pgspot PS017 allowlist entry
- Update docs, examples, CHANGELOG, and upgrade-testing notes
The DSL operators now live in df (microsoft#202), so df must be on a role's search_path for the unqualified operator syntax to resolve. Add a fourth optional parameter to df.grant_usage(), set_search_path boolean DEFAULT true, that adds df to the target role's search_path via ALTER ROLE during onboarding (append-only and idempotent; opt out with set_search_path => false). df.revoke_usage(text) keeps its signature and now removes the df entry again. Both ALTER ROLE blocks tolerate insufficient_privilege with a NOTICE so the grant/revoke otherwise succeeds. The 0.2.2->0.2.3 upgrade script drops and recreates grant_usage for the new signature and re-revokes PUBLIC EXECUTE. It includes CREATE SCHEMA IF NOT EXISTS df (a runtime no-op) so the recreated function bodies stay byte-identical to src/lib.rs and pass the pgspot gate; the resulting PS010 finding is allowlisted in run-pgspot.sh. Add E2E test 50_grant_usage_search_path.sql (catalog-based assertions on pg_db_role_setting) and update USER_GUIDE, README, api-reference, upgrade-testing, and CHANGELOG.
Add df to the database-level search_path in 00_init so the unqualified operator syntax (~>, |=>, &, |, ?>, !>, @>) resolves in the regression tests after the operators moved from public to df. Use ALTER DATABASE current_database() in a DO/format() block so it works for both the contrib_regression and postgres databases used by the CI jobs. A per-role setting is not sufficient because the tests use SET ROLE, which does not reload role-level search_path. Update expected/00_init.out for the new df.grant_usage search_path NOTICE and the appended search_path setup block.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #202.
pgspot flagged pg_durable's 7 DSL operators (
~>,|=>,&,|,?>,!>,@>) as being created in thepublicschema (PS017). This PR moves them into thedfschema so the extension no longer pollutespublic— and, so the unqualified operator syntax keeps working without every user hand-editingsearch_path, teachesdf.grant_usage()to manage the role'ssearch_pathduring onboarding.Part 1 — Move operators into
dfOperators inside
df.start('a' ~> 'b')(anddf.explain('...')) are resolved in the caller's session beforedf.start()/df.explain()run. With the operators now indf, the unqualified operator syntax requiresdfon the sessionsearch_path. This is a deliberate, pre-1.0 behavior change; fully-qualified DSL function calls (e.g.df.seq(...)) are unaffected.src/lib.rs— qualify all 7CREATE OPERATORstatements asdf.<op>(fresh install); addsearch_pathGUC to the unit-testpostgresql_conf_options.sql/pg_durable--0.2.2--0.2.3.sql— for existing installs,DROPthepublicoperators and re-CREATEthem indf(0.2.3 is unreleased, so its upgrade script is reused).scripts/run-pgspot.sh— remove the now-obsolete PS017 allowlist entry.Part 2 —
df.grant_usage()managessearch_path(no manual setup)So users get the ergonomic operator syntax out of the box, onboarding now puts
dfon the role'ssearch_path:df.grant_usage(role)gains a fourth optional parameter,set_search_path boolean DEFAULT true. When true (the default) it addsdfto the target role'ssearch_pathviaALTER ROLE— append-only and idempotent (appends, dfto an existing per-role setting, or sets"$user", public, dfwhen none exists). Passset_search_path => falseto opt out and manage it yourself.df.revoke_usage(role)keeps its signature and now also removes thedfentry again (idempotent; other entries preserved;RESETwhen nothing else remains).ALTER ROLEblocks tolerateinsufficient_privilegewith aNOTICE, so a delegated admin who can't alter the target role still completes the grant/revoke.DROPs and re-CREATEsgrant_usage(re-REVOKEing PUBLICEXECUTE) andCREATE OR REPLACEsrevoke_usage. It includes aCREATE SCHEMA IF NOT EXISTS df;— a runtime no-op since the schema already exists — so the recreated function bodies stay byte-identical tosrc/lib.rsand pass the pgspot gate (the resultingPS010is allowlisted inrun-pgspot.sh). Rationale is documented indocs/upgrade-testing.md.Backward compatibility
The new
.socontinues to work against a non-upgraded ≤0.2.2 schema: operators still live inpublicthere, and the caller's defaultsearch_pathincludespublic; the 3-arggrant_usage/ oldrevoke_usageremain callable exactly as before. No Rust code hard-references the operator schema or these admin helpers. AfterALTER EXTENSION UPDATEto 0.2.3, operators move todfandgrant_usage/revoke_usagegain thesearch_pathbehavior.Docs & tests
USER_GUIDE.md,README.md,docs/{grammar,api-reference,ARCHITECTURE,upgrade-testing}.md,.agents/skills/pg-durable-sql/SKILL.md,.github/copilot-instructions.md,CHANGELOG.md, and the example SQL files + operational-scenarios README document/apply thesearch_pathstory.scripts/test-e2e-{local,docker}.shsetdfon the databasesearch_pathafterCREATE EXTENSIONso the suite's unqualified operator usage resolves; newtests/e2e/sql/50_grant_usage_search_path.sqlasserts the grant/revokesearch_pathbehavior (default addsdfonce,set_search_path => falseopts out, existing paths are appended to, double-grant is idempotent, revoke removesdfwhile preserving other entries) via thepg_db_role_settingcatalog.Testing
CI runs the format check, clippy,
cargo pgrx test pg17, the E2E suite, and the upgrade tests (schema comparison + backward-compat) plus the pgspot gate. The modified upgrade script was verified against pgspot locally (passes with only the allowlistedPS010). The build and full E2E suite (including the new test) were also validated locally in Docker (linux/amd64).