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

fix: dateadd supports more than 25 days for every unit of time (DNA-21769: DNA-21829) #86

Merged
merged 7 commits into from
Jan 4, 2024

Conversation

cverhoef
Copy link
Contributor

@cverhoef cverhoef commented Jan 4, 2024

Description

  • To fix the bug: the number argument is casted to a bigint for Databricks. Only for Databricks we do computations on this argument which can result in (negative) overflow values when the number is of type int instead of bigint.
  • Simplified the implementation:
    • Replaced the date_add used for day and week with the same approach as for the more granular units. We now have 2 different approaches instead of 3 to cover all scenarios.
    • Moved the "Jinja set variables" to places more close to where it is used.
  • Docs: updated the readme to fix the table of contents and removed the condition that the number of units should be an integer.
  • Added tests:
    • Additions of more than 25 days. Both in seconds, which we use ourselves, but also in days as this seems to be a more standard scenario.
    • Additions of doubles. This is also supported and used, although the values differ per database as doubles are rounded differently.

Release

  • Direct release (main)
  • Merge to dev (or other) branch
    • Why:

Did you consider?

  • Does it Work on Automation Suite / SQL Server
  • Does it Work on Automation Cloud / Snowflake
  • What is the performance impact?
  • The version number in dbt_project.yml

@cverhoef cverhoef changed the title fix: dateadd also support more than 25 days for every unit of time fix: dateadd supports more than 25 days for every unit of time (DNA-21769: DNA-21829) Jan 4, 2024
@cverhoef cverhoef merged commit 45362e2 into main Jan 4, 2024
4 checks passed
@cverhoef cverhoef deleted the fix/dateadd branch January 4, 2024 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants