Standardize courserun_is_current in dbt#1939
Conversation
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| {% macro is_courserun_current(courserun_start_on, courserun_end_on) %} | ||
| case | ||
| when | ||
| date(from_iso8601_timestamp({{ courserun_start_on }})) <= current_date |
There was a problem hiding this comment.
I think this might need to use the cross-database macro for timestamp handling in order for it to work locally with DuckDB.
There was a problem hiding this comment.
Currently we can't run these two models in duckdb, but I've added the cross-database macro to work with DuckDB
| then true | ||
| when | ||
| {{ from_iso8601_timestamp('mitxonline_runs.courserun_start_on') }} <= current_date | ||
| and {{ from_iso8601_timestamp('mitxonline_runs.courserun_end_on') }} > current_date |
There was a problem hiding this comment.
Not sure how important this detail is but should the new macro's logic match the older logic exactly? I'm wondering about how this code has ">" but the new code has ">="
There was a problem hiding this comment.
This code is doing a timestamp boundary comparison:
from_iso8601_timestamp(mitxonline_runs.courserun_end_on) > current_date
vs
the new code performs a date-based comparison:
cast(from_iso8601_timestamp(end_on_timestamp_str) as date) >= current_date
The old logic is time-precision based, while the new logic is date-based, so they behave slightly differently.
If we change from cast(from_iso8601_timestamp(end_on_timestamp_str) as date) >= current_date to cast(from_iso8601_timestamp(end_on_timestamp_str) as date) > current_date will produce false if end_date is today. Also, this field is not highly time-sensitive, given how frequently the dbt models are built, so a date-based comparison should be enough unless the requirements change in the future
quazi-h
left a comment
quazi-h
left a comment
There was a problem hiding this comment.
Changes look good to me. The only thing I was curious about was the. detail about greater than vs greater than or equal to for the logic with current_date.
3 participants
What are the relevant tickets?
NA
noticed when reviewing #1906 (review)
Description (What does it do?)
Standardize
courserun_is_currentLogic in dbt, as currently its implemented in two different ways. But they should be consistentHow can this be tested?
dbt build --select int__combined__course_runs mitxonline_video_engagements_w_video_counts
Additional Context