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

[Feature] Remove the age_groups and age coalesce in adgroup_history staging model #25

Closed
1 of 4 tasks
fivetran-joemarkiewicz opened this issue Nov 18, 2024 · 0 comments · Fixed by #28
Closed
1 of 4 tasks
Assignees
Labels
good first issue Good for newcomers type:enhancement New functionality or enhancement

Comments

@fivetran-joemarkiewicz
Copy link
Collaborator

Is there an existing feature request for this?

  • I have searched the existing issues

Describe the Feature

Back in release v0.4.0 we added the new age_groups field which was meant to replace the soon to be deprecated age field. As this was over a year ago we should be comfortable removing the age field from the age_group coalesce within the stg_tiktok_ads__ad_group_history model.

How would you implement this feature?

To implement this feature we will simply remove the age field from the coalesce and no longer leverage a coalesce, instead we will simply reference the age_groups field.

coalesce(age_groups, age) as age_groups,

Additionally, we will want to remove the age field from the fill staging macro for the adgroup history table.

{"name": "age", "datatype": dbt.type_string()},

Describe alternatives you've considered

Leaving the field in and not removing it from the coalesce.

Are you interested in contributing this feature?

  • Yes.
  • Yes, but I will need assistance.
  • No.

Anything else?

While this is a feature request, we should also treat this as an enhancement which will address a number of issues raised by customers due to the age_groups field being interpreted as a json datatype. Customers have reported experiencing the following error type:

cannot coalesce types JSON and string

Therefore, removing this coalesce will ensure only the one datatype is inferred and there is no clash when trying to coalesce fields. I was able to test this theory locally with a JSON datatype and confirmed that removing the age coalesce ensured the data model succeeded as expected.

@fivetran-joemarkiewicz fivetran-joemarkiewicz added good first issue Good for newcomers type:enhancement New functionality or enhancement labels Nov 18, 2024
@fivetran-catfritz fivetran-catfritz self-assigned this Dec 6, 2024
@fivetran-catfritz fivetran-catfritz linked a pull request Dec 9, 2024 that will close this issue
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers type:enhancement New functionality or enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants