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.

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

Make require_partition_filter=true in dbt_project.yml affect only partitioned tables #107

Closed
oliverrmaa opened this issue Jan 25, 2022 · 5 comments · Fixed by #109
Closed
Assignees
Labels
enhancement New feature or request good_first_issue Good for newcomers

Comments

@oliverrmaa
Copy link
Contributor

Describe the feature

Currently if I add require_partition_filter=true to my dbt_project.yml it will apply this to all models in my repo, including the models that materialize as views or non-partitioned tables when materialized. This will throw the following error in BigQuery:

require_partition_filter value in table OPTIONS is only supported for a CREATE TABLE statement with a PARTITION BY clause

I would like a feature that lets me in my dbt_project.yml to be able to only apply the require_partition_filter to models that will materialize as partitioned tables.

Describe alternatives you've considered

One alternative is to go through every single model config in my repo and only add require_partition_filter to the model configs that materialize as partitioned tables. However we have hundreds of models so this would be a very manual and cumbersome process.

Additional context

N/A

Who will this benefit?

This will benefit users that want to add require_partition_filter universally and directly in their dbt_project.yml and have it affect only the right models instead of manually going through all their configs.

Are you interested in contributing this feature?

Based on difficulty / timeline for making this feature available, I could be willing to help

@oliverrmaa oliverrmaa added enhancement New feature or request triage labels Jan 25, 2022
@McKnight-42 McKnight-42 self-assigned this Jan 25, 2022
@McKnight-42
Copy link
Contributor

at @oliverrmaa Thank you for the write up. Firstly I have gotten a few older issues and PRs that fall in the vein of your issue with require_partition_filter :

I think #65 really has a lot we can work with either trying something like it, or to add a condition in get_table_options to check that the table has a partition_by config.

I believe this could be a small change but add some more finite control for us in regards to configuration control, so I'm going to mark this a good first issue. Would you be interested in contributing the fix?

@oliverrmaa
Copy link
Contributor Author

oliverrmaa commented Jan 26, 2022

Hi @McKnight-42 ! Sure I think I can give it a try. If I am understanding you correctly, I think it really is a very simple change. I opened a PR here https://github.com/dbt-labs/dbt-bigquery/pull/109/files

I followed the contribution guidelines but am having a bit with my test.env file. What are the items that need to go in BIGQUERY_TEST_SERVICE_ACCOUNT_JSON? Additionally, what would I put in BIGQUERY_TEST_DATABASE? BQ itself is the database here right?

I will be able to run tests once I have the test.env file figured out.

@McKnight-42
Copy link
Contributor

McKnight-42 commented Jan 26, 2022

@oliverrmaa oh fantastic I can try to clear as much as I can. With the test.env you would want to create a service account key for your bigquery project and can follow these instructions here you would put the json object in the BIGQUERY_TEST_SERVICE_ACCOUNT_JSON location, BIGQUERY_TEST_DATABASE seems to be same as the project_id, and for BIGQUERY_TEST_ALT_DATABASE i'd say just place a different name than what your current BIGQUERY_TEST_DATABASE is or use a different projects project_id here. PLEASE make sure you git ignore these files before pushing up.

@oliverrmaa
Copy link
Contributor Author

Awesome, thanks @McKnight-42, everything is working now and I see the tests passing. I have checked that off the testing requirement in my checklist in the PR and it is ready for review by your and your team. Thanks again!

@McKnight-42
Copy link
Contributor

McKnight-42 commented Jan 27, 2022

Fantastic! Thank you for all your hard work on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good_first_issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants