-
Notifications
You must be signed in to change notification settings - Fork 742
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(query): function about convert_timezone #16177 #16181
base: main
Are you sure you want to change the base?
Conversation
Hi @florann |
Hello ! @b41sh, I correct all the points you previously mentioned. :^) I'll try to watch a bit in depth how unit test configuration is working otherwise I'll ask for your help |
Okay, so I really don't get how I need to set up my unit test. |
@@ -108,6 +109,9 @@ pub fn register(registry: &mut FunctionRegistry) { | |||
|
|||
// [date | timestamp] +/- number | |||
register_timestamp_add_sub(registry); | |||
|
|||
// convert_timezone( target_timezone, date, src_timezone) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The arguments are in the wrong order. They should be src_timezone, target_timezone, source_timestamp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes.
Need support
CONVERT_TIMEZONE( <source_tz> , <target_tz> , <source_timestamp_ntz> )
and
CONVERT_TIMEZONE( <target_tz> , <source_timestamp> )
eval_convert_timezone, | ||
); | ||
|
||
// 3 arguments function [target_timezone, src_timestamp, src_timezone] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
"convert_timezone", | ||
|_, _, _, _| FunctionDomain::MayThrow, | ||
vectorize_with_builder_3_arg::<StringType, StringType, TimestampType, TimestampType>( | ||
|src_tz, target_tz, src_timestamp, output, ctx| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check validity here to handle NULL
value
if let Some(validity) = &ctx.validity {
if !validity.get_bit(output.len()) {
output.push_default();
return;
}
}
|
||
|
||
// Create dummy timezone | ||
let utc_now: DateTime<Utc> = Utc::now(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think no need to create dummy timezone. You can use this https://docs.rs/chrono/latest/chrono/offset/trait.TimeZone.html#method.timestamp_opt to convert ts to source timezone.
Then convert the ts with src tz to target tz.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
// Convert the UTC time to the specified target timezone | ||
let target_datetime: DateTime<Tz> = datetime.with_timezone(&t_tz); | ||
let result_timestamp = target_datetime.timestamp(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
timestamp_micros()
test_convert_timezone(file); | ||
} | ||
|
||
fn test_convert_timezone(file: &mut impl Write) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool.
SELECT CONVERT_TIMEZONE(
'America/Los_Angeles',
'America/New_York',
to_timestamp('2024-01-01 14:00:00')
) AS conv;
SELECT CONVERT_TIMEZONE(
'Europe/Warsaw',
'UTC',
'2024-01-01 00:00:00'::TIMESTAMP
) AS conv;
SELECT CONVERT_TIMEZONE(
'America/Los_Angeles',
'2024-04-05 12:00:00 +02:00'::TIMESTAMP
) AS time_in_la;
SELECT
CURRENT_TIMESTAMP() AS now_in_la,
CONVERT_TIMEZONE('America/New_York', CURRENT_TIMESTAMP()) AS now_in_nyc,
CONVERT_TIMEZONE('Europe/Paris', CURRENT_TIMESTAMP()) AS now_in_paris,
CONVERT_TIMEZONE('Asia/Tokyo', CURRENT_TIMESTAMP()) AS now_in_tokyo;
Need to add these tests in logical test.
I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/
Summary
The goal was to add a function that allow to convert a timestamp, with a timezone or not, to a specific timezone
Tests
Type of change
This change is