Skip to content

DX-126900: [C++][Gandiva] Add TimeIR for unit-aware time64[us/ns] support - #146

Merged
lriggs merged 2 commits into
dremio:dremio_27.0_23_19from
akravchukdremio:DX-126900-gandiva-time64
Nov 4, 2026
Merged

lriggs merged 2 commits into
dremio:dremio_27.0_23_19from
akravchukdremio:DX-126900-gandiva-time64

Conversation

@akravchukdremio

@akravchukdremio akravchukdremio commented Oct 3, 2026 •

Copy link
Copy Markdown

Rationale for this change

Dremio now produces native Arrow time64[us] / time64[ns] columns for TIME(6) / TIME(9) (epic DX-122844). Gandiva registered no time64 functions, so every such expression fell back to Java. This adds native time64 support the same way TimestampIR (#137, #138) added timestamp[us/ns] support. Jira: https://dremio.atlassian.net/browse/DX-126900

What changes are included in this PR?

  • Registers time64 signatures with time64[us]:
    • extractHour/Minute/Second (aliases hour/minute/second)
    • equal (eq, same), not_equal, less_than, less_than_or_equal_to, greater_than, greater_than_or_equal_to
    • isnull, isnotnull, is_distinct_from, is_not_distinct_from
  • function_signature.cc: the time64 unit is not part of the signature (like timestamp), so time64[ns] matches the same entry.
  • time_ir.h/.cc + LLVMGenerator::ResolveTimePcName(): remaps time64[ns] calls to _ns variants in the new precompiled/time_unit_ops.cc. Comparisons and null checks are unit-agnostic and need no variant.
  • Safety differences from TimestampIR:
  • Time32 functions are unchanged. time64 is int64, so the time32 (int32, millis) functions cannot be reused.

Are these changes tested?

Yes.

  • precompiled/time_test.cc: raw _time64 / _time64_ns extract functions, including boundary values.
  • tests/date_time_test.cc, end to end through Projector for both units:
    • extract at 00:00:00, 23:00:00 (> INT32_MAX) and 23:59:59.999999(999)
    • parity with the time32 functions
    • comparisons between values 1 unit apart
    • null handling
    • rejection of mixed units
  • llvm_generator_test.cc: ResolveTimePcName cases, plus VerifyTime64Functions, which checks every registered time64 function resolves to an existing precompiled function for both units.
  • function_signature_test.cc: time64[us] and time64[ns] signatures compare equal and hash equally.

Locally, after rebasing onto the latest dremio_27.0_23_19 (includes #145 and #148): gandiva-precompiled-test (133), gandiva-internals-test (197) and gandiva-projector-test (258, with TZ=UTC) all pass.

GitHub CI: the failing jobs are pre-existing and unrelated. The same jobs fail on #145 and #148 (both merged), mostly because the MinIO download returns HTTP 410 and a pinned Homebrew pkg-config@0.29.2 formula was removed.

Test jar build from this branch + dremio/arrow-java#35, used by dremio/dremio#29364:

Are there any user-facing changes?

New time64 functions in the Gandiva registry. Dremio routes TIME(6)/TIME(9) expressions to them via its pushdown sieve (DX-124702).

Requires the companion arrow-java fix that reports time64 types as TIME64 in the expression registry (dremio/arrow-java#35).

🤖 Generated with Claude Code

Comment thread cpp/src/gandiva/llvm_generator_test.cc Resolved
Comment thread cpp/src/gandiva/time_ir.h Resolved
lriggs
lriggs previously approved these changes Oct 30, 2026
akravchukdremio and others added 2 commits November 4, 2026 01:13
…port

Register time64 function signatures (extractHour/Minute/Second, comparisons,
isnull/isnotnull, is_distinct_from/is_not_distinct_from) with time64[us].
Signature matching ignores the time64 unit, and LLVMGenerator::ResolveTimePcName()
remaps time64[ns] calls to the _ns precompiled variants, mirroring TimestampIR.

Unlike timestamp, a time64[ns] call without an _ns variant is rejected instead of
falling through to the microsecond function, and mixed time64 units are rejected.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…n, test negative time values

Note above TIME_TYPES that time64 and timestamp[us/ns] are handled outside the
macro (separate time64[us] registration + codegen remapping via TimeIR/TimestampIR),
and add a test showing negative time64 values behave like time32.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@akravchukdremio
akravchukdremio force-pushed the DX-126900-gandiva-time64 branch from e796b06 to 67d5c78 Compare November 3, 2026 23:17
@akravchukdremio

Copy link
Copy Markdown
Author

Rebased onto the latest dremio_27.0_23_19 (bc1150c, #150). No conflicts. Gandiva precompiled, internals, and projector tests pass locally.

@lriggs
lriggs merged commit f21f62c into dremio:dremio_27.0_23_19 Nov 4, 2026
19 of 39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants