-
Notifications
You must be signed in to change notification settings - Fork 39
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
add update deduplication when there are multiple entries that updates the #30
base: main
Are you sure you want to change the base?
add update deduplication when there are multiple entries that updates the #30
Conversation
…me ad performance entry
We're also seeing issues related to this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@chmstimoteo I added a dedup filter to the source sqls, if there's intereste in that solution I can add a PR for it and skip the workaraound in this PR. |
@martenlindblad pls create a PR with the proposed changes. |
Just added it: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pls, apply similar fix to the update_fact_ad_conversion_daily.sqlx
if yet not applied.
ad_id, | ||
date_id, | ||
device, | ||
MAX(account_status) AS account_status, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kingman these columns with MAX() operators to deduplicate are slow changing dimensions, right?
Click, costs, impressions can be set to MAX... to get the latest performance values.
However, statuses we should get the current statuses (check this: https://cloud.google.com/bigquery/docs/google-ads-transfer#query_your_data)
deduplication when there are multiple entries that updates the same ad performance entry