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.

Sign up for GitHub

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

Coalesce Incremental Load Fields in shopify__customer_cohorts #86

Merged
merged 3 commits into from
Aug 26, 2024
Merged

Coalesce Incremental Load Fields in shopify__customer_cohorts #86

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
16423a8
coalesce incremental load values
advolut-team Aug 10, 2024
b9f022f
Merge branch 'fivetran:main' into main
advolut-team Aug 17, 2024
2e8b5ed
coalesce windows columns
advolut-team Aug 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions models/shopify__customer_cohorts.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,10 @@ with calendar as (
windows.order_count_in_month,
windows.total_price_in_month,
windows.line_item_count_in_month,
backfill_lifetime_sums.previous_cohort_month_number + windows.cohort_month_number as cohort_month_number,
backfill_lifetime_sums.previous_total_price_lifetime + windows.total_price_lifetime as total_price_lifetime,
backfill_lifetime_sums.previous_order_count_lifetime + windows.order_count_lifetime as order_count_lifetime,
backfill_lifetime_sums.previous_line_item_count_lifetime + windows.line_item_count_lifetime as line_item_count_lifetime,
coalesce(backfill_lifetime_sums.previous_cohort_month_number, 0) + windows.cohort_month_number as cohort_month_number,
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think we should coalesce windows.cohort_month_number in the rarer (but possible) case that this value returns null on an incremental run? There could be an incremental period where no orders are brought in during the window and thus the partition is run on an empty table.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Definitely a rare case, but I think there's no harm adding a coalesce statement to maintain data quality :) I've tested locally by running dbt run with coalesce(windows. cohort_month_number) and I have the same result. Let me add that in my next commit, along with the CHANGELOG and version bump

coalesce(backfill_lifetime_sums.previous_total_price_lifetime, 0) + windows.total_price_lifetime as total_price_lifetime,
coalesce(backfill_lifetime_sums.previous_order_count_lifetime, 0) + windows.order_count_lifetime as order_count_lifetime,
coalesce(backfill_lifetime_sums.previous_line_item_count_lifetime, 0) + windows.line_item_count_lifetime as line_item_count_lifetime,
{{ dbt_utils.generate_surrogate_key(['windows.date_month','windows.customer_id','windows.source_relation']) }} as customer_cohort_id
from windows
left join backfill_lifetime_sums
Expand Down