Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Fix grants statements on materialized views #1267

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: Fix grants statements on materialized views #1267

wants to merge 2 commits into from

Conversation

AcidFlow
Copy link

@AcidFlow AcidFlow commented Jun 25, 2024 •
edited
Loading

resolves #1268

Problem

Granting privileges on materialized views was failing because the generated SQL statement used the relation type materialized_view as part of the query instead of materialized view (with a space).

Solution

This PR defines an override for this specific materialization in the and grant/revoke macros to fix the generated query.

Checklist

  • I have read the contributing guide and understand what's expected of me
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • This PR has no interface changes (e.g. macros, cli, logs, json artifacts, config files, adapter interface, etc) or this PR has already received feedback and approval from Product or DX

@cla-bot cla-bot bot added the cla:yes label Jun 25, 2024
@AcidFlow
fix: Fix grants statements on materialized views
953deed 
Granting privileges on materialized views was failing because the
generated SQL statement used the relation type `materialized_view`
as part of the query instead of `materialized view` (with a space).

This commit defines an override for this specific materialization in
the and grant/revoke macros.
@AcidFlow AcidFlow force-pushed the fix/grants-materialized-view branch from 5712dcf to 953deed Compare June 25, 2024 12:59
@AcidFlow AcidFlow mentioned this pull request Jun 25, 2024
Open
2 tasks
@AcidFlow AcidFlow marked this pull request as ready for review June 25, 2024 14:06
@AcidFlow AcidFlow requested a review from a team as a code owner June 25, 2024 14:06
@kubikb
Copy link

kubikb commented Oct 10, 2024

Hello @AcidFlow @VersusFacit @colin-rogers-dbt , could you please take a look at this PR? This is an issue that requires a custom workaround from us. This PR has been open for some time and the fix here would resolve this persistent issue.

@AcidFlow
Copy link
Author

Hello @AcidFlow @VersusFacit @colin-rogers-dbt , could you please take a look at this PR? This is an issue that requires a custom workaround from us. This PR has been open for some time and the fix here would resolve this persistent issue.

Hey @kubikb !

As a workaround if this PR does not get enough attention, what we did internally was to copy the two macros changed in this PR, to our dbt project macros directory. They will then take precedence over the one in the dbt-bigquery adapter and fix the issue until a fix is available upstream.

@colin-rogers-dbt
Copy link
Contributor

Apologies for the delay here, we do want to get this merged in but will likely look to extend our functional tests to cover this case here

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

@VersusFacit VersusFacit

Labels
cla:yes materialized_views
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug] Using grants on materialized_view raises an error
4 participants
@AcidFlow @kubikb @colin-rogers-dbt @VersusFacit