Skip to content

fix(bindings): fix distance return types; add + shift alias for tstzset/tstzspan+interval#107

Closed
estebanzimanyi wants to merge 1 commit into
mainfrom
fix/span-distance-return-types
Closed

fix(bindings): fix distance return types; add + shift alias for tstzset/tstzspan+interval#107
estebanzimanyi wants to merge 1 commit into
mainfrom
fix/span-distance-return-types

Conversation

@estebanzimanyi
Copy link
Copy Markdown
Member

@estebanzimanyi estebanzimanyi commented May 4, 2026

👀 Reviewers: tier ranking, dependency chains and the standards checklist live in doc/contributing/reviewer-guide.md.

Summary

  • Integer and date span distance operators were registered with INTEGER/BIGINT return types but the C++ executor wrote DOUBLE values → INTERNAL error at runtime. Fixed by using BinaryExecutor<..., int32_t/int64_t> with DatumGetInt32/Int64.
  • tstzspan distance was registered as DOUBLE but callers expect INTERVAL (e.g. 2 days, not 172800.0). Fixed to return interval_t via SecsToInterval.
  • Added + and shift aliases for tstzset/tstzspan + interval (DuckDB cannot define custom infix operators, so + is registered as a named function alias).

Test plan

  • intspan '[1,3]' <-> intspan '[5,7]' returns 2 (INTEGER)
  • datespan '[2000-01-01,2000-01-03]' <-> datespan '[2000-01-10,2000-01-15]' returns correct day count
  • tstzspan '[2000-01-01,2000-01-03]' <-> tstzspan '[2000-01-10,2000-01-15]' returns 7 days INTERVAL

🤖 Generated with Claude Code

…et/tstzspan+interval

Distance operators for integer and date spans were registered with
INTEGER/BIGINT return types but the C++ executor wrote DOUBLE values →
INTERNAL error at runtime.  tstzset/tstzspan distance was registered as
DOUBLE but callers expect INTERVAL (2 days, not 172800.0).

Changes:
- Distance_span_value / Distance_value_span: change int/bigint/date cases
  to BinaryExecutor<..., int32_t/int64_t> using DatumGetInt32/Int64;
  change tstzspan case to BinaryExecutor<..., interval_t> via SecsToInterval.
- Distance_span_span: replace single double-returning executor with a
  type-dispatching switch (int32/int64/double/interval_t per span type);
  uses distance_tstzspan_tstzspan for the tstzspan branch.
- Distance_set_value / Distance_value_set / Distance_set_set: change
  tstzset branch from double to interval_t, converting via SecsToInterval.
- set.cpp / span.cpp: update tstzset distance registrations to INTERVAL;
  add "+"(tstzset, INTERVAL) and "+"(tstzspan, INTERVAL) as shift aliases.
- time_util.hpp: add SecsToInterval helper (seconds → DuckDB interval_t).
- test/sql/parity/009_time_ops.test: activate all 4 assertions (previously
  inside mode skip).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@estebanzimanyi
Copy link
Copy Markdown
Member Author

Consolidation note — overlaps with #106

See #106 comment for the full overlap matrix. Both PRs touch distance return types in src/temporal/span_functions.cpp with different helper names (SecondsToInterval here vs SecsToInterval in this PR's time_util.hpp) and different Distance_span_span shapes (this PR's per-type switch is the more complete one).

Maintainer call which to keep, which to fold.

@estebanzimanyi
Copy link
Copy Markdown
Member Author

Superseded by the consolidated parity batch at #126 (feat/parity-additions-batch, commit c53a482) and the final 14-name closing batch at #130 (feat/parity-final-batch, commit 22c64c0).

The work on this branch — distance return types + tstzset/tstzspan+interval alias — is now part of the squashed addressable-parity surface that brings MobilityDuck to 943/943 = 100.0% of the active addressable temporal + geo names. Closing to keep the review queue focused on the three active stacked PRs (#126#130#129) and the four orthogonal PRs (#105 / #111 / #113 / #117 / #120).

Branch fix/span-distance-return-types is left intact; reopen if a regression appears in the squashed batches and the originals turn out to carry a unique fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant