Incremental snapshot sql fails on postgres 9.6 on dbt >= 0.17.0

[DavidAkroyd]

Describe the bug

Incremental snapshots do not work on Postgres 9.6

Steps To Reproduce

Create a snapshot using DBT when using a Postgres 9.6 database, and then attempt an incremental snapshot (i.e. run dbt snapshot twice)
Error received of

failed to find conversion function from unknown to text

This is a regression of #1665
Another user has identified on the slack that this was caused by #2390 due to the union using non-casted fields introducing the issue in dbt 0.17.0

Expected behavior

DBT incremental snapshots should not throw an error on Postgres 9.6

System information

Which database are you using dbt with?

• postgres
• redshift
• bigquery
• snowflake
• other (specify: ____________)

The output of dbt --version:

Affects dbt >= 0.17.0

Additional context

Whilst I appreciate that this is for a unsupported "legacy" version of Postgres, I hope to be able to personally contribute to introduce to the Postgres plugin the equivalent fix as was introduced in #1665 which should extend the life/support of dbt on 9.6 until it's designated EoL of Nov 2021

[jtcohen6]

@DavidAkroyd Thanks for the detailed bug report. I'm in favor of continuing support for Postgres 9.6, so long the way to do it is straightforward enough.

Is this just a matter of updating lines 83, 101, and 118?

'insert'::text as dbt_change_type,
...
'update'::text as dbt_change_type,
...
'delete'::text as dbt_change_type,

This is just slightly trickier than #1666, because unlike snapshot_merge_sql, snapshot_staging_table is not a dispatched macro—every adapter expects to use the same implementation. There are some other databases that will be less happy with the presence of ::text, so I think the best move here is to create a dispatched adapter macro, string_literal(), along the lines of:

{% macro string_literal(string) -%}
    {{ return(adapter.dispatch('string_literal')(string)) }}
{%- endmacro %}

{% macro default__string_literal(string) %}
    {% set literal -%} '{{ string }}' {%- endset %}
    {{ return(literal)}}
{% endmacro %}

{% macro postgres__string_literal(string) %}
    {% set literal -%} '{{ string }}'::text {%- endset %}
    {{ return(literal)}}
{% endmacro %}

And then the lines in snapshot_staging_table can become:

{{ string_literal('insert') }} as dbt_change_type,
...
{{ string_literal('update') }} as dbt_change_type,
...
{{ string_literal('delete') }} as dbt_change_type,

Is that a fix you'd be interested in contributing?

[DavidAkroyd]

@jtcohen6 Thanks! Yes, as far as I can tell, it simply just requires the updating of these three lines which are causing the issue.
Creating the dispatched adapter macro for string literal sounds like a very sensible approach - I was trying to think of ways which didn't involve changing large amounts of code, and I think this is the most elegant way

I need to confirm if I can find time to set up the environment/sign contributors form etc, and then I should hopefully be able to a) test that this approach fixes the issue and b) contribute it in a PR

Thanks for your help!

[vitoravancini]

Hello,

I'm in a similar possition, needing to overwrite a macro that is not dispatched on order to make snapshots available for oracle database. Specifically this macro:

https://github.com/fishtown-analytics/dbt/blob/38f278cce0a5ccd2f92cc3381f3f0d02d30babe8/core/dbt/include/global_project/macros/materializations/snapshot/snapshot.sql#L142

Should I open another issue for that? I'm asking here because a I could implement changes for both macros.

Thankyou!

[jtcohen6]

Hi @vitoravancini! Is the presence of select *, in that macro all that's standing between you and implementing the default snapshot materialization on Oracle? I think at least snapshot_staging_table is also guilty of the same syntax. If it's just those, your best recourse is to open a PR against dbt to do either/both of:

• Changing select *, to select [alias].*, in each CTE where it occurs
• Making build_snapshot_table and snapshot_staging_table into dispatched macros with default implementations, such that that you (or another adapter author) could more easily reimplement

If it turns out that there are other things you'd need to change about the default materialization, then you'll already need to define {% materialization snapshot, adapter = 'oracle' %}, and you may find it simplest to just copy-paste-update the line:
https://github.com/fishtown-analytics/dbt/blob/38f278cce0a5ccd2f92cc3381f3f0d02d30babe8/core/dbt/include/global_project/macros/materializations/snapshot/snapshot.sql#L217
To instead read:

      {% set build_sql = oracle_build_snapshot_table(strategy, model['compiled_sql']) %}

Where oracle_build_snapshot_table points to your reimplemented version.

[vitoravancini]

I'll change the dbt code in my python env here to see if I need to change anything else.

Any of other database plugins change the default snapshot implementation? If I do have to change it will I have to re implement the whole materialization?

[vitoravancini]

Just saw the dbt-spark do change the snapshot implementation, I can use it as an example if I have to go down that path.

Thanks for the instructions @jtcohen6

[github-actions]

This issue has been marked as Stale because it has been open for 180 days with no activity. If you would like the issue to remain open, please remove the stale label or comment on the issue, or it will be closed in 7 days.