[SQL] Support for TIME/DATE/TIMESTAMP constructors#6315
Conversation
| | Function | Arguments | Result | Description | Example | | ||
| |----------|-----------|--------|-------------|---------| | ||
| |<a id="time_trunc"></a>`TIME_TRUNC(time, *unit*)` | `*unit*` is a time unit, as described above, between `HOUR` and `SECOND` | `TIME` | Rounds down the time to the specified time unit | `TIME_TRUNC('12:34:56.78', MINUTE)` => `12:34:00` | | ||
| |<a id="time_ceil"></a> `CEIL(time TO *unit*)` | `*unit*` is a time unit between `HOUR` and `MICROSECOND` | `TIME` | It rounds a time upward to the start of the next unit boundary unless the value is already exactly on that boundary. | `CEIL('17:01:00' TO HOUR)` => `18:00:00` | |
There was a problem hiding this comment.
I just ordered these alphabetically
mythical-fred
left a comment
There was a problem hiding this comment.
A few blockers — see inline. Most important: year 9999 is rejected by an exclusive range (off-by-one vs. the documented date range), and MAKE_TIMESTAMP is not documented in datetime.md although the docs checkbox is checked.
| |<a id="time_ceil"></a> `CEIL(time TO *unit*)` | `*unit*` is a time unit between `HOUR` and `MICROSECOND` | `TIME` | It rounds a time upward to the start of the next unit boundary unless the value is already exactly on that boundary. | `CEIL('17:01:00' TO HOUR)` => `18:00:00` | | ||
| |<a id="time_floor"></a>`FLOOR(time TO *unit*)` | `*unit*` is a time unit between `HOUR` and `MICROSECOND` | `TIME` | It rounds a time downward to the start of the next unit boundary unless the value is already exactly on that boundary. | `FLOOR('17:23:59' TO MINUTE)` => `17:23:00` | | ||
| |<a id="time_extract"></a>`EXTRACT(*unit* FROM time)` | `*unit*` is a time unit from `HOUR`, `MINUTE`, `SECOND`, `MILLISECOND` | `BIGINT` | Extracts the specified part | `EXTRACT(HOUR FROM '17:23:59')` => `17` | | ||
| | <a id="make_time"></a>`MAKE_TIME(hour, minute, second)` | Any numeric type; last argument can be fractional | `TIME` | Creates a `TIME` from a hour, minute, second; returns `NULL` if the `TIME` is invalid. | `MAKE_TIME(20, 1, 1)` => `20:01:01` | |
There was a problem hiding this comment.
MAKE_TIMESTAMP is added as a new SQL function but is not documented in this file. The PR checklist has Documentation updated ticked, but only MAKE_DATE and MAKE_TIME rows are present. Please add an entry describing the 6-argument signature, the polymorphic second argument, and the NULL-on-invalid semantics. The pre-existing prose around line 222 about a user-defined MAKE_TIMESTAMP(SECONDS BIGINT) should also be reconciled — that example collides with the new built-in name and will confuse readers.
8284454 to
93dfa17
Compare
|
|
||
| pub(crate) use some_polymorphic_function3; | ||
|
|
||
| // If there exists a function is f_t1_t2_t3(x: T, y: S, z: V) -> Option<U>, this creates |
There was a problem hiding this comment.
| // If there exists a function is f_t1_t2_t3(x: T, y: S, z: V) -> Option<U>, this creates | |
| /// If there exists a function is f_t1_t2_t3(x: T, y: S, z: V) -> Option<U>, this creates | |
| ... |
| // If there exists a function is f_t1_t2_t3(x: T, y: S, z: V) -> Option<U>, this creates | ||
| // seven functions: | ||
| // - f_t1_t2_t3N(x: T, y: S, z: Option<V>) -> Option<U> | ||
| // - f_t1_t2N_t2(x: T, y: Option<S>, z: V) -> Option<U> |
There was a problem hiding this comment.
There may be a universe where you can solve this using some trait with impl for From<Option> and no ugly macro hacks
There was a problem hiding this comment.
It is a little sad that for implementing one SQL function sometimes I need tens of Rust functions. Maybe we are too strongly typed.
mythical-fred
left a comment
There was a problem hiding this comment.
All blockers from my previous review are addressed. Four nits inline — none blocking.
| this.ensureInteger(node, ops, 0); | ||
| this.ensureInteger(node, ops, 1); | ||
| validateArgCount(node, operationName, ops.size(), 3); | ||
| validateArgCount(node, operationName, ops.size(), 3); |
There was a problem hiding this comment.
Nit: duplicate validateArgCount call — line 1713 already does the same check.
| NULL | ||
| (1 row) | ||
|
|
||
| SELECT MAKE_DATE(9999, 12, 31); |
There was a problem hiding this comment.
Nit: duplicate test — MAKE_DATE(9999, 12, 31) is already tested at line 432.
| 01:02:03.5 | ||
| (1 row) | ||
|
|
||
| SELECT MAKE_TIME(1, 2, 35e-1); |
There was a problem hiding this comment.
Nit: duplicate test — MAKE_TIME(1, 2, 35e-1) is already at line 293.
| <td><code>TIMESTAMP</code></td> | ||
| <td>Rounds a timestamp upward to the start of the next unit boundary unless the value is already exactly on that boundary.</td> | ||
| <td><code>CEIL('2024-05-04 17:23:01' TO MINUTE)</code> => <code>2024-05-04 17:24:00</code></td> | ||
| <td>Creates a <code>TIMESTAMP</code> from parts; returns `NULL` if the result is not a valid timestamp.</td> |
There was a problem hiding this comment.
Nit: backtick markdown (NULL) won't render inside an HTML <td>. Use <code>NULL</code> instead.
Signed-off-by: Mihai Budiu <mbudiu@feldera.com>
Adds support for 3 Spark functions
MAKE_TIMEMAKE_DATEMAKE_TIMESTAMPThese functions are very difficult to implement otherwise (they require creating a string and parsing it), so I decided to build them as primitives. felderize flagged them as missing. See the tests and the documentation for a precise definition.
Two of these functions are polymorphic, accepting integers, decimals, and float for expressing fractional seconds; our Rust implementation is not polymorphic, so supporting all variants requires implementing multiple Rust functions, depending on argument types.
Checklist