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

adapters w/o boolean support can't use 'check' strategy on snapshots with new columns #4488

Closed
dataders opened this issue Dec 14, 2021 · 2 comments
Labels
enhancement New feature or request jira Team:Adapters Issues designated for the adapter area of the code
Milestone

Comments

@dataders
Copy link
Contributor

dataders commented Dec 14, 2021

I'm surprised no dbt-sql server user ever reports this issue before, but this TRUE causes problems for SQL dialects that don't support boolean types. related: dbt-msft/dbt-sqlserver#186 thanks to @oscar-konig for making a reproducible example that led to this fix!

Rather than asking adapter maintainers to override snapshot_check_strategy, a better answer would be a simple macro for generating a boolean expression with the default of course being TRUE, but in non-core adapters, it could be 1=1

{%- set row_changed_expr -%}
(
{%- if column_added -%}
TRUE
{%- else -%}
{%- for col in check_cols -%}
{{ snapshotted_rel }}.{{ col }} != {{ current_rel }}.{{ col }}
or
(
(({{ snapshotted_rel }}.{{ col }} is null) and not ({{ current_rel }}.{{ col }} is null))
or
((not {{ snapshotted_rel }}.{{ col }} is null) and ({{ current_rel }}.{{ col }} is null))
)
{%- if not loop.last %} or {% endif -%}
{%- endfor -%}
{%- endif -%}
)
{%- endset %}

@jtcohen6 jtcohen6 added Team:Adapters Issues designated for the adapter area of the code triage labels Dec 15, 2021
@jtcohen6 jtcohen6 added this to the v1.1.0 milestone Jan 4, 2022
@jtcohen6 jtcohen6 added the enhancement New feature or request label Jan 5, 2022
@McKnight-42
Copy link
Contributor

@dataders tracking your work on the PR, thank you so much for pointing this out, please let me know if there is anything I can do to help.

@McKnight-42
Copy link
Contributor

Thank you @dataders for all the work you put in on this closing this issue as it was pushed up via #4871

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request jira Team:Adapters Issues designated for the adapter area of the code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants