fivetran / dbt_stripe

Data models for Stripe built using dbt.
https://fivetran.github.io/dbt_stripe/
Apache License 2.0
30 stars 31 forks source link

[Bug] Timezone conversion doesn't work on postgres #35

Closed johnf closed 2 years ago

johnf commented 2 years ago

Is there an existing issue for this?

Describe the issue

When trying to set the timezone I get an error.

PostgreSQL doesn't have a convert_timezone function. If you look at dbt_packages/dbt_utils/macros/cross_db_utils/current_timestamp.sql you will see that they use something like ({{ column }} at time zone '{{ var("stripe_timezone") }}')::{{dbt_utils.type_timestamp()}} instead

Relevant error log or model output

07:21:34  Database Error in model stripe__customer_overview (models/stripe__customer_overview.sql)
07:21:34    function convert_timezone(unknown, timestamp without time zone) does not exist
07:21:34    LINE 63:     convert_timezone('Sydney/Australia', created_at)
07:21:34                 ^
07:21:34    HINT:  No function matches the given name and argument types. You might need to add explicit type casts.
07:21:34    compiled SQL at target/run/stripe/models/stripe__customer_overview.sql
07:21:34

Expected behavior

No error and timezones should be converted

dbt Project configurations

vars: using_invoices: false using_subscriptions: false using_credit_notes: false stripe_timezone: "Sydney/Australia" stripe_schema: stripe_load stripe_database: dwh_prod

Package versions

packages:

What database are you using dbt with?

postgres

dbt Version

nstalled version: 1.0.4 latest version: 1.0.4

Up to date!

Plugins:

Additional Context

No response

Are you willing to open a PR to help address this issue?

fivetran-joemarkiewicz commented 2 years ago

Hi @johnf thanks so much for opening this issue!

I see exactly what you are talking about with postgres support for the macro is not possible at the moment. However, you do call out a good point with how dbt_utils handles this currently for postgres. To adjust this macro to allow for postgres support I believe all we would need to do is open a PR that adds a new postgres__date_timezone(column) section within the macro. Similar to how we have it for bigquery: https://github.com/fivetran/dbt_stripe/blob/de061f7ac38f49b029b95e4dffe01becb930ff05/macros/date_timezone.sql#L7-L14

I noticed you mentioned you would be open to creating a PR on your end! We are always excited when people from the community want to contribute back to our packages. Please let me know if you would like to work on a PR to account for postgres compatibility in this macro. I am happy to help answer any questions you may have!

Otherwise, my team can tackle this issue in a future sprint. Let me know!

johnf commented 2 years ago

PR ready to roll! :)

fivetran-reneeli commented 2 years ago

Hi @johnf, thank you for opening the PR! We've tested and included your changes to the package. Closing this issue!