feat(functions): add DATE_FROM_PARTS, TIMESTAMP_FROM_PARTS and TIMESTAMP_TZ_FROM_PARTS#19508
feat(functions): add DATE_FROM_PARTS, TIMESTAMP_FROM_PARTS and TIMESTAMP_TZ_FROM_PARTS#19508tharunn0 wants to merge 6 commits intodatabendlabs:mainfrom
Conversation
|
Hi @tharunn0 |
tests/sqllogictests/suites/query/functions/02_0012_function_datetimes.test
Outdated
Show resolved
Hide resolved
…amp_tz_from_parts - Implement DATE_FROM_PARTS, TIMESTAMP_FROM_PARTS and TIMESTAMP_TZ_FROM_PARTS - Handle overflow for month/day/time components - Move normalization helpers to date_helper.rs - Use Int64 arguments for consistency with timestamp functions - Support optional nanoseconds and timezone arguments - Add sqllogictests for timezone, nanoseconds and overflow cases fix: use SignedDuration::from_hours instead of from_days
641e9f6 to
d817e95
Compare
tests/sqllogictests/suites/query/functions/02_0012_function_datetimes.test
Show resolved
Hide resolved
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2cf15e3498
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
2cf15e3 to
329ab95
Compare
| let total_months = year | ||
| .checked_mul(12) | ||
| .and_then(|y| y.checked_add(month - 1)) |
There was a problem hiding this comment.
normalize_date_parts still does unchecked arithmetic on user inputs via month - 1 (and later day - 1) before the surrounding
checked_* calls run. Inputs near the INT64 bounds, such as date_from_parts(1, -9223372036854775808, 1), will therefore panic
in the repo's dev/test profiles and wrap to a bogus value in release builds instead of returning a normal query error; this
also leaks into timestamp_from_parts/timestamp_tz_from_parts because they reuse the same helper.
| let total_seconds = hour | ||
| .checked_mul(3600) | ||
| .and_then(|h| h.checked_add(minute * 60)) | ||
| .and_then(|m| m.checked_add(second)) | ||
| .and_then(|s| s.checked_add(extra_secs_from_nanos)) |
There was a problem hiding this comment.
The overflow handling in normalize_timestamp_micros is incomplete because minute * 60 is evaluated before it enters the
checked_add chain. Large INT64 minute values will panic in checked builds and wrap in release builds, so timestamp_from_parts/
timestamp_tz_from_parts can return an incorrect timestamp or crash instead of surfacing BadArguments for out-of-range
components.
| /// Normalize month/day values that may be outside normal ranges. | ||
| /// Supports values like month=0 (meaning Dec of previous year), | ||
| /// month=13 (meaning Jan of next year), and day=100 (100th day from month start). | ||
| pub fn normalize_date_parts(year: i64, month: i64, day: i64) -> std::result::Result<Date, String> { |
There was a problem hiding this comment.
Why not return databend_common_exception::Result? Can return Errorcode::BadArgument
| .ok_or_else(|| format!("Day value out of bounds: {day}"))?; | ||
|
|
||
| let result = base | ||
| .checked_add(SignedDuration::from_hours(hours_to_add)) |
There was a problem hiding this comment.
Why not use checked_add(SignedDuration::from_days(day - 1))? And please use checked day-1
| (ts_values, offset_values, len) | ||
| } | ||
|
|
||
| fn timestamp_from_parts_fn( |
There was a problem hiding this comment.
Why not call clamp_timestamp check timestamp?
| let base = Date::new(norm_year, norm_month, 1) | ||
| .map_err(|_| format!("Invalid date: year={year}, month={month}, day={day}"))?; | ||
|
|
||
| // 2. Safe day normalization (Fixing the second bot flag) |
There was a problem hiding this comment.
need to delete AI comment.
|
|
||
| # Negative nanoseconds | ||
| query ? | ||
| select timestamp_tz_from_parts(2023, 1, 1, 12, 0, 0, -1000000000, 'UTC'); |
There was a problem hiding this comment.
Please add test like this:
-- date_from_parts with table data and NULLs
statement ok
CREATE TABLE t_date_parts(year INT64 NULL, month INT64 NULL, day INT64 NULL)
statement ok
INSERT INTO t_date_parts VALUES (2023, 6, 15), (2010, 1, 100), (NULL, 6, 1), (2023, NULL, 1), (2023, 6, NULL)
query ?
SELECT date_from_parts(year, month, day) FROM t_date_parts ORDER BY year, month, day
----
NULL
NULL
2010-04-10
2023-06-15
-- timestamp_from_parts with table data and NULLs
statement ok
CREATE TABLE t_ts_parts(year INT64 NULL, month INT64 NULL, day INT64 NULL, hour INT64 NULL, minute INT64 NULL, second INT64
NULL)
statement ok
INSERT INTO t_ts_parts VALUES (2013, 4, 5, 12, 0, 0), (NULL, 4, 5, 12, 0, 0), (2013, 4, 5, NULL, 0, 0)
query ?
SELECT timestamp_from_parts(year, month, day, hour, minute, second) FROM t_ts_parts ORDER BY year, month
----
NULL
NULL
2013-04-05 12:00:00.000000
-- timestamp_tz_from_parts with table data and NULLs
statement ok
CREATE TABLE t_tstz_parts(year INT64 NULL, month INT64 NULL, day INT64 NULL, hour INT64 NULL, minute INT64 NULL, second INT64
NULL, tz VARCHAR NULL)
statement ok
INSERT INTO t_tstz_parts VALUES (2023, 1, 1, 12, 0, 0, 'UTC'), (2023, 7, 1, 12, 0, 0, 'Asia/Shanghai'), (NULL, 1, 1, 12, 0, 0,
'UTC'), (2023, 1, 1, 12, 0, 0, NULL)
query ?
SELECT timestamp_tz_from_parts(year, month, day, hour, minute, second, tz) FROM t_tstz_parts ORDER BY year, tz
----
NULL
NULL
2023-01-01 12:00:00.000000 +0000
2023-07-01 12:00:00.000000 +0800
statement ok
DROP TABLE t_date_parts
statement ok
DROP TABLE t_ts_parts
statement ok
DROP TABLE t_tstz_parts
I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/
Summary
This PR introduces support for the following datetime construction functions:
These functions construct date and timestamp values from individual numeric components.
Integration tests and sqllogictests were added to verify correctness.
Fixes #19435
Type of change
Tests
This change is