-
Notifications
You must be signed in to change notification settings - Fork 22
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/revamp -- add new tables and fields #39
Feature/revamp -- add new tables and fields #39
Conversation
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.
Hi @fivetran-jamie wow lots of new tables! It looks good, just had some minor things:
- cast timestamps fields in staging models with dbt.type_timestamp (I recall this being included in our standardization updates so may as well follow it here)
- maybe update 'index' in the respective staging models to specify which model it's from? This is more if you think this field gets used downstream at some point, and there's so many models that the same field (customer_tag, abandoned_checkout_discount_code, order_shipping_tax_line, order_tag, product_tag)
- I noticed you added the
union data
feature to the tmp models, don't forget to add thesource relation
macro and column to staging models!
ah yeah index won't really get used so i think we're good not renaming it. otherwise, added the source relation stuff and the timestamp casts! will also make sure to add the timestamp casts to the other models in my other PR |
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.
The timestamp updates and union data macro addition look good to go 👍
…an/dbt_shopify_source into feature/revamp/new-tables
Are you a current Fivetran customer?
fivetran made PR
What change(s) does this PR introduce?