Expose execution time zone to UDFs #16573

findepi · 2025-06-26T15:55:40Z

The execution time zone serves as the session zone from SQL semantics
perspective. In particular, it influences how a CAST form timestamp to
timestamp with time zone is performed. This is, however, applied at
query parser stage. The execution/session time zone should be also
available to functions in both invoke and simplify APIs.

relates to

Previously the field was optional. However, when set to a None value, it would trigger invalid behavior, with `timestamp with time zone` type being parsed as if it was just `timestamp` type.

The execution time zone serves as the session zone from SQL semantics perspective. In particular, it influences how a CAST form `timestamp` to `timestamp with time zone` is performed. This is, however, applied at query parser stage. The execution/session time zone should be also available to functions in both invoke and simplify APIs.

alamb

Thank you @findepi -- I have a few code level comments, but in general I think this approach to threading the timezone down makes a lot of sense to me

FYI @Omega359 - what do you think of this particular approach?

alamb · 2025-06-26T17:33:54Z

datafusion/core/tests/expr_api/simplification.rs

@@ -133,7 +133,7 @@ fn test_evaluate_with_start_time(
    date_time: &DateTime<Utc>,
 ) {
    let execution_props =
-        ExecutionProps::new().with_query_execution_start_time(*date_time);
+        ExecutionProps::new().with_query_execution_start_time(*date_time, "UTC".into());


this uses "UTC" but the default timezone in session config is "+0:00"

Is this discrepancy intended?

UTC is used in other tests as well

i thought that "+0:00" and "UTC" is the same thing

alamb · 2025-06-26T17:35:18Z

datafusion/expr/src/execution_props.rs

@@ -61,15 +65,18 @@ impl ExecutionProps {
    pub fn with_query_execution_start_time(


I think it is strange to have a function named with_query_execution_start_time that sets both start time and timezone

Would it be possible to make a second separate function for setting the timezone?

alamb · 2025-06-26T17:35:48Z

datafusion/expr/src/execution_props.rs

    ) -> Self {
        self.query_execution_start_time = query_execution_start_time;
+        self.query_execution_time_zone = query_execution_time_zone;
        self
    }

    /// Marks the execution of query started timestamp.
    /// This also instantiates a new alias generator.


I think the comment could also use some updating

alamb · 2025-06-26T17:37:28Z

datafusion/expr/src/udf.rs

@@ -311,6 +311,8 @@ pub struct ScalarFunctionArgs {
    /// or `return_field_from_args`) when creating the physical expression
    /// from the logical expression
    pub return_field: FieldRef,
+    /// The configured execution time zone (a.k.a. session time zone)
+    pub execution_time_zone: String,


If we add a String here it will require another allocation and copy on each function call for each batch

Would it be possible to make this Arc<str> (more similar to a Java string which is refcounted and so less expensive to pass around?

I though that zones are short strings and short strings are cheap to copy. while Arc clone involves atomic CAS (which i believe is unlike in Java).
I don't have numbers to base this on, and may be totally wrong, but i read the title of the following page https://blog.031410.xyz/blog/arc_str_vs_string_is_it_really_faster

If we care about perf here, then even cheaper would be to pass &str here. Requires adding a lifetime to ScalarFunctionArgs (breaking change), and that's likely the only downside.

Omega359 · 2025-06-26T17:49:13Z

Thank you @findepi -- I have a few code level comments, but in general I think this approach to threading the timezone down makes a lot of sense to me

FYI @Omega359 - what do you think of this particular approach?

I'll be honest. I think it's too limited. For example, you still wouldn't have access to any custom config, var providers, etc.

findepi · 2025-06-27T13:49:27Z

I'll be honest. I think it's too limited. For example, you still wouldn't have access to any custom config, var providers, etc.

I definitely don't need that, maybe because I don't know what good var providers can do for me.

Do we need to know all of the desirable items from the start, or just anticipate we gonna evolve set of what's accessible as the need arises?

we wrap the time zone into a new struct. It will store time zone
- and maybe execution start time for now.
The struct will be passed by reference to ScalarFunctionArgs
- so ScalarFunctionArgs gains a lifetime param, a breaking change
this opens the door for easy exposing more info to scalars, without performance overhead, and without future breaking changes

wdyt?

findepi · 2025-07-01T07:48:25Z

It looks we didn't reach a conclusion here yet.
There is "just a String" option as currently implemented in this PR.
There is "a new struct passed by reference (or Arc)" option as proposed in the comment above.
There is also "add SessionConfig by reference (or Arc)" option (#13519) that I am not very supportive of.

@alamb @Omega359 thoughts?

alamb · 2025-07-02T20:30:37Z

It looks we didn't reach a conclusion here yet. There is "just a String" option as currently implemented in this PR. There is "a new struct passed by reference (or Arc)" option as proposed in the comment above. There is also "add SessionConfig by reference (or Arc)" option (#13519) that I am not very supportive of.

@alamb @Omega359 thoughts?

So it is still my opinion that copying fields from ConfigProperties into ExecutionProps one at a time will over time result in more and more "hair" (aka we'll have some subset of the fields and it won't be clear why some are included and some are not)

For that reason, I still think figuring out some way to pass ConfigOptions via ExecutionProps is the right thing to do long term.

Also as @Omega359 using ConfigOptions aligns us with the asyn udfs as well.

I was wondering if we can changeSessionState to have an Arc<ConfigOptions> and pass that into ExecutionProperties

We could use Arc::make_mut to implement "copy on write" semantics.

Let me see if I can whip up a demo

findepi · 2025-07-03T07:19:03Z

So it is still my opinion that copying fields from ConfigProperties into ExecutionProps one at a time will over time result in more and more "hair" (aka we'll have some subset of the fields and it won't be clear why some are included and some are not)

That was definitely my intention -- expose only those fields or state attributes that are relevant. Keep the API surface as small as only sufficient.
(I need blah, so i pass-through blah is a trivial change to make by anyone who needs blah in the future.)

If that's controversial, or problematic, then sure we can expose all the config options like in #16661

This was referenced Jun 26, 2025

Make from_unixtime aware of execution timezone #12892

Open

Add SessionConfig reference to ScalarFunctionArgs #13519

Open

findepi added 2 commits June 26, 2025 17:59

Fix type of ExecutionOptions::time_zone

a65c21e

Previously the field was optional. However, when set to a None value, it would trigger invalid behavior, with `timestamp with time zone` type being parsed as if it was just `timestamp` type.

findepi force-pushed the findepi/expose-zone branch from 7a3c90e to 378bf35 Compare June 26, 2025 16:06

github-actions bot added the physical-plan Changes to the physical-plan crate label Jun 26, 2025

findepi mentioned this pull request Jun 26, 2025

Fix type of ExecutionOptions::time_zone #16569

Merged

alamb reviewed Jun 26, 2025

View reviewed changes

alamb mentioned this pull request Jul 2, 2025

POC: Add ConfigOptions to ExecutionProps when execution is started #16661

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Expose execution time zone to UDFs #16573

Expose execution time zone to UDFs #16573

findepi commented Jun 26, 2025 •

edited

Loading

Uh oh!

alamb left a comment

Uh oh!

alamb Jun 26, 2025

Uh oh!

findepi Jul 3, 2025

Uh oh!

alamb Jun 26, 2025

Uh oh!

alamb Jun 26, 2025

Uh oh!

alamb Jun 26, 2025

Uh oh!

findepi Jun 27, 2025

Uh oh!

Omega359 commented Jun 26, 2025

Uh oh!

findepi commented Jun 27, 2025

Uh oh!

findepi commented Jul 1, 2025

Uh oh!

alamb commented Jul 2, 2025

Uh oh!

findepi commented Jul 3, 2025

Uh oh!

Uh oh!

		@@ -61,15 +65,18 @@ impl ExecutionProps {
		pub fn with_query_execution_start_time(

Expose execution time zone to UDFs #16573

Are you sure you want to change the base?

Expose execution time zone to UDFs #16573

Conversation

findepi commented Jun 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

alamb Jun 26, 2025

Choose a reason for hiding this comment

Uh oh!

findepi Jul 3, 2025

Choose a reason for hiding this comment

Uh oh!

alamb Jun 26, 2025

Choose a reason for hiding this comment

Uh oh!

alamb Jun 26, 2025

Choose a reason for hiding this comment

Uh oh!

alamb Jun 26, 2025

Choose a reason for hiding this comment

Uh oh!

findepi Jun 27, 2025

Choose a reason for hiding this comment

Uh oh!

Omega359 commented Jun 26, 2025

Uh oh!

findepi commented Jun 27, 2025

Uh oh!

findepi commented Jul 1, 2025

Uh oh!

alamb commented Jul 2, 2025

Uh oh!

findepi commented Jul 3, 2025

Uh oh!

Uh oh!

findepi commented Jun 26, 2025 •

edited

Loading