Use timestamp_tz type in microbatch delete DDL
#1257
+23
−3
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…sql incremental_predicates
MichelleArk
commented
Nov 27, 2024
| @@ -58,10 +58,10 @@ | |||
|
|
|||
| {#-- Add additional incremental_predicates to filter for batch --#} | |||
| {% if model.config.get("__dbt_internal_microbatch_event_time_start") -%} | |||
| {% do incremental_predicates.append("DBT_INTERNAL_TARGET." ~ model.config.event_time ~ " >= TIMESTAMP '" ~ model.config.__dbt_internal_microbatch_event_time_start ~ "'") %} | |||
| {% do incremental_predicates.append("DBT_INTERNAL_TARGET." ~ model.config.event_time ~ " >= to_timestamp_tz('" ~ model.config.__dbt_internal_microbatch_event_time_start ~ "')") %} | |||
There was a problem hiding this comment.
Interestingly, just using the raw timestamp (that includes UTC offset, which model.config.__dbt_internal_microbatch_event_time_start does) works well. E.g. just removing TIMESTAMP would have been enough here. However, the to_timestamp_tz piece for more intentional so I've opted for that.
There was a problem hiding this comment.
I like how using TIMESTAMP was causing the issue 🫠 I like th emove to using to_timestamp_tz instead of just removing TIMESTAMP 👍
QMalcolm
approved these changes
Nov 27, 2024
You are crushiiiing it| @@ -58,10 +58,10 @@ | |||
|
|
|||
| {#-- Add additional incremental_predicates to filter for batch --#} | |||
| {% if model.config.get("__dbt_internal_microbatch_event_time_start") -%} | |||
| {% do incremental_predicates.append("DBT_INTERNAL_TARGET." ~ model.config.event_time ~ " >= TIMESTAMP '" ~ model.config.__dbt_internal_microbatch_event_time_start ~ "'") %} | |||
| {% do incremental_predicates.append("DBT_INTERNAL_TARGET." ~ model.config.event_time ~ " >= to_timestamp_tz('" ~ model.config.__dbt_internal_microbatch_event_time_start ~ "')") %} | |||
There was a problem hiding this comment.
I like how using TIMESTAMP was causing the issue 🫠 I like th emove to using to_timestamp_tz instead of just removing TIMESTAMP 👍
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
resolves ##1256
docs dbt-labs/docs.getdbt.com/#
Problem
When specifying a timestamp like
TIMESTAMP '2024-11-05 00:00:00+00:00', snowflake uses the system timezone, even if the timestamp has offsets. This causes issues in microbatch because thedeleteDDL statement is leveraging theTIMESTAMPtyping, and is deleting the wrong records, potentially causing duplicate rows to be inserted.Solution
to_timestamp_tzwhen computing incremental predicates for microbatch DDLChecklist