Add 'duckdb' support #67
Conversation
ericmichael
changed the title
There was a problem hiding this comment.
@ericmichael thanks for opening this PR! As @elanfivetran mentioned in his response to your initial issue #67 we welcome this contribution and your efforts to update the package to be compatible with DuckDB. However, we do not feel comfortable claiming that the package is fully compatible with DuckDB since we do not have the backend infrastructure to test DuckDB compatibility with our integration tests at the moment.
As such, I would be open to approving and merging this PR. However, we will not claim we support DuckDB until we build it out in our backend integration testing pipeline. If that works for you, I have a few suggestions to be applied to your PR. Once these are applied this should be good to approve and merge!
| @@ -7,7 +7,7 @@ with spine as ( | |||
| {% endset %} | |||
| {% set first_date = run_query(first_date_query).columns[0][0]|string %} | |||
|
|
|||
| {% if target.type == 'postgres' %} | |||
| {% if target.type == 'postgres' or target.type == 'duckdb' %} | |||
There was a problem hiding this comment.
Small suggestion to combine postgres and duckdb in the same conditional check.
| {% if target.type == 'postgres' or target.type == 'duckdb' %} | |
| {% if target.type in ('postgres', 'duckdb') %} |
| @@ -34,7 +34,7 @@ with spine as ( | |||
| {% else %} {% set last_date = run_query(current_date_query).columns[0][0]|string %} | |||
| {% endif %} | |||
|
|
|||
| {% if target.type == 'postgres' %} | |||
| {% if target.type == 'postgres' or target.type == 'duckdb' %} | |||
There was a problem hiding this comment.
| {% if target.type == 'postgres' or target.type == 'duckdb' %} | |
| {% if target.type in ('postgres', 'duckdb') %} |
| @@ -1,3 +1,10 @@ | |||
| # dbt_stripe v0.10.2 | |||
| ## 🎉 Feature Updates | |||
| - Included support for DuckDB via ([#67](https://github.com/fivetran/dbt_stripe/pull/67)) | |||
There was a problem hiding this comment.
I am a bit cautious to boldly claim that this package is compatible with DuckDB since we do not have the infrastructure on our end to validate that this does in fact work for DuckDB destinations.
I would request we change the wording of the CHANGELOG entry to reflect this approach. My thoughts are something along the following:
| - Included support for DuckDB via ([#67](https://github.com/fivetran/dbt_stripe/pull/67)) | |
| - Included DuckDB destination in conditional logic within the `int_stripe__date_spine` model. ([#67](https://github.com/fivetran/dbt_stripe/pull/67)) |
We can then claim proudly that we support DuckDB once we have it as part of our integration testing pipeline. However, for now this should be good!
Please provide your name and company
Eric Martinez, 80/20 Jiu-Jitsu
Link the issue/feature request which this PR is meant to address
#66
Detail what changes this PR introduces and how this addresses the issue/feature request linked above.
I encountered an error related to the
+operator not being supported for adding an interval to a string in DuckDB.The specific issue arises in the
int_stripe__date_spine.sqlmodel, where thedbt.dateaddfunction tries to add 1 day to thelast_date_adjustvariable, which is a string. This operation is not supported in DuckDB, causing the model to fail.Since DuckDB uses Postgres syntax, we can simply handle it in the same way as Postgres is handled.
How did you validate the changes introduced within this PR?
Validated locally. Additional changes may be required.
Which warehouse did you use to develop these changes?
DuckDB.
Did you update the CHANGELOG?
Did you update the dbt_project.yml files with the version upgrade (please leverage standard semantic versioning)? (In both your main project and integration_tests)
Provide an emoji that best describes your current mood
💃
Feedback
We are so excited you decided to contribute to the Fivetran community dbt package! We continue to work to improve the packages and would greatly appreciate your feedback on our existing dbt packages or what you'd like to see next.
PR Template
Community Pull Request Template (default)
Maintainer Pull Request Template (to be used by maintainers)