Skip to content

Commit 7fbd000

Browse files
rampage644claude
andauthored
Fix timezone handling in to_timestamp functions (#136)
* Fix TIMESTAMP_NTZ cast not stripping timezone from timestamp_tz input to_timestamp_ntz had early-return paths in return_field_from_args and invoke_with_args that preserved the input timestamp type as-is, including its timezone. This caused convert_timezone(...)::TIMESTAMP_NTZ to return timestamp_tz instead of timestamp_ntz. Now both paths apply the function's timezone policy via self.timezone(), which returns None for NTZ variants. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(clippy): use Copy deref instead of clone on TimeUnit TimeUnit implements Copy, so unit.clone() triggers clippy::clone_on_copy (denied via clippy::all in workspace lints). Use *unit to satisfy the lint. https://claude.ai/code/session_01YUH8HvzyjxmHtjrNcALJXj --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent ebc8a17 commit 7fbd000

3 files changed

Lines changed: 26 additions & 8 deletions

File tree

crates/executor/src/tests/sql/functions/timestamp/snapshots/to_timestamp/query_timestamp_with_timezone_to_timestamp.snap

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@ description: "\"SELECT TO_TIMESTAMP(CONVERT_TIMEZONE('UTC', '2024-12-31 10:00:00
44
---
55
Ok(
66
[
7-
"+----------------------+",
8-
"| model_tstamp |",
9-
"+----------------------+",
10-
"| 2024-12-31T10:00:00Z |",
11-
"+----------------------+",
7+
"+---------------------+",
8+
"| model_tstamp |",
9+
"+---------------------+",
10+
"| 2024-12-31T10:00:00 |",
11+
"+---------------------+",
1212
],
1313
)

crates/executor/src/utils.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -950,7 +950,7 @@ mod tests {
950950
(
951951
TimeUnit::Nanosecond,
952952
Some(1_627_846_261_233_222_111),
953-
"1627846261.233222111 1440", // 1020-1440 = -420 (PDT)
953+
"1627846261.233222111 1440",
954954
Some("UTC".to_string()),
955955
(1_627_846_261, 233_222_111, 1_627_846_261_233_222_111),
956956
),

crates/functions/src/conversion/to_timestamp.rs

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -204,15 +204,19 @@ impl ScalarUDFImpl for ToTimestampFunc {
204204
}
205205
}
206206

207-
// If the first argument is already a timestamp type, return it as is (with its timezone).
207+
// If the first argument is already a timestamp type, preserve the unit but
208+
// apply this function's timezone policy (e.g. to_timestamp_ntz strips timezone).
208209
if matches!(
209210
args.arg_fields[0].data_type(),
210211
DataType::Timestamp(TimeUnit::Microsecond, _)
211212
| DataType::Timestamp(TimeUnit::Nanosecond, Some(_))
212213
) {
214+
let DataType::Timestamp(unit, _) = args.arg_fields[0].data_type() else {
215+
unreachable!()
216+
};
213217
return Ok(Arc::new(Field::new(
214218
self.name(),
215-
args.arg_fields[0].data_type().clone(),
219+
DataType::Timestamp(*unit, self.timezone()),
216220
true,
217221
)));
218222
}
@@ -317,6 +321,20 @@ impl ScalarUDFImpl for ToTimestampFunc {
317321
DataType::Timestamp(TimeUnit::Microsecond, _)
318322
| DataType::Timestamp(TimeUnit::Nanosecond, Some(_))
319323
) {
324+
// If the input timezone differs from this function's target timezone
325+
// (e.g. to_timestamp_ntz needs to strip tz), cast to apply the policy.
326+
let DataType::Timestamp(unit, ref tz) = arr.data_type().clone() else {
327+
unreachable!()
328+
};
329+
let target_tz = self.timezone();
330+
if *tz != target_tz {
331+
let arr = cast_with_options(
332+
&arr,
333+
&DataType::Timestamp(unit, target_tz),
334+
&DEFAULT_CAST_OPTIONS,
335+
)?;
336+
return Ok(ColumnarValue::Array(arr));
337+
}
320338
return Ok(ColumnarValue::Array(Arc::new(arr)));
321339
}
322340

0 commit comments

Comments
 (0)