Skip to content

Allow Date + Time = DateTime#102421

Merged
nihalzp merged 10 commits intoClickHouse:masterfrom
nihalzp:date-plus-time
Apr 11, 2026
Merged

Allow Date + Time = DateTime#102421
nihalzp merged 10 commits intoClickHouse:masterfrom
nihalzp:date-plus-time

Conversation

@nihalzp
Copy link
Copy Markdown
Member

@nihalzp nihalzp commented Apr 10, 2026

SET use_legacy_to_time = 0;
SELECT toDate('2024-07-15') + toTime('14:30:25') AS dt, toTypeName(dt);
┌──────────────────dt─┬─toTypeName(dt)─┐
│ 2024-07-15 14:30:25 │ DateTime       │
└─────────────────────┴────────────────┘
SELECT toDate('2024-07-15') + toTime64('14:30:25.123456', 6) AS dt, toTypeName(dt);
┌─────────────────────────dt─┬─toTypeName(dt)─┐
│ 2024-07-15 14:30:25.123456 │ DateTime64(6)  │
└────────────────────────────┴────────────────┘

Changelog category (leave one):

  • New Feature

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

Date and Date32 values can now be added to Time and Time64 values using the + operator, producing a DateTime or DateTime64 result. For example, SELECT toDate('2024-01-15') + toTime('14:30:25') returns 2024-01-15 14:30:25. The result is computed in the session timezone, and out-of-range results are handled according to the date_time_overflow_behavior setting. Closes #95914.

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh Bot commented Apr 10, 2026

Workflow [PR], commit [46600b2]

Summary:


AI Review

Summary

This PR adds Date/Date32 + Time/Time64 support in plus, returning DateTime/DateTime64 with overflow behavior controlled by date_time_overflow_behavior, plus comprehensive docs and stateless coverage. Functionality and edge-case tests look strong. I found one major compile-time concern already reported inline by clickhouse-gh[bot]: a heavy include was added to a very high-fan-out header.

Findings

⚠️ Majors

  • [src/Functions/FunctionBinaryArithmetic.h:53] Interpreters/Context.h was added into a hot, widely included header path. This increases transitive rebuild scope and compile time across many translation units. Keep only lightweight declarations in the header and move settings/context-dependent logic behind a .cpp boundary.
ClickHouse Rules
Item Status Notes
Deletion logging
Serialization versioning
Core-area scrutiny
No test removal
Experimental gate
No magic constants
Backward compatibility
SettingsChangesHistory.cpp
PR metadata quality
Safe rollout
Compilation time ⚠️ High-fan-out header gained heavy interpreter dependency (Interpreters/Context.h).
No large/binary files
Final Verdict
  • Status: ⚠️ Request changes
  • Minimum required actions:
    • Remove the heavy transitive include dependency from FunctionBinaryArithmetic.h by moving context/settings-dependent implementation details out of the header.

@clickhouse-gh clickhouse-gh Bot added the pr-feature Pull request with new product feature label Apr 10, 2026
Comment thread src/Functions/FunctionBinaryArithmetic.h Outdated
@nihalzp nihalzp requested a review from yariks5s April 10, 2026 20:24
@yariks5s yariks5s self-assigned this Apr 10, 2026
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh Bot commented Apr 10, 2026

LLVM Coverage Report

Metric Baseline Current Δ
Lines 84.00% 84.00% +0.00%
Functions 90.90% 90.90% +0.00%
Branches 76.50% 76.50% +0.00%

Changed lines: 94.55% (208/220) · Uncovered code

Full report · Diff report

Copy link
Copy Markdown
Member

@alexey-milovidov alexey-milovidov left a comment

Choose a reason for hiding this comment

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

Looks good! First I thought about time zones, but as neither Date or Time have them, the only option is to use the default (session) timezone for DateTime.

@alexey-milovidov alexey-milovidov self-assigned this Apr 11, 2026
@nihalzp nihalzp added this pull request to the merge queue Apr 11, 2026
Merged via the queue into ClickHouse:master with commit 093a980 Apr 11, 2026
164 checks passed
@nihalzp nihalzp deleted the date-plus-time branch April 11, 2026 03:38
@robot-clickhouse-ci-2 robot-clickhouse-ci-2 added the pr-synced-to-cloud The PR is synced to the cloud repo label Apr 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-feature Pull request with new product feature pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Function to combine Date and Time into DateTime

4 participants