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

Backport: Add support for Dimension(...).grain(...) in where filter #162

Merged
merged 1 commit into from
Oct 3, 2023

Conversation

DevonFulcher
Copy link
Contributor

Resolving: SL-631

Description

Due to MetricFlow's reliance on version 0.2.x of DSI, the updates to support Dimension(...).grain(...) need to be made to this branch as well. I just combined the work from these PRs into this one #156, #154, #152

Checklist

@linear
Copy link

linear bot commented Sep 29, 2023

SL-631 Where Clause & Group By Updates

At launch (tomorrow):

  1. Group by is the same
  2. Where clause for time dimensions: where="{{ TimeDimension('metric_time', 'DAY') > '2020-01-01'}}"
  3. Where clause for categorical: where="{{ Dimension('customer__country') == 'US' }} "

Post launch we update where clause to do this

  1. Where clause will support the same as the GROUP BY - group_by={{ [Dimension('metric_time').grain('day') }}"
  2. Any time dimension will require grain at all times in the group by (AND WHERE)
  3. Both TimeDimension() and Dimension() will be supported in both GROUP_BY and WHERE

We should discuss remove "Dimension" out of categorical (e..g, allow a user to do {{ 'customer__**country' }} rather than {{ Dimension('customer__**country') }}

@cla-bot cla-bot bot added the cla:yes label Sep 29, 2023
@DevonFulcher DevonFulcher force-pushed the backport_query_interface branch from 6867db7 to 8df2758 Compare September 29, 2023 17:47
Copy link
Collaborator

@tlento tlento left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all seems fine to me, given my understanding of the changes and how the backport deployment works with core 1.6.x, but I'd like @QMalcolm to take a look to confirm this is backwards compatible with existing 1.6 installations. In particular, I think there are some changes to the where_filter_parser along with the file move here.

Copy link
Collaborator

@QMalcolm QMalcolm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I unfortunately don't think we can back port this. Core imports the WhereFilterParser, and this changes that import path. Core's import of DSI is ~=0.2.0 in dbt-core 1.6.0 through 1.6.5, if we backported and released this, all fresh installs of those versions would break.

@tlento
Copy link
Collaborator

tlento commented Oct 2, 2023

I unfortunately don't think we can back port this. Core imports the WhereFilterParser, and this changes that import path. Core's import of DSI is ~=0.2.0 in dbt-core 1.6.0 through 1.6.5, if we backported and released this, all fresh installs of those versions would break.

Is that the only issue? If so, can we add an import alias to the backport then? We should be able to put in a module in the original location that imports this WhereFilterParser (or defines a WhereFilterParser class that wraps it, if there are API changes that need to be managed).

Copy link
Collaborator

@QMalcolm QMalcolm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DevonFulcher brought up the idea of moving the where_filter_parser.py file back out for the backport in slack, also @tlento brought it in an above comment. Combing through these changes, and imports of dbt-semantic-interfaces in dbt-core, I think we can get away with this by doing so.

@DevonFulcher DevonFulcher force-pushed the backport_query_interface branch from 8df2758 to c72779f Compare October 2, 2023 22:03
@DevonFulcher DevonFulcher force-pushed the backport_query_interface branch from c72779f to 828752c Compare October 2, 2023 22:11
@tlento
Copy link
Collaborator

tlento commented Oct 2, 2023

Yay! 🚢

@DevonFulcher DevonFulcher merged commit ac3ae95 into 0.2.latest Oct 3, 2023
7 checks passed
@DevonFulcher DevonFulcher deleted the backport_query_interface branch October 3, 2023 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants