Skip to content

fix: convert_timezone must use to_local_time(), not the Postgres pattern #1

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.

Sign up for GitHub

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

Merged
eddietejeda merged 2 commits into from
Jul 27, 2026
Merged

fix: convert_timezone must use to_local_time(), not the Postgres pattern #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ba4ab52
fix: convert_timezone must use to_local_time(), not the Postgres pattern
eddietejeda Jul 27, 2026
5951bb6
Address review: default falsy source_tz to UTC; changelog entry
eddietejeda Jul 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Fixed

- `convert_timezone` returned the UTC instant unshifted; it now converts via
`to_local_time()` (DST-aware, non-UTC sources verified) and defaults a
falsy `source_tz` to UTC.

## [0.1.0] - 2026-07-27

### Added
Expand Down
19 changes: 13 additions & 6 deletions dbt/include/hotdata/macros/utils.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,19 @@
{%- endmacro %}

{% macro hotdata__convert_timezone(column, target_tz, source_tz) -%}
{#- dbt_date dispatch hook: DataFusion supports the Postgres double
AT TIME ZONE pattern, so delegate to it. -#}
cast(
cast({{ column }} as timestamp)
at time zone '{{ source_tz }}' at time zone '{{ target_tz }}'
as timestamp
{#-
dbt_date dispatch hook (packages need a root-project shim or dispatch
config to reach it). NOT the Postgres double-AT TIME ZONE + cast-to-naive
pattern: on DataFusion that final cast re-renders the UTC instant,
silently losing the shift. Here AT TIME ZONE on a naive timestamp
localizes it as the source zone's wall clock (12:00 @ America/New_York
-> 12:00-05:00, verified — non-UTC sources convert correctly), and
to_local_time() renders the target zone's wall clock (DST-aware).
-#}
{%- set source_tz = source_tz or 'UTC' -%}
to_local_time(
(cast({{ column }} as timestamp) at time zone '{{ source_tz }}')
at time zone '{{ target_tz }}'
)
Comment on lines +26 to 29

@claude claude Bot Jul 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

source_tz is interpolated but has no effect, so any non-UTC source silently returns the wrong wall clock — the same failure mode this PR is fixing.

The PR description's own finding establishes it: cast(x as timestamp) at time zone src at time zone tgt cast back to naive returns the original value. That round-trip is only lossless if AT TIME ZONE on DataFusion is metadata-only (attach/relabel the display zone, UTC instant unchanged) rather than Postgres-style localization. Given that, ... at time zone '{{ source_tz }}' is a pure no-op and the expression reduces to to_local_time(cast({{ column }} as timestamp) at time zone '{{ target_tz }}') — the input is always treated as UTC.

Concretely, convert_timezone(ts, target_tz='UTC', source_tz='America/New_York') on 2024-01-01 12:00 should yield 17:00; this returns 12:00, with no error. The verification in the description only covers source_tz='UTC', which is why it passed.

Two options, either is fine:

  1. Reject what isn't supported, matching the datediff pattern already in this file:
{%- set source_tz = source_tz or 'UTC' -%}
{%- if source_tz | upper != 'UTC' -%}
  {% do exceptions.raise_compiler_error("convert_timezone source_tz '" ~ source_tz ~ "' is not supported on Hotdata; only UTC sources can be converted") %}
{%- endif -%}
  1. Actually apply the source offset, if the engine has a from_local_time-style inverse.

Note the source_tz or 'UTC' default is worth keeping either way: dbt_date can dispatch here with source_tz=None, which currently renders at time zone 'None'.

@eddietejeda eddietejeda Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested the premise before choosing an option — source_tz does work on this engine. AT TIME ZONE on a naive timestamp localizes it as the source zone's wall clock (Postgres-style), not a metadata relabel:

  • cast('2024-01-01 12:00:00' as timestamp) at time zone 'America/New_York' → 2024-01-01T12:00:00-05:00
  • NY 12:00 → UTC target: 17:00 ✓ (your concrete case)
  • NY 12:00 → LA target: 09:00 ✓

The round-trip observation that motivated the inference has a different explanation: the loss was in the cast-to-naive rendering step (it renders the UTC instant), not in AT TIME ZONE — so the round trip is lossless even though the conversion is real.

Applied the part that stands: source_tz or 'UTC' default (a direct dispatch with source_tz=None did render at time zone 'None'), plus a comment documenting the verified source semantics, plus the changelog entry from the nit. Verified end-to-end through the macro via dbt show: NY source → 17:00, None source → UTC default → 04:00 LA.

{%- endmacro %}

Expand Down
Loading