search

fivetran / dbt_linkedin

Fivetran's Linkedin Ads dbt package
https://fivetran.github.io/dbt_linkedin/
Apache License 2.0
2 stars 12 forks source link

look into why we add a day to the adapter model joins... #20

Closed fivetran-jamie closed 1 year ago

fivetran-jamie commented 2 years ago

Is there an existing feature request for this?

Describe the Feature

investigate why we changed it to that...

https://getdbt.slack.com/archives/C01D1R2JLLA/p1650539408651009

Describe alternatives you've considered

No response

Are you interested in contributing this feature?

Anything else?

No response

fivetran-jamie commented 2 years ago

So this will actually become a non-issue soon. in every other ad reporting package, we only join in the latest version of entities like accounts or campaigns or creatives. We will be extending this to linkedin as well, as it currently joins in the historical version of the entity that was valid at the time.

So, these lines

 from metrics
    left join creatives
        on metrics.creative_id = creatives.creative_id
        and {{ fivetran_utils.timestamp_add('day', 1, 'metrics.date_day') }} >= creatives.valid_from
        and {{ fivetran_utils.timestamp_add('day', 1, 'metrics.date_day') }} <= coalesce(creatives.valid_to, {{ fivetran_utils.timestamp_add('day', 1, dbt_utils.current_timestamp()) }})

will instead look like

 from metrics
    left join creatives
        on metrics.creative_id = creatives.creative_id
        and creatives.is_lastest_version = true

We'll be rolling this out in the next month or so as part of our big Ad Reporting V2 updates.

As for the original question though (why are we adding a day) I believe this is because the valid_to and valid_from fields are timestamps that could be in the middle of a date_day, which starts at 00:00. an alternative solution would've been to date_trunc the valid_from and valid_to fields and then compare them (without adding a day)

fivetran-joemarkiewicz commented 1 year ago

The latest version of the Ad Reporting package (v.1.0.0) does not use this logic anymore. As such, we can comfortably close this issue as it is no longer relevant.