Optimize grant macros for performance, case-insensitive roles, and empty-list fix#34
Merged
jonhopper-dataengineers merged 4 commits intoJun 25, 2026
Conversation
…atching Grant macros now check existing state before issuing SQL statements, skipping schemas/objects where grants are already in place. This reduces runtime from 20-30 minutes to under 2-3 minutes on large databases where grants are already applied. Also makes all role comparisons case-insensitive via normalization to uppercase. Adds integration tests for grant helpers and idempotency. .... Generated with [Cortex Code](https://docs.snowflake.com/en/user-guide/cortex-code/cortex-code) Co-Authored-By: Cortex Code <noreply@snowflake.com>
…idempotency - _grants_get_schema_object_privs: removed unused _role variable, added internal grantees uppercase normalization, added optional object_type parameter to filter by specific object type - grant_schema_read_specific: removed incorrect per-privilege skip logic that aggregated across object types. GRANT ON ALL is idempotent in Snowflake so always issue it. Only skip schema USAGE (single check) and future grants (which error on duplicates) - grant_schema_object_privileges: now passes object_type to both the helper query and the revoke query to avoid cross-object-type leakage .... Generated with [Cortex Code](https://docs.snowflake.com/en/user-guide/cortex-code/cortex-code) Co-Authored-By: Cortex Code <noreply@snowflake.com>
Guard NOT IN () clauses with length checks to prevent invalid SQL when grant_roles/role_list is an empty list. Snowflake rejects NOT IN () as a syntax error. .... Generated with [Cortex Code](https://docs.snowflake.com/en/user-guide/cortex-code/cortex-code) Co-Authored-By: Cortex Code <noreply@snowflake.com>
Contributor
Reviewer's GuideRefactors Snowflake grant macros to be faster and idempotent by bulk-querying privilege state, normalizing role/application names to uppercase for case-insensitive matching, and making grant_object grant-only, while adding helper/query macros, tests, and documentation/version updates. Sequence diagram for optimized grant_schema_read_specific flowsequenceDiagram
actor DbtRunner
participant grant_schema_read_specific
participant _grants_normalize_roles
participant _grants_get_schema_grants
participant _grants_get_future_grants
participant information_schema
DbtRunner->>grant_schema_read_specific: call grant_schema_read_specific(schemas, grant_roles,...)
grant_schema_read_specific->>_grants_normalize_roles: normalize grant_roles
_grants_normalize_roles-->>grant_schema_read_specific: uppercased grant_roles
loop per schema
grant_schema_read_specific->>_grants_get_schema_grants: _grants_get_schema_grants(schema, 'USAGE', 'ROLE')
_grants_get_schema_grants->>information_schema: show grants on schema
information_schema-->>_grants_get_schema_grants: existing USAGE grants
_grants_get_schema_grants-->>grant_schema_read_specific: roles_with_usage
alt revoke_current_grants and grant_roles not empty
grant_schema_read_specific->>information_schema: select table_privileges for roles not in grant_roles
information_schema-->>grant_schema_read_specific: rows to revoke
grant_schema_read_specific->>grant_schema_read_specific: append revoke statements
end
alt include_future_grants
grant_schema_read_specific->>_grants_get_future_grants: _grants_get_future_grants(schema)
_grants_get_future_grants->>information_schema: show future grants in schema
information_schema-->>_grants_get_future_grants: existing future grants
_grants_get_future_grants-->>grant_schema_read_specific: role_future map
end
grant_schema_read_specific->>grant_schema_read_specific: build grant statements (usage, read, future)
opt schema has changes
grant_schema_read_specific->>information_schema: run_query(grant/revoke statements)
end
end
grant_schema_read_specific-->>DbtRunner: completed with many schemas skipped when no changes
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 4 issues, and left some high level feedback:
- The new
_grants_normalize_roleshelper assumes it always receives a list; consider making it resilient to common alternative inputs (e.g., strings,none) so downstream macros don’t have to pre-normalize types themselves. - The share-grant logic that builds
existing_share_objectsfromDESC SHAREis duplicated and slightly varied acrossgrant_external_share_read,grant_internal_share_read, andgrant_share_read_specific_schema; factoring this into a shared helper would reduce repetition and make future changes to the share-inspection logic easier to apply consistently.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `_grants_normalize_roles` helper assumes it always receives a list; consider making it resilient to common alternative inputs (e.g., strings, `none`) so downstream macros don’t have to pre-normalize types themselves.
- The share-grant logic that builds `existing_share_objects` from `DESC SHARE` is duplicated and slightly varied across `grant_external_share_read`, `grant_internal_share_read`, and `grant_share_read_specific_schema`; factoring this into a shared helper would reduce repetition and make future changes to the share-inspection logic easier to apply consistently.
## Individual Comments
### Comment 1
<location path="macros/grants/_helpers.sql" line_range="160" />
<code_context>
+{% endmacro %}
+
+{# Check schema-level grants (USAGE etc) for given roles. Returns list of roles that already have the privilege. #}
+{% macro _grants_get_schema_grants(schema, privilege, grantee_type) %}
+ {% set existing = [] %}
+ {% set query %}
</code_context>
<issue_to_address>
**issue (bug_risk):** Schema grant helper returns raw role names, which can conflict with uppercased `grant_roles` comparisons.
Most callers uppercase `grant_roles` via `_grants_normalize_roles`, but `_grants_get_schema_grants` returns `row.grantee_name` without normalization. In macros like `grant_schema_read`, `grant_schema_operate_specific`, and `grant_schema_monitor_specific`, you compare an uppercased `role` against `roles_with_usage` containing mixed-case values, so case-only differences can cause the membership check to fail and lead to redundant `grant usage on schema` statements. To align behavior, normalize `row.grantee_name` to uppercase inside `_grants_get_schema_grants` so its output matches the casing of `grant_roles`.
</issue_to_address>
### Comment 2
<location path="macros/grants/grant_procedure_usage.sql" line_range="36" />
<code_context>
- {% set grant_results = run_query(grant_query) %}
+ {# Get existing USAGE grants on procedures in this schema in one query #}
+ {% set existing_usage_roles = [] %}
+ {% set usage_query %}
+ select distinct grantee
+ from information_schema.object_privileges
</code_context>
<issue_to_address>
**issue (bug_risk):** Procedure usage revokes are not filtered by grantee type, which may revoke from non-role grantees.
In `grant_schema_procedure_usage_specific`, `usage_query` selects `grantee` for `USAGE` on `PROCEDURE` but doesn’t filter by `granted_to = 'ROLE'`. As a result, grants to non-role grantees (e.g. APPLICATION, SHARE, ACCOUNT) will be treated as roles and generate invalid `revoke usage ... from role {{ role_with_usage }}` statements. Please add `and granted_to = 'ROLE'` to keep this macro consistent with its role-only assumption.
</issue_to_address>
### Comment 3
<location path="macros/grants/grant_schema_object_privileges.sql" line_range="83" />
<code_context>
+ {# Build revoke statements for roles NOT in the list that have these privs on this object type #}
+ {% set revoke_statements = [] %}
+ {% if role_list | length > 0 %}
+ {% set revoke_query %}
+ select distinct privilege_type, grantee
+ from information_schema.object_privileges
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Revoke query relies on caller-supplied privilege casing, which may not match `information_schema` values.
`permission_list` is used directly in:
```sql
and privilege_type in ({{ permission_list | map('tojson') | join(', ') }})
```
without normalizing case, while `information_schema.object_privileges.privilege_type` is typically uppercase. If the caller passes lower/mixed-case privileges, the filter may not match and revokes will be skipped. For consistency with `_grants_get_schema_object_privs`, normalize `permission_list` to uppercase (or upper-case in the query) before using it here.
```suggestion
and privilege_type in ({{ permission_list | map('upper') | map('tojson') | join(', ') }})
```
</issue_to_address>
### Comment 4
<location path="macros/grants/grant_schema_operate.sql" line_range="21-25" />
<code_context>
+ {% set result_map = {} %}
+ {% set priv_filter = privilege_types | map('upper') | list %}
+ {% set grantees_upper = grantees | map('upper') | list %}
+ {% set query %}
+ select privilege_type, grantee
+ from information_schema.object_privileges
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Operate revokes are generated only as bulk revokes on tasks/pipes, regardless of actual object_type.
The query selects all `OPERATE` grants from `information_schema.object_privileges` without filtering by `object_type`, but revokes are only issued as bulk revokes on tasks and pipes. If `OPERATE` is granted on other object types (e.g. stages or future types), those grants won’t be revoked and the reconcile behavior becomes inconsistent. Please either restrict the query to `object_type in ('PIPE','TASK')` or expand the revoke statements to cover all object types you intend to manage so the query and revokes stay aligned.
```suggestion
{% set query %}
select privilege_type, grantee
from information_schema.object_privileges
where privilege_type = 'OPERATE'
and object_schema = '{{ schema }}'
and object_type in ('PIPE','TASK')
{% endset %}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
… and granted_to
- _grants_get_schema_grants: now uppercases grantee_name so output
matches normalized grant_roles for consistent comparisons
- grant_procedure_usage: added granted_to = 'ROLE' filter to prevent
picking up APPLICATION/SHARE grantees and generating invalid revokes
- grant_schema_object_privileges: uppercase permission_list in revoke
query to match information_schema which stores privileges uppercase
- grant_schema_operate_specific: filter by object_type in ('PIPE','TASK')
so OPERATE grants on other object types don't affect reconciliation
- grant_schema_monitor_specific: same object_type filter for consistency
.... Generated with [Cortex Code](https://docs.snowflake.com/en/user-guide/cortex-code/cortex-code)
Co-Authored-By: Cortex Code <noreply@snowflake.com>
jonhopper-dataengineers
deleted the
feature/grant-performance-optimization
branch
June 25, 2026 09:06
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
1 participant
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
Test plan
dbt run-operation test_grants_helpers— validates helper macro logicdbt run-operation test_grants_idempotency— validates helpers and skip-logic against live Snowflakedbt run-operation grants_smoke_test --vars '{"grants_dry_run": true}'— exercises all major grant macros in dry-run modedbt run-operation grant_privilegeson a test environment and verify reduced statement count.... Generated with Cortex Code
Summary by Sourcery
Optimize Snowflake grant macros for faster, idempotent execution and case-insensitive role handling, while tightening grant-only behavior and improving share/database utilities.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests: