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

Support for date_part for Dimension & TimeDimension #183

Merged
merged 1 commit into from
Oct 24, 2023

Conversation

DevonFulcher
Copy link
Contributor

Description

This PR adds support for the date_part for Dimension & TimeDimension. This is helpful for our Tableau integration. A time_dimension_call_parameter_set is now created if Dimension(...).grain(...) or Dimension(...).date_part(...) is used or both.

Checklist

@cla-bot cla-bot bot added the cla:yes label Oct 23, 2023
@DevonFulcher DevonFulcher marked this pull request as ready for review October 23, 2023 16:00
@DevonFulcher DevonFulcher requested review from tlento, plypaul and QMalcolm and removed request for tlento October 23, 2023 16:00
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.

Let's not add support for WEEK here, I'll just need to remove it again later. And we might want to update the docstring on the DatePart enum to indicate the reasoning.

Otherwise, this looks good to me!

dbt_semantic_interfaces/type_enums/date_part.py Outdated Show resolved Hide resolved
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.

This looks great! Some other people left comments on some things to add / change, and I don't have more to add.

Separately, I noticed that the issue in the changie doc is listed as None. Can we start creating issues for this kind of work moving forward? We have a nice template for this kind of internally motivated work. Having the issue is helpful for when we generate the release notes with changie. Having the issue in advance gives us in core an idea of whats coming down the pipe.

Additionally, is this going to need to be backported and put into a 0.3.latest patch release? If so we need to plan this work on the core side. Again, having a DSI issue will help with this.

tests/parsing/test_where_filter_parsing.py Outdated Show resolved Hide resolved
dbt_semantic_interfaces/type_enums/date_part.py Outdated Show resolved Hide resolved
@DevonFulcher DevonFulcher force-pushed the date_part_filter branch 2 times, most recently from 757a91a to ddb5db4 Compare October 24, 2023 22:10
@DevonFulcher DevonFulcher merged commit 4c98d80 into main Oct 24, 2023
9 checks passed
@DevonFulcher DevonFulcher deleted the date_part_filter branch October 24, 2023 23:01
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.

4 participants