Skip to content

Conversation

@Omega359
Copy link
Contributor

@Omega359 Omega359 commented Oct 29, 2025

Which issue does this PR close?

Rationale for this change

Default timezone was previously zulu however with the recent change to support default tz in now(), current_date(), etc which used to have no default tz the choice was made to unset the system wide timezone.

What changes are included in this PR?

Code, tests, upgrading doc.

Are these changes tested?

Yes, with existing tests.

Are there any user-facing changes?

Yes. Any query that used to use the default timezone would return a timestamp with a timezone of 'Z' will now return a timestamp without a timezone. This can be changed back to the previous behaviour with the sql

SET TIMEZONE = '+00:00';

@github-actions github-actions bot added documentation Improvements or additions to documentation sql SQL Planner core Core DataFusion crate sqllogictest SQL Logic Tests (.slt) common Related to common crate functions Changes to functions implementation labels Oct 29, 2025
@Omega359 Omega359 marked this pull request as ready for review October 29, 2025 18:06
@alamb alamb changed the title Default time_zone config to None Change default time_zone to None (was "+00:00") Oct 29, 2025
Copy link
Member

@Weijun-H Weijun-H left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks @Omega359 👍

| datafusion.execution.collect_statistics | true | Should DataFusion collect statistics when first creating a table. Has no effect after the table is created. Applies to the default `ListingTableProvider` in DataFusion. Defaults to true. |
| datafusion.execution.target_partitions | 0 | Number of partitions for query execution. Increasing partitions can increase concurrency. Defaults to the number of CPU cores on the system |
| datafusion.execution.time_zone | +00:00 | The default time zone Some functions, e.g. `EXTRACT(HOUR from SOME_TIME)`, shift the underlying datetime according to this time zone, and then extract the hour |
| datafusion.execution.time_zone | NULL | The default time zone Some functions, e.g. `now` return timestamps in this time zone |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
| datafusion.execution.time_zone | NULL | The default time zone Some functions, e.g. `now` return timestamps in this time zone |
| datafusion.execution.time_zone | NULL | The default time zone. Some functions, e.g. `now` return timestamps in this time zone. If set to NULL, no time zone is applied. |

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently that isn't supported:

1. statement failed: DataFusion error: Error during planning: Unsupported value Null
[SQL] SET TIMEZONE = NULL
at /opt/dev/datafusion/datafusion/sqllogictest/test_files/set_variable.slt:176

An empty string works though the returned value is (empty) not null.

I'll take a look at what it'll take to support null.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Weijun-H Supporting un-setting a config option will be more than just a small change and I think that should be done in a separate PR. Right now it will cause a plan_err to be returned.

Currently the SetVariable struct requires a string, changing that to an option will require threading that change throughout the codebase + additional testing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed as #18384

@alamb
Copy link
Contributor

alamb commented Oct 31, 2025

I merged up to resolve a conflict

@Omega359
Copy link
Contributor Author

Omega359 commented Nov 2, 2025

I'll merge up to fix the conflict tonight or tomorrow.

@alamb alamb added this pull request to the merge queue Nov 3, 2025
@alamb
Copy link
Contributor

alamb commented Nov 3, 2025

Thank you @Omega359

Merged via the queue into apache:main with commit 9ea67f5 Nov 3, 2025
29 checks passed
@alamb alamb mentioned this pull request Nov 3, 2025
33 tasks
@Omega359 Omega359 deleted the feature/default_time_zone branch November 3, 2025 22:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

common Related to common crate core Core DataFusion crate documentation Improvements or additions to documentation functions Changes to functions implementation sql SQL Planner sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NowFunc::new() seeds the timezone with "+00" while the canonical UTC offset remains "+00:00" in ConfigOptions::default().

3 participants