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

[ADAP-381] Enable tagging of query_group in configs #376

Open
3 tasks done
trouze opened this issue Mar 16, 2023 · 5 comments
Open
3 tasks done

[ADAP-381] Enable tagging of query_group in configs #376

trouze opened this issue Mar 16, 2023 · 5 comments
Labels
enhancement New feature or request good_first_issue Good for newcomers

Comments

@trouze
Copy link

trouze commented Mar 16, 2023

Is this your first time submitting a feature request?

  • I have read the expectations for open source contributors
  • I have searched the existing issues, and I could not find an existing issue for this feature
  • I am requesting a straightforward extension of existing dbt-redshift functionality, rather than a Big Idea better suited to a discussion

Describe the feature

Much akin to dbt-snowflake's implementation of query tagging, I think it'd be great to allow users to set a query_group for their models. I'd imagine the implementation of this would be quite straightforward as the functionality is quite similar to that of Snowflake's, but hoping to get input there.

Describe alternatives you've considered

Not aware that there are any alternatives.

Who will this benefit?

Anybody who uses the Redshift query logs to perform analysis on their queries.

Are you interested in contributing this feature?

Certainly! Feels like it could be a good first issue.

Anything else?

No response

@trouze trouze added enhancement New feature or request triage labels Mar 16, 2023
@github-actions github-actions bot changed the title Enable tagging of query_group in configs [ADAP-381] Enable tagging of query_group in configs Mar 16, 2023
@dbeatty10 dbeatty10 self-assigned this Mar 17, 2023
@dbeatty10
Copy link
Contributor

Good idea @trouze !

Agreed that this might be relatively straightforward especially having the query_tag config in dbt-snowflake as prior art. Will label this as a good_first_issue.

One way to see the relevant implementation of the feature and its tests in dbt-snowflake is searching in GitHub:
https://github.com/dbt-labs/dbt-snowflake/search?q=query_tag

Personally, I prefer to do a git grep with a local clone of the dbt-snowflake repo for this type of search:

git grep "query_tag" .

@dbeatty10 dbeatty10 added good_first_issue Good for newcomers and removed triage labels Mar 17, 2023
@dbeatty10 dbeatty10 removed their assignment Mar 17, 2023
@dbeatty10
Copy link
Contributor

A design decision for refinement

The name of the actual configuration is a piece of the design that we'll need to refine:

  • query_group vs. query_tag

Argument for query_group:

  • query_group is the vendor-specific name of the session setting in Redshift
  • query_tag is the vendor-specific name of the session setting in Snowflake

Argument for query_tag:

  • From what I can tell, query_group in Redshift and query_tag in Redshift basically do the same thing
  • query_tag is a pre-existing config name within dbt (for dbt-snowflake) so the naming and documentation would be more uniform to keep this name the same

Overall, I think either choice would work fine -- there are trade-offs either way.

A final option would be to allow query_tag and query_group to be aliases of each other in both dbt-redshift and dbt-snowflake.

@dbeatty10 dbeatty10 added the refinement Product or leadership input needed label Mar 17, 2023
@trouze
Copy link
Author

trouze commented Mar 17, 2023

Sweet!

I think the biggest question I have at the outset here is over materializations. In dbt-snowflake the set_query_tag() method is called in each of the adapter-specific materializations. Obviously, dbt-redshift currently relies on the base dbt-core materializations supplied through the global_project. So the question is, does this necessitate the need for adapter specific materializations to be introduced to dbt-redshift, or is there another way to add this functionality without doing that?

@trouze
Copy link
Author

trouze commented Mar 20, 2023

To follow up, there is some development that maintainers can take a look at here.

Regarding question above, only other option I foresee would be to adjust logic in the create_table_as() and create_view_as() macros, but this would mean any pre or post-hooks would not be captured within the desired query group.

@dbeatty10
Copy link
Contributor

Nice progress @trouze !

So the question is, does this necessitate the need for adapter specific materializations to be introduced to dbt-redshift, or is there another way to add this functionality without doing that?

It seems like there would be two main ways to approach this:

  1. introduce adapter specific materializations to dbt-redshift
  2. introduce query_tag configuration to dbt-core (and most adapters would just raise NotImplemented (or something similar)

Approach #1 seems the least complicated in the short-term. Adapter specific materializations will probably be introduced by #204 if they aren't introduced here first.

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
3 participants