You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datafusion.apache.org by gi...@apache.org on 2026/10/03 03:22:38 UTC
(datafusion) 01/01: fix: Validate `execution.time_zone` at `SET` time (#25788)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-25788-801b017bb8824e577593ac6deefad6a425cc6ac3
in repository https://gitbox.apache.org/repos/asf/datafusion.git
commit b550f59884c47cb03675295d3ef9f35159afc7d5
Author: Kassahun Sanayew <87...@users.noreply.github.com>
AuthorDate: Sat Oct 3 03:22:13 2026 +0000
fix: Validate `execution.time_zone` at `SET` time (#25788)
## Which issue does this PR close?
- Part of #17498. This covers `datafusion.execution.time_zone` only.
## Rationale for this change
`datafusion.execution.time_zone` is stored as a plain string, so an
invalid value such as `SET TIME ZONE = 'Asia/Taipei2'` (or `'08:00'`,
`'+08:00:00'`) is accepted and shown by `SHOW TIME ZONE`. The error only
appears later, on the first `::TIMESTAMPTZ` cast. Worse,
`current_date()` and `current_time()` parse the value with `.ok()`, so
they silently ignore a bad time zone and return results as if none was
set.
PostgreSQL and Spark (`spark.sql.session.timeZone` is checked with
`isValidTimezone` in `SQLConf`) both reject an invalid time zone when it
is set.
This picks up #23224 by @Probablism, which was closed as stale, and
answers its open review questions:
- **Normalization** (the `timestamps.slt` question): #23224 stored a
parsed `Tz`, so `+08` came back as `+08:00` and `arrow_typeof(now())`
changed. Here the value is checked with `Tz::from_str` but stored
exactly as written, so the only change users see is that invalid values
are rejected. `timestamps.slt` is unchanged, and `set_variable.slt` now
checks that `SHOW TIME ZONE` returns `+08`.
- **`AEST` in tests**: `AEST` was never a valid Arrow time zone. The
tests that used it only passed the string through without parsing it, so
they now use `Australia/Sydney`.
## What changes are included in this PR?
- Add `ConfigTimeZone` in `datafusion/common/src/config.rs`. It can only
be built through `FromStr`, which checks the value with Arrow's `Tz`
parser and keeps the original string. It provides `as_str()` and
`Display`.
- Change `ExecutionOptions::time_zone` from `Option<String>` to
`Option<ConfigTimeZone>`. Its `ConfigField` impl parses before
assigning, following `DFParquetStatistics`, so a rejected value leaves
the previous value (or `None`) in place, and `RESET` restores `None`.
- Update the readers (SQL planner, `now`, `current_date`,
`current_time`, `to_timestamp*`, `from_unixtime`, Spark `cast` /
`date_trunc`) to use `as_str()`.
- Remove `to_timestamp_invalid_execution_timezone_behavior` and
`to_timestamp_formats_invalid_execution_timezone_behavior`, along with
their now-unused helper. They put an invalid string straight into the
config to test the error at invoke time, and that state can no longer be
constructed. The unit test and slt cases below cover the rejection
instead.
## What is the testing strategy for this PR?
- `set_variable.slt`: the invalid values `+08:00:00`, `08:00`, `08` and
`Asia/Taipei2` now fail on `SET` (through both `SET TIME ZONE` and `SET
datafusion.execution.time_zone`), and `SHOW TIME ZONE` confirms the
previous value is kept. It also checks that `+08` is shown as written.
- `test_execution_time_zone_validation` in `datafusion-common`: valid
values round-trip exactly; invalid values error and keep the previous
value; an invalid update of an unset value leaves it unset; nested keys
are rejected.
- Existing time zone suites pass unchanged (`timestamps.slt`,
`timestamps_timezone.slt`, `current_*_timezone.slt`,
`to_timestamp_timezone.slt`, `from_unixtime_timezone.slt`, Spark
cast/date_trunc).
Commands run (all passing):
- `cargo test -p datafusion-common --lib config::tests` (with and
without `--features parquet`)
- `cargo test --profile=ci --test sqllogictests` (all 523 files)
- `cargo test -p datafusion-functions --lib datetime`
- `cargo test -p datafusion-spark --lib function::`
- `cargo test -p datafusion-sql --lib`
- `cargo test -p datafusion --test user_defined_integration
config_options`
- `cargo test -p datafusion-ffi --features integration-tests --test
ffi_udf`
- `cargo test -p datafusion --test parquet_integration`
- `cargo fmt --all -- --check`
- `cargo clippy --all-targets --all-features -- -D warnings`
## Are there any user-facing changes?
Yes. `SET TIME ZONE` / `SET datafusion.execution.time_zone` now fails
immediately on an invalid time zone instead of failing on a later query,
or being silently ignored by `current_date()` / `current_time()`.
This changes the public field type `ExecutionOptions::time_zone` from
`Option<String>` to `Option<ConfigTimeZone>`. Code that reads it can use
`.as_ref().map(ConfigTimeZone::as_str)`, and code that sets it can use
`Some("UTC".parse()?)`. Could a maintainer please add the `api change`
label? I'm happy to add a note to the 56.0.0 upgrade guide if that's
wanted.
cc @Jefffrey (you reviewed #23224), @kumarUjjawal @alamb (reviewed the
other #17498 slices)
---
datafusion/common/src/config.rs | 116 ++++++++++++++++-
.../user_defined/user_defined_scalar_functions.rs | 11 +-
datafusion/ffi/src/tests/udf_udaf_udwf.rs | 7 +-
datafusion/ffi/tests/ffi_udf.rs | 7 +-
.../benches/datetime_expressions/to_timestamp.rs | 2 +-
datafusion/functions/src/datetime/current_date.rs | 2 +-
datafusion/functions/src/datetime/current_time.rs | 4 +-
datafusion/functions/src/datetime/from_unixtime.rs | 6 +-
datafusion/functions/src/datetime/to_timestamp.rs | 137 +--------------------
datafusion/physical-expr/src/scalar_function.rs | 2 +-
datafusion/spark/src/function/conversion/cast.rs | 4 +-
datafusion/sql/src/planner.rs | 7 +-
.../sqllogictest/test_files/set_variable.slt | 37 +++---
docs/source/library-user-guide/upgrading/56.0.0.md | 25 ++++
14 files changed, 198 insertions(+), 169 deletions(-)
diff --git a/datafusion/common/src/config.rs b/datafusion/common/src/config.rs
index 4246dcc451..1e23ec894c 100644
--- a/datafusion/common/src/config.rs
+++ b/datafusion/common/src/config.rs
@@ -943,7 +943,7 @@ config_namespace! {
/// The default time zone
///
/// Some functions, e.g. `now` return timestamps in this time zone
- pub time_zone: Option<String>, default = None
+ pub time_zone: Option<ConfigTimeZone>, default = None
/// Parquet options
pub parquet: ParquetOptions, default = Default::default()
@@ -2013,6 +2013,76 @@ impl From<ConfigDurationFormat> for arrow::util::display::DurationFormat {
}
}
+/// A time zone that is known to parse as an Arrow
+/// [`Tz`](arrow::array::timezone::Tz), used for
+/// [`ExecutionOptions::time_zone`].
+///
+/// The value is kept as it was written, so `+08` stays `+08` rather than
+/// being normalized to `+08:00`.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct ConfigTimeZone(String);
+
+impl ConfigTimeZone {
+ /// Returns the time zone as it was written.
+ pub fn as_str(&self) -> &str {
+ &self.0
+ }
+}
+
+impl FromStr for ConfigTimeZone {
+ type Err = DataFusionError;
+
+ fn from_str(s: &str) -> Result<Self, Self::Err> {
+ match s.parse::<arrow::array::timezone::Tz>() {
+ Ok(_) => Ok(Self(s.to_string())),
+ Err(_) => _config_err!(
+ "Invalid time zone: {s}. Valid values are UTC offsets such as +08:00 or IANA time zone names such as Asia/Taipei"
+ ),
+ }
+ }
+}
+
+impl Display for ConfigTimeZone {
+ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+ f.write_str(&self.0)
+ }
+}
+
+/// `ConfigField` for `Option<ConfigTimeZone>` parses before assigning so an
+/// invalid value leaves the current setting unchanged.
+impl ConfigField for Option<ConfigTimeZone> {
+ fn visit<V: Visit>(&self, v: &mut V, key: &str, description: &'static str) {
+ match self {
+ Some(time_zone) => v.some(key, time_zone, description),
+ None => v.none(key, description),
+ }
+ }
+
+ fn set(&mut self, key: &str, value: &str) -> Result<()> {
+ if !key.is_empty() {
+ return _config_err!(
+ "Config field is a scalar Option<ConfigTimeZone> and does not have nested field \"{}\"",
+ key
+ );
+ }
+
+ *self = Some(ConfigTimeZone::from_str(value)?);
+ Ok(())
+ }
+
+ fn reset(&mut self, key: &str) -> Result<()> {
+ if key.is_empty() {
+ *self = None;
+ Ok(())
+ } else {
+ _config_err!(
+ "Config field is a scalar Option<ConfigTimeZone> and does not have nested field \"{}\"",
+ key
+ )
+ }
+ }
+}
+
config_namespace! {
/// Options controlling the format of output when printing record batches
/// Copies [`arrow::util::display::FormatOptions`]
@@ -4061,7 +4131,6 @@ impl Display for OutputFormat {
#[cfg(test)]
mod tests {
- #[cfg(feature = "parquet")]
use crate::assert_contains;
use crate::config::TableParquetOptions;
use crate::config::{
@@ -4694,6 +4763,49 @@ mod tests {
assert_eq!(scalar, DFParquetStatistics::Page);
}
+ #[test]
+ fn test_execution_time_zone_validation() {
+ use crate::config::ConfigOptions;
+
+ const KEY: &str = "datafusion.execution.time_zone";
+ let mut config = ConfigOptions::default();
+ assert_eq!(config.execution.time_zone, None);
+
+ // Valid values are kept exactly as written.
+ for value in ["+08:00", "-08:00", "+0800", "+08", "UTC", "Asia/Taipei"] {
+ config.set(KEY, value).unwrap();
+ assert_eq!(
+ config.execution.time_zone.as_ref().map(|tz| tz.as_str()),
+ Some(value)
+ );
+ }
+
+ // A rejected value leaves the previous one in place.
+ for value in ["+08:00:00", "08:00", "08", "Asia/Taipei2", "AEST", ""] {
+ let err = config.set(KEY, value).unwrap_err();
+ assert_contains!(err.to_string(), format!("Invalid time zone: {value}."));
+ assert_eq!(
+ config.execution.time_zone.as_ref().map(|tz| tz.as_str()),
+ Some("Asia/Taipei")
+ );
+ }
+
+ // An invalid update of an unset value leaves it unset.
+ config.reset(KEY).unwrap();
+ assert_eq!(config.execution.time_zone, None);
+ assert!(config.set(KEY, "Asia/Taipei2").is_err());
+ assert_eq!(config.execution.time_zone, None);
+
+ // The option has no nested fields.
+ config.set(KEY, "UTC").unwrap();
+ assert!(config.set(&format!("{KEY}.typo"), "UTC").is_err());
+ assert!(config.reset(&format!("{KEY}.typo")).is_err());
+ assert_eq!(
+ config.execution.time_zone.as_ref().map(|tz| tz.as_str()),
+ Some("UTC")
+ );
+ }
+
#[cfg(feature = "parquet")]
#[test]
fn test_parquet_compression_validation() {
diff --git a/datafusion/core/tests/user_defined/user_defined_scalar_functions.rs b/datafusion/core/tests/user_defined/user_defined_scalar_functions.rs
index ef2aac0743..0ec5642fce 100644
--- a/datafusion/core/tests/user_defined/user_defined_scalar_functions.rs
+++ b/datafusion/core/tests/user_defined/user_defined_scalar_functions.rs
@@ -1965,7 +1965,12 @@ async fn test_config_options_work_for_scalar_func() -> Result<()> {
}
fn invoke_with_args(&self, args: ScalarFunctionArgs) -> Result<ColumnarValue> {
- let tz = args.config_options.execution.time_zone.clone();
+ let tz = args
+ .config_options
+ .execution
+ .time_zone
+ .as_ref()
+ .map(ToString::to_string);
Ok(ColumnarValue::Scalar(ScalarValue::from(tz)))
}
}
@@ -1975,7 +1980,7 @@ async fn test_config_options_work_for_scalar_func() -> Result<()> {
});
let mut config = SessionConfig::new();
- config.options_mut().execution.time_zone = Some("AEST".into());
+ config.options_mut().execution.time_zone = Some("Australia/Sydney".parse().unwrap());
let ctx = SessionContext::new_with_config(config);
@@ -1988,7 +1993,7 @@ async fn test_config_options_work_for_scalar_func() -> Result<()> {
let expected_schema = Schema::new(vec![Field::new("a", DataType::Utf8, false)]);
let expected = RecordBatch::try_new(
SchemaRef::from(expected_schema),
- vec![create_array!(Utf8, ["AEST"])],
+ vec![create_array!(Utf8, ["Australia/Sydney"])],
)?;
assert_eq!(expected, actual[0]);
diff --git a/datafusion/ffi/src/tests/udf_udaf_udwf.rs b/datafusion/ffi/src/tests/udf_udaf_udwf.rs
index 830c639c74..ea7c03b5d6 100644
--- a/datafusion/ffi/src/tests/udf_udaf_udwf.rs
+++ b/datafusion/ffi/src/tests/udf_udaf_udwf.rs
@@ -102,7 +102,12 @@ impl ScalarUDFImpl for TimeZoneUDF {
&self,
args: ScalarFunctionArgs,
) -> datafusion_common::Result<ColumnarValue> {
- let tz = args.config_options.execution.time_zone.clone();
+ let tz = args
+ .config_options
+ .execution
+ .time_zone
+ .as_ref()
+ .map(ToString::to_string);
Ok(ColumnarValue::Scalar(ScalarValue::from(tz)))
}
diff --git a/datafusion/ffi/tests/ffi_udf.rs b/datafusion/ffi/tests/ffi_udf.rs
index 73fafef134..f97153526f 100644
--- a/datafusion/ffi/tests/ffi_udf.rs
+++ b/datafusion/ffi/tests/ffi_udf.rs
@@ -141,7 +141,8 @@ mod tests {
assert!(result[0].column(0).as_string::<i32>().is_null(0));
let mut config = SessionConfig::new();
- config.options_mut().execution.time_zone = Some("AEST".into());
+ config.options_mut().execution.time_zone =
+ Some("Australia/Sydney".parse().unwrap());
let ctx = SessionContext::new_with_config(config);
@@ -154,7 +155,7 @@ mod tests {
assert_eq!(result.len(), 1);
assert!(!result[0].column(0).as_string::<i32>().is_null(0));
let result = result[0].column(0).as_string::<i32>().value(0);
- assert_eq!(result, "AEST");
+ assert_eq!(result, "Australia/Sydney");
Ok(())
}
@@ -175,7 +176,7 @@ mod tests {
);
let mut options = ConfigOptions::default();
- options.execution.time_zone = Some("AEST".into());
+ options.execution.time_zone = Some("Australia/Sydney".parse().unwrap());
let updated = foreign_udf
.with_updated_config(&options)
diff --git a/datafusion/functions/benches/datetime_expressions/to_timestamp.rs b/datafusion/functions/benches/datetime_expressions/to_timestamp.rs
index 79a9395a6e..aaf6610e1c 100644
--- a/datafusion/functions/benches/datetime_expressions/to_timestamp.rs
+++ b/datafusion/functions/benches/datetime_expressions/to_timestamp.rs
@@ -113,7 +113,7 @@ fn criterion_benchmark(c: &mut Criterion) {
let arg_field = Field::new("a", DataType::Utf8, false).into();
let arg_fields = vec![arg_field];
let mut options = ConfigOptions::default();
- options.execution.time_zone = Some("UTC".into());
+ options.execution.time_zone = Some("UTC".parse().unwrap());
let config_options = Arc::new(options);
let to_timestamp_udf = to_timestamp(config_options.as_ref());
diff --git a/datafusion/functions/src/datetime/current_date.rs b/datafusion/functions/src/datetime/current_date.rs
index e93a64e8cc..10410e7597 100644
--- a/datafusion/functions/src/datetime/current_date.rs
+++ b/datafusion/functions/src/datetime/current_date.rs
@@ -119,7 +119,7 @@ impl ScalarUDFImpl for CurrentDateFunc {
.execution
.time_zone
.as_ref()
- .and_then(|tz| tz.parse::<Tz>().ok())
+ .and_then(|tz| tz.as_str().parse::<Tz>().ok())
.map_or_else(
|| datetime_to_days(&now_ts),
|tz| {
diff --git a/datafusion/functions/src/datetime/current_time.rs b/datafusion/functions/src/datetime/current_time.rs
index 1e202ada1d..75f9a0c8a6 100644
--- a/datafusion/functions/src/datetime/current_time.rs
+++ b/datafusion/functions/src/datetime/current_time.rs
@@ -116,7 +116,7 @@ impl ScalarUDFImpl for CurrentTimeFunc {
.execution
.time_zone
.as_ref()
- .and_then(|tz| tz.parse::<Tz>().ok())
+ .and_then(|tz| tz.as_str().parse::<Tz>().ok())
.map_or_else(
|| datetime_to_time_nanos(&now_ts),
|tz| {
@@ -158,7 +158,7 @@ mod tests {
config.execution.time_zone = if tz.is_empty() {
None
} else {
- Some(tz.to_string())
+ Some(tz.parse().unwrap())
};
let schema = Arc::new(DFSchema::empty());
SimplifyContext::builder()
diff --git a/datafusion/functions/src/datetime/from_unixtime.rs b/datafusion/functions/src/datetime/from_unixtime.rs
index d6aa09fb31..89a7e807e3 100644
--- a/datafusion/functions/src/datetime/from_unixtime.rs
+++ b/datafusion/functions/src/datetime/from_unixtime.rs
@@ -353,7 +353,7 @@ mod test {
timezone: Option<&str>,
) -> Result<ColumnarValue> {
let mut options = ConfigOptions::default();
- options.execution.time_zone = timezone.map(str::to_string);
+ options.execution.time_zone = timezone.map(|tz| tz.parse().unwrap());
let func = FromUnixtimeFunc::new_with_config(&options);
let arg_field: FieldRef = Field::new("a", DataType::Int64, true).into();
@@ -560,7 +560,7 @@ mod test {
#[test]
fn test_session_timezone_is_used_without_explicit_timezone() {
let mut options = ConfigOptions::default();
- options.execution.time_zone = Some("America/Denver".to_string());
+ options.execution.time_zone = Some("America/Denver".parse().unwrap());
let func = FromUnixtimeFunc::new_with_config(&options);
@@ -599,7 +599,7 @@ mod test {
#[test]
fn test_explicit_timezone_overrides_session_timezone() {
let mut options = ConfigOptions::default();
- options.execution.time_zone = Some("America/Denver".to_string());
+ options.execution.time_zone = Some("America/Denver".parse().unwrap());
let func = FromUnixtimeFunc::new_with_config(&options);
diff --git a/datafusion/functions/src/datetime/to_timestamp.rs b/datafusion/functions/src/datetime/to_timestamp.rs
index 57a2634a2a..6720152cbd 100644
--- a/datafusion/functions/src/datetime/to_timestamp.rs
+++ b/datafusion/functions/src/datetime/to_timestamp.rs
@@ -935,30 +935,6 @@ mod tests {
udfs
}
- fn validate_expected_error(
- options: &mut ConfigOptions,
- args: ScalarFunctionArgs,
- expected_err: &str,
- ) {
- let udfs = udfs_and_timeunit();
-
- for (udf, _) in udfs {
- match udf
- .with_updated_config(options)
- .unwrap()
- .invoke_with_args(args.clone())
- {
- Ok(_) => panic!("Expected error but got success"),
- Err(e) => {
- assert!(
- e.to_string().contains(expected_err),
- "Can not find expected error '{expected_err}'. Actual error '{e}'"
- );
- }
- }
- }
- }
-
#[test]
fn to_timestamp_arrays_and_nulls() -> Result<()> {
// ensure that arrow array implementation is wired up and handles nulls correctly
@@ -1067,7 +1043,7 @@ mod tests {
let udfs = udfs_and_timeunit();
let mut options = ConfigOptions::default();
- options.execution.time_zone = Some("-05:00".to_string());
+ options.execution.time_zone = Some("-05:00".parse().unwrap());
let time_zone: Option<Arc<str>> = options
.execution
@@ -1178,7 +1154,7 @@ mod tests {
#[test]
fn to_timestamp_array_respects_execution_timezone() -> Result<()> {
let mut options = ConfigOptions::default();
- options.execution.time_zone = Some("-05:00".to_string());
+ options.execution.time_zone = Some("-05:00".parse().unwrap());
// every string array flavor `handle`/`handle_multiple` dispatch on
let string_arrays: [fn(Vec<&str>) -> ArrayRef; 3] = [
@@ -1233,7 +1209,7 @@ mod tests {
let udfs = udfs_and_timeunit();
let mut options = ConfigOptions::default();
- options.execution.time_zone = Some("-05:00".to_string());
+ options.execution.time_zone = Some("-05:00".parse().unwrap());
let time_zone: Option<Arc<str>> = options
.execution
@@ -1373,113 +1349,6 @@ mod tests {
Ok(())
}
- #[test]
- fn to_timestamp_invalid_execution_timezone_behavior() -> Result<()> {
- let field: Arc<Field> = Field::new("arg", Utf8, true).into();
- let return_field: Arc<Field> =
- Field::new("f", Timestamp(Nanosecond, None), true).into();
-
- let mut options = ConfigOptions::default();
- options.execution.time_zone = Some("Invalid/Timezone".to_string());
-
- let args = ScalarFunctionArgs {
- args: vec![ColumnarValue::Scalar(ScalarValue::Utf8(Some(
- "2020-09-08T13:42:29Z".to_string(),
- )))],
- arg_fields: vec![Arc::clone(&field)],
- number_rows: 1,
- return_field: Arc::clone(&return_field),
- config_options: Arc::new(options.clone()),
- };
-
- let expected_err =
- "Invalid timezone \"Invalid/Timezone\": failed to parse timezone";
-
- validate_expected_error(&mut options, args, expected_err);
-
- Ok(())
- }
-
- #[test]
- fn to_timestamp_formats_invalid_execution_timezone_behavior() -> Result<()> {
- let expr_field: Arc<Field> = Field::new("arg", Utf8, true).into();
- let format_field: Arc<Field> = Field::new("fmt", Utf8, true).into();
- let return_field: Arc<Field> =
- Field::new("f", Timestamp(Nanosecond, None), true).into();
-
- let mut options = ConfigOptions::default();
- options.execution.time_zone = Some("Invalid/Timezone".to_string());
-
- let expected_err =
- "Invalid timezone \"Invalid/Timezone\": failed to parse timezone";
-
- let make_args = |value: &str, format: &str| ScalarFunctionArgs {
- args: vec![
- ColumnarValue::Scalar(ScalarValue::Utf8(Some(value.to_string()))),
- ColumnarValue::Scalar(ScalarValue::Utf8(Some(format.to_string()))),
- ],
- arg_fields: vec![Arc::clone(&expr_field), Arc::clone(&format_field)],
- number_rows: 1,
- return_field: Arc::clone(&return_field),
- config_options: Arc::new(options.clone()),
- };
-
- for (value, format, _expected_str) in [
- (
- "2020-09-08 09:42:29 -05:00",
- "%Y-%m-%d %H:%M:%S %z",
- Some("2020-09-08 09:42:29 -05:00"),
- ),
- (
- "2020-09-08T13:42:29Z",
- "%+",
- Some("2020-09-08 08:42:29 -05:00"),
- ),
- (
- "2020-09-08 13:42:29 +0000",
- "%Y-%m-%d %H:%M:%S %z",
- Some("2020-09-08 08:42:29 -05:00"),
- ),
- (
- "+0000 2024-01-01 12:00:00",
- "%z %Y-%m-%d %H:%M:%S",
- Some("2024-01-01 07:00:00 -05:00"),
- ),
- (
- "20200908134229+0100",
- "%Y%m%d%H%M%S%z",
- Some("2020-09-08 07:42:29 -05:00"),
- ),
- (
- "2020-09-08+0230 13:42",
- "%Y-%m-%d%z %H:%M",
- Some("2020-09-08 06:12:00 -05:00"),
- ),
- ] {
- let args = make_args(value, format);
- validate_expected_error(&mut options.clone(), args, expected_err);
- }
-
- let args = ScalarFunctionArgs {
- args: vec![
- ColumnarValue::Scalar(ScalarValue::Utf8(Some(
- "2020-09-08T13:42:29".to_string(),
- ))),
- ColumnarValue::Scalar(ScalarValue::Utf8(Some(
- "%Y-%m-%dT%H:%M:%S".to_string(),
- ))),
- ],
- arg_fields: vec![Arc::clone(&expr_field), Arc::clone(&format_field)],
- number_rows: 1,
- return_field: Arc::clone(&return_field),
- config_options: Arc::new(options.clone()),
- };
-
- validate_expected_error(&mut options.clone(), args, expected_err);
-
- Ok(())
- }
-
#[test]
fn to_timestamp_invalid_input_type() -> Result<()> {
// pass the wrong type of input array to to_timestamp and test
diff --git a/datafusion/physical-expr/src/scalar_function.rs b/datafusion/physical-expr/src/scalar_function.rs
index a2e6c73eb4..f7bd5469cd 100644
--- a/datafusion/physical-expr/src/scalar_function.rs
+++ b/datafusion/physical-expr/src/scalar_function.rs
@@ -501,7 +501,7 @@ mod tests {
),
] {
let mut config = ConfigOptions::default();
- config.execution.time_zone = session_tz.map(str::to_owned);
+ config.execution.time_zone = session_tz.map(|tz| tz.parse().unwrap());
let udf = FromUnixtimeFunc::new_with_config(&config);
let mut args: Vec<Arc<dyn PhysicalExpr>> =
vec![Arc::new(Column::new("c", 0))];
diff --git a/datafusion/spark/src/function/conversion/cast.rs b/datafusion/spark/src/function/conversion/cast.rs
index 45d1b33626..2571ab7935 100644
--- a/datafusion/spark/src/function/conversion/cast.rs
+++ b/datafusion/spark/src/function/conversion/cast.rs
@@ -373,7 +373,7 @@ mod tests {
));
let mut config = ConfigOptions::default();
if let Some(tz) = timezone {
- config.execution.time_zone = Some(tz.to_string());
+ config.execution.time_zone = Some(tz.parse().unwrap());
}
ScalarFunctionArgs {
args: vec![
@@ -761,7 +761,7 @@ mod tests {
true,
));
let mut config = ConfigOptions::default();
- config.execution.time_zone = Some("UTC".to_string());
+ config.execution.time_zone = Some("UTC".parse().unwrap());
config.execution.enable_ansi_mode = enable_ansi_mode;
ScalarFunctionArgs {
args: vec![
diff --git a/datafusion/sql/src/planner.rs b/datafusion/sql/src/planner.rs
index d7a1aaf299..87a58bec68 100644
--- a/datafusion/sql/src/planner.rs
+++ b/datafusion/sql/src/planner.rs
@@ -782,7 +782,12 @@ impl<'a, S: ContextProvider> SqlToRel<'a, S> {
// timezone-naive `Timestamp<TimeUnit, None>`. Whether that
// should remain the behavior is tracked in
// https://github.com/apache/datafusion/issues/25166
- self.context_provider.options().execution.time_zone.clone()
+ self.context_provider
+ .options()
+ .execution
+ .time_zone
+ .as_ref()
+ .map(|tz| tz.as_str().to_string())
} else {
// Timestamp Without Time zone
None
diff --git a/datafusion/sqllogictest/test_files/set_variable.slt b/datafusion/sqllogictest/test_files/set_variable.slt
index 8bb65519db..ab86363f67 100644
--- a/datafusion/sqllogictest/test_files/set_variable.slt
+++ b/datafusion/sqllogictest/test_files/set_variable.slt
@@ -209,27 +209,29 @@ SELECT '2000-01-01T00:00:00'::TIMESTAMP::TIMESTAMPTZ
----
2000-01-01T00:00:00+08:00
-# set time zone bad time zone format
+# the time zone is shown as it was written, not normalized
+query TT
+SHOW TIME ZONE
+----
+datafusion.execution.time_zone +08
+
+# set time zone bad time zone format: rejected at SET time, keeping the previous value
statement ok
set datafusion.catalog.information_schema = true
-statement ok
+statement error Invalid or Unsupported Configuration: Invalid time zone: \+08:00:00\. Valid values are UTC offsets such as \+08:00 or IANA time zone names such as Asia/Taipei
SET TIME ZONE = '+08:00:00'
-statement error Arrow error: Parser error: Invalid timezone "\+08:00:00": failed to parse timezone
-SELECT '2000-01-01T00:00:00'::TIMESTAMP::TIMESTAMPTZ
-
-statement ok
+statement error Invalid time zone: 08:00\.
SET TIME ZONE = '08:00'
-statement error Arrow error: Parser error: Invalid timezone "08:00": failed to parse timezone
-SELECT '2000-01-01T00:00:00'::TIMESTAMP::TIMESTAMPTZ
-
-statement ok
+statement error Invalid time zone: 08\.
SET TIME ZONE = '08'
-statement error Arrow error: Parser error: Invalid timezone "08": failed to parse timezone
-SELECT '2000-01-01T00:00:00'::TIMESTAMP::TIMESTAMPTZ
+query TT
+SHOW TIME ZONE
+----
+datafusion.execution.time_zone +08
statement ok
SET TIME ZONE = 'Asia/Taipei'
@@ -239,11 +241,16 @@ SELECT '2000-01-01T00:00:00'::TIMESTAMP::TIMESTAMPTZ
----
2000-01-01T00:00:00+08:00
-statement ok
+statement error Invalid time zone: Asia/Taipei2\.
SET TIME ZONE = 'Asia/Taipei2'
-statement error Arrow error: Parser error: Invalid timezone "Asia/Taipei2": failed to parse timezone
-SELECT '2000-01-01T00:00:00'::TIMESTAMP::TIMESTAMPTZ
+statement error Invalid time zone: Asia/Taipei2\.
+SET datafusion.execution.time_zone = 'Asia/Taipei2'
+
+query TT
+SHOW TIME ZONE
+----
+datafusion.execution.time_zone Asia/Taipei
# reset variable restores default
statement ok
diff --git a/docs/source/library-user-guide/upgrading/56.0.0.md b/docs/source/library-user-guide/upgrading/56.0.0.md
index 6c251c5cfb..4cc7c30e17 100644
--- a/docs/source/library-user-guide/upgrading/56.0.0.md
+++ b/docs/source/library-user-guide/upgrading/56.0.0.md
@@ -269,6 +269,31 @@ let mut config = SessionConfig::new();
config.options_mut().execution.enable_nlj_coordinated_fallback = false;
```
+### `ExecutionOptions::time_zone` is now a validated `ConfigTimeZone`
+
+`ExecutionOptions::time_zone` changed from `Option<String>` to
+`Option<ConfigTimeZone>`. The value is now checked when it is set, so
+`SET TIME ZONE = 'Asia/Taipei2'` fails immediately instead of on a later query.
+The string is kept exactly as written (`+08` stays `+08`).
+
+**Who is affected:**
+
+- Code that reads or assigns `ExecutionOptions::time_zone` directly.
+- Code that sets the value through SQL `SET` or `ConfigOptions::set` needs no
+ code changes, but invalid time zones are now rejected immediately.
+
+**Migration guide:**
+
+```rust,ignore
+// Before
+options.execution.time_zone = Some("America/Denver".to_string());
+let tz: Option<String> = options.execution.time_zone.clone();
+
+// After
+options.execution.time_zone = Some("America/Denver".parse()?);
+let tz: Option<&str> = options.execution.time_zone.as_ref().map(|tz| tz.as_str());
+```
+
### `ExecutionOptions` has a new `soft_max_bytes_per_output_file` field
`ExecutionOptions` gained a public `soft_max_bytes_per_output_file: ConfigNonZeroUsize`
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org