From f93f0e86825caa20926241c3f932f7e403a3a40d Mon Sep 17 00:00:00 2001 From: ritchie46 Date: Wed, 26 Aug 2026 14:40:59 +0200 Subject: [PATCH 1/6] fix: Fix SQL casts --- crates/polars-expr/src/dispatch/strings.rs | 85 +++++---- crates/polars-sql/src/sql_expr.rs | 45 ++++- .../polars-stream/src/nodes/strptime_infer.rs | 10 ++ py-polars/tests/unit/sql/test_cast.py | 162 +++++++++++++++++- 4 files changed, 254 insertions(+), 48 deletions(-) diff --git a/crates/polars-expr/src/dispatch/strings.rs b/crates/polars-expr/src/dispatch/strings.rs index d8792f6334c5..f8e9f42a7c32 100644 --- a/crates/polars-expr/src/dispatch/strings.rs +++ b/crates/polars-expr/src/dispatch/strings.rs @@ -466,23 +466,38 @@ pub(super) fn split_regex(s: &[Column], inclusive: bool, strict: bool) -> Polars Ok(out.into_column()) } +/// Without an explicit format, non-strict parsing yields nulls rather than +/// raising when no value in the column allows the format to be inferred. +#[cfg(feature = "temporal")] +fn null_on_failed_inference( + out: PolarsResult, + s: &Column, + dtype: &DataType, + options: &StrptimeOptions, +) -> PolarsResult { + match out { + Err(_) if !options.strict && options.format.is_none() => { + Ok(Column::full_null(s.name().clone(), s.len(), dtype)) + }, + out => out, + } +} + #[cfg(feature = "dtype-date")] fn to_date(s: &Column, options: &StrptimeOptions) -> PolarsResult { let ca = s.str()?; - let out = { - if options.exact { - ca.as_date(options.format.as_deref(), options.cache)? - .into_column() - } else { - ca.as_date_not_exact(options.format.as_deref())? - .into_column() - } - }; + let out = if options.exact { + ca.as_date(options.format.as_deref(), options.cache) + } else { + ca.as_date_not_exact(options.format.as_deref()) + } + .map(|ca| ca.into_column()); + let out = null_on_failed_inference(out, s, &DataType::Date, options)?; if options.strict && ca.null_count() != out.null_count() { handle_casting_failures(s.as_materialized_series(), out.as_materialized_series())?; } - Ok(out.into_column()) + Ok(out) } #[cfg(feature = "dtype-datetime")] @@ -511,33 +526,32 @@ fn to_datetime( }; let out = if options.exact { - datetime_strings - .as_datetime( - options.format.as_deref(), - *time_unit, - options.cache, - tz_aware, - time_zone, - ambiguous, - )? - .into_column() + datetime_strings.as_datetime( + options.format.as_deref(), + *time_unit, + options.cache, + tz_aware, + time_zone, + ambiguous, + ) } else { - datetime_strings - .as_datetime_not_exact( - options.format.as_deref(), - *time_unit, - tz_aware, - time_zone, - ambiguous, - true, - )? - .into_column() - }; + datetime_strings.as_datetime_not_exact( + options.format.as_deref(), + *time_unit, + tz_aware, + time_zone, + ambiguous, + true, + ) + } + .map(|ca| ca.into_column()); + let dtype = DataType::Datetime(*time_unit, time_zone.cloned()); + let out = null_on_failed_inference(out, &s[0], &dtype, options)?; if options.strict && datetime_strings.null_count() != out.null_count() { handle_casting_failures(s[0].as_materialized_series(), out.as_materialized_series())?; } - Ok(out.into_column()) + Ok(out) } #[cfg(feature = "dtype-time")] @@ -548,13 +562,14 @@ fn to_time(s: &Column, options: &StrptimeOptions) -> PolarsResult { let ca = s.str()?; let out = ca - .as_time(options.format.as_deref(), options.cache)? - .into_column(); + .as_time(options.format.as_deref(), options.cache) + .map(|ca| ca.into_column()); + let out = null_on_failed_inference(out, s, &DataType::Time, options)?; if options.strict && ca.null_count() != out.null_count() { handle_casting_failures(s.as_materialized_series(), out.as_materialized_series())?; } - Ok(out.into_column()) + Ok(out) } #[cfg(feature = "concat_str")] diff --git a/crates/polars-sql/src/sql_expr.rs b/crates/polars-sql/src/sql_expr.rs index d3a2850c3787..f7840585718a 100644 --- a/crates/polars-sql/src/sql_expr.rs +++ b/crates/polars-sql/src/sql_expr.rs @@ -116,6 +116,24 @@ fn cast_literal_series(s: &Series, dtype: &DataType) -> PolarsResult { s.strict_cast(dtype) } +/// Parse a string expression into `Date`/`Time`/`Datetime`; non-strict yields +/// nulls on unparseable input (used by `TRY_CAST`). +fn parse_string_as_temporal(expr: Expr, dtype: &DataType, strict: bool) -> PolarsResult { + let options = StrptimeOptions { + strict, + ..Default::default() + }; + Ok(match dtype { + DataType::Date => expr.str().to_date(options), + DataType::Time => expr.str().to_time(options), + DataType::Datetime(tu, tz) => { + expr.str() + .to_datetime(Some(*tu), tz.clone(), options, lit("latest")) + }, + _ => polars_bail!(SQLInterface: "cannot parse string as {:?}", dtype), + }) +} + /// Extract the literal value; returns `(sql_value, optional_op)`. fn extract_literal_with_op<'a>( expr: &'a SQLExpr, @@ -954,7 +972,7 @@ impl SQLExprVisitor<'_> { dtype, DataType::Date | DataType::Time | DataType::Datetime(_, _) ) { - return elems.strict_cast(dtype); + return cast_literal_series(&elems, dtype); } } } @@ -1005,12 +1023,31 @@ impl SQLExprVisitor<'_> { return Ok(expr.str().json_decode(DataType::Struct(Vec::new()))); } let polars_type = map_sql_dtype_to_polars(dtype)?; - Ok(match cast_kind { - CastKind::Cast | CastKind::DoubleColon => expr.strict_cast(polars_type), - CastKind::TryCast | CastKind::SafeCast => expr.cast(polars_type), + let strict = matches!(cast_kind, CastKind::Cast | CastKind::DoubleColon); + + // `CAST( AS DATE/TIME/TIMESTAMP)` parses rather than casts + if matches!( + polars_type, + DataType::Date | DataType::Time | DataType::Datetime(_, _) + ) && self.is_string_expr(&expr) + { + return parse_string_as_temporal(expr, &polars_type, strict); + } + Ok(if strict { + expr.strict_cast(polars_type) + } else { + expr.cast(polars_type) }) } + /// Resolve whether `expr` is known to be `String`-typed against the active + /// schema; false when the dtype cannot be determined. + fn is_string_expr(&self, expr: &Expr) -> bool { + let empty = Schema::default(); + let schema = self.active_schema.unwrap_or(&empty); + matches!(expr.to_field(schema), Ok(fld) if fld.dtype == DataType::String) + } + /// Visit a SQL literal. /// /// e.g. 1, 'foo', 1.0, NULL diff --git a/crates/polars-stream/src/nodes/strptime_infer.rs b/crates/polars-stream/src/nodes/strptime_infer.rs index 01c243aa3e8c..02ad8d6fa48b 100644 --- a/crates/polars-stream/src/nodes/strptime_infer.rs +++ b/crates/polars-stream/src/nodes/strptime_infer.rs @@ -183,6 +183,16 @@ impl ComputeNode for StrptimeInferNode { if let Some(idx) = ca.first_non_null() { *infer_slot = FormatInfer::try_new(ca.get(idx).unwrap(), dtype, options)?; + + let unit = if matches!(dtype, DataType::Time) { + "time" + } else { + "date" + }; + polars_ensure!( + infer_slot.is_some() || !options.strict, + parse_fmt_idk = unit + ); } } diff --git a/py-polars/tests/unit/sql/test_cast.py b/py-polars/tests/unit/sql/test_cast.py index 73adb3b6cca3..4e2330aaf994 100644 --- a/py-polars/tests/unit/sql/test_cast.py +++ b/py-polars/tests/unit/sql/test_cast.py @@ -1,12 +1,17 @@ from __future__ import annotations +from datetime import date, datetime, time from typing import Any import pytest import polars as pl import polars.selectors as cs -from polars.exceptions import InvalidOperationError, SQLInterfaceError +from polars.exceptions import ( + ComputeError, + InvalidOperationError, + SQLInterfaceError, +) from polars.testing import assert_frame_equal @@ -164,20 +169,47 @@ def test_cast() -> None: @pytest.mark.parametrize( - ("values", "cast_op", "error"), + ("values", "cast_op", "exc", "error"), [ - ([1.0, -1.0], "values::uint8", "conversion from `f64` to `u64` failed"), - ([10, 0, -1], "values::uint4", "conversion from `i64` to `u32` failed"), - ([int(1e8)], "values::int1", "conversion from `i64` to `i8` failed"), - (["a", "b"], "values::time", "conversion from `str` to `time` failed"), - (["a", "b"], "values::int4", "conversion from `str` to `i32` failed"), + ( + [1.0, -1.0], + "values::uint8", + InvalidOperationError, + "conversion from `f64` to `u64` failed", + ), + ( + [10, 0, -1], + "values::uint4", + InvalidOperationError, + "conversion from `i64` to `u32` failed", + ), + ( + [int(1e8)], + "values::int1", + InvalidOperationError, + "conversion from `i64` to `i8` failed", + ), + ( + ["a", "b"], + "values::time", + ComputeError, + "could not find an appropriate format to parse times", + ), + ( + ["a", "b"], + "values::int4", + InvalidOperationError, + "conversion from `str` to `i32` failed", + ), ], ) -def test_cast_errors(values: Any, cast_op: str, error: str) -> None: +def test_cast_errors( + values: Any, cast_op: str, exc: type[Exception], error: str +) -> None: df = pl.DataFrame({"values": values}) # invalid CAST should raise an error... - with pytest.raises(InvalidOperationError, match=error): + with pytest.raises(exc, match=error): df.sql(f"SELECT {cast_op} FROM self") # ... or return `null` values if using TRY_CAST @@ -186,6 +218,118 @@ def test_cast_errors(values: Any, cast_op: str, error: str) -> None: assert None in res.to_series() +@pytest.mark.parametrize( + ("sql_type", "dtype", "value", "expected"), + [ + ("date", pl.Date, "2000-02-01", date(2000, 2, 1)), + ( + "timestamp", + pl.Datetime("us"), + "2000-02-01 12:30:00", + datetime(2000, 2, 1, 12, 30), + ), + ( + "datetime", + pl.Datetime("us"), + "2000-02-01 12:30:00", + datetime(2000, 2, 1, 12, 30), + ), + ("time", pl.Time, "12:30:00", time(12, 30)), + ], +) +def test_cast_string_to_temporal( + sql_type: str, dtype: pl.DataType, value: str, expected: Any +) -> None: + df = pl.DataFrame({"s": [value, None]}) + + for cast_op in ( + f"CAST(s AS {sql_type})", + f"TRY_CAST(s AS {sql_type})", + f"s::{sql_type}", + ): + res = df.sql(f"SELECT {cast_op} AS x FROM self") + assert_frame_equal( + res, pl.DataFrame({"x": [expected, None]}, schema={"x": dtype}) + ) + + for cast_op in ( + f"CAST('{value}' AS {sql_type})", + f"TRY_CAST('{value}' AS {sql_type})", + f"'{value}'::{sql_type}", + ): + res = df.sql(f"SELECT {cast_op} AS x FROM self") + assert_frame_equal( + res, pl.DataFrame({"x": [expected, expected]}, schema={"x": dtype}) + ) + + +@pytest.mark.parametrize( + ("sql_type", "dtype"), + [ + ("date", pl.Date), + ("timestamp", pl.Datetime("us")), + ("time", pl.Time), + ], +) +def test_try_cast_string_to_temporal_nulls(sql_type: str, dtype: pl.DataType) -> None: + df = pl.DataFrame({"s": ["not a temporal value"]}) + + res = df.sql(f"SELECT TRY_CAST(s AS {sql_type}) AS x FROM self") + assert_frame_equal(res, pl.DataFrame({"x": [None]}, schema={"x": dtype})) + + with pytest.raises(ComputeError, match="could not find an appropriate format"): + df.sql(f"SELECT CAST(s AS {sql_type}) AS x FROM self") + + +def test_cast_temporal_to_temporal_is_not_parsed() -> None: + df = pl.DataFrame( + {"dtm": [datetime(2000, 2, 1, 12, 30)]}, + schema={"dtm": pl.Datetime("us")}, + ) + res = df.sql( + """ + SELECT + CAST(dtm AS date) AS d, + CAST(dtm AS time) AS t + FROM self + """ + ) + assert_frame_equal( + res, + pl.DataFrame({"d": [date(2000, 2, 1)], "t": [time(12, 30)]}), + ) + + +def test_cast_string_to_date_in_between() -> None: + df = pl.DataFrame( + {"d": [date(1999, 1, 1), date(1999, 3, 1), date(2000, 1, 1)]}, + ) + res = df.sql( + "SELECT * FROM self WHERE d BETWEEN CAST('1999-02-22' AS date) AND CAST('1999-03-24' AS date)" + ) + assert_frame_equal(res, pl.DataFrame({"d": [date(1999, 3, 1)]})) + + +def test_temporal_in_string_list() -> None: + df = pl.DataFrame( + {"d": [date(1999, 1, 1), date(1999, 3, 1), date(2000, 1, 1)]}, + ) + res = df.sql("SELECT * FROM self WHERE d IN ('1999-03-01', '2000-01-01')") + assert_frame_equal(res, pl.DataFrame({"d": [date(1999, 3, 1), date(2000, 1, 1)]})) + + +def test_try_cast_string_to_temporal_partial() -> None: + df = pl.DataFrame({"s": ["2000-02-01", "nope"]}) + + res = df.sql("SELECT TRY_CAST(s AS date) AS x FROM self") + assert_frame_equal( + res, pl.DataFrame({"x": [date(2000, 2, 1), None]}, schema={"x": pl.Date}) + ) + + with pytest.raises(InvalidOperationError, match=r"conversion .* failed"): + df.sql("SELECT CAST(s AS date) AS x FROM self") + + @pytest.mark.may_fail_cloud # reason: eager construct to_struct @pytest.mark.xfail # this is a construct we cannot deal with anymore def test_cast_json() -> None: From b61eff23257a5baeb235d7d42e974abcb8a005c8 Mon Sep 17 00:00:00 2001 From: ritchie46 Date: Wed, 26 Aug 2026 16:10:43 +0200 Subject: [PATCH 2/6] test: Cover temporal cast literals and strptime engine consistency Extends the SQL string->temporal cast tests with literal operands, which exercise the constant-folded (eager) path rather than the streaming node. Moves the engine-consistency check out of the SQL suite into test_to_datetime.py, parametrised over both engines and all three of to_date/to_datetime/to_time, since it is about strptime rather than casts. Also includes a doc-comment trim on parse_string_as_temporal that was already present in the working tree. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01MsiVh8nEYU5sBSXDcMgGSB --- crates/polars-sql/src/sql_expr.rs | 3 +-- .../namespaces/temporal/test_to_datetime.py | 27 ++++++++++++++++++- py-polars/tests/unit/sql/test_cast.py | 9 ++++--- 3 files changed, 32 insertions(+), 7 deletions(-) diff --git a/crates/polars-sql/src/sql_expr.rs b/crates/polars-sql/src/sql_expr.rs index f7840585718a..64a1b8f3e9b1 100644 --- a/crates/polars-sql/src/sql_expr.rs +++ b/crates/polars-sql/src/sql_expr.rs @@ -116,8 +116,7 @@ fn cast_literal_series(s: &Series, dtype: &DataType) -> PolarsResult { s.strict_cast(dtype) } -/// Parse a string expression into `Date`/`Time`/`Datetime`; non-strict yields -/// nulls on unparseable input (used by `TRY_CAST`). +/// Parse a string expression into `Date`/`Time`/`Datetime` fn parse_string_as_temporal(expr: Expr, dtype: &DataType, strict: bool) -> PolarsResult { let options = StrptimeOptions { strict, diff --git a/py-polars/tests/unit/operations/namespaces/temporal/test_to_datetime.py b/py-polars/tests/unit/operations/namespaces/temporal/test_to_datetime.py index be59743c5fb1..2b3517399045 100644 --- a/py-polars/tests/unit/operations/namespaces/temporal/test_to_datetime.py +++ b/py-polars/tests/unit/operations/namespaces/temporal/test_to_datetime.py @@ -16,7 +16,7 @@ if TYPE_CHECKING: from hypothesis.strategies import DrawFn - from polars._typing import TimeUnit + from polars._typing import EngineType, TimeUnit DATE_FORMATS = ["%Y{}%m{}%d", "%d{}%m{}%Y"] @@ -329,3 +329,28 @@ def test_to_datetime_inexact_unicode_multibyte() -> None: assert_frame_equal( out, pl.DataFrame({"a": [datetime(2020, 2, 3, 12, 53, 11), None]}) ) + + +@pytest.mark.parametrize("engine", ["in-memory", "streaming"]) +@pytest.mark.parametrize( + ("method", "dtype"), + [ + ("to_date", pl.Date), + ("to_datetime", pl.Datetime("us")), + ("to_time", pl.Time), + ], +) +def test_strptime_uninferrable_format( + engine: EngineType, method: str, dtype: pl.DataType +) -> None: + lf = pl.LazyFrame({"s": ["not a temporal value"]}) + + non_strict = getattr(pl.col("s").str, method)(strict=False) + assert_frame_equal( + lf.select(non_strict).collect(engine=engine), + pl.DataFrame({"s": [None]}, schema={"s": dtype}), + ) + + strict = getattr(pl.col("s").str, method)(strict=True) + with pytest.raises(ComputeError, match="could not find an appropriate format"): + lf.select(strict).collect(engine=engine) diff --git a/py-polars/tests/unit/sql/test_cast.py b/py-polars/tests/unit/sql/test_cast.py index 4e2330aaf994..20080194577d 100644 --- a/py-polars/tests/unit/sql/test_cast.py +++ b/py-polars/tests/unit/sql/test_cast.py @@ -274,11 +274,12 @@ def test_cast_string_to_temporal( def test_try_cast_string_to_temporal_nulls(sql_type: str, dtype: pl.DataType) -> None: df = pl.DataFrame({"s": ["not a temporal value"]}) - res = df.sql(f"SELECT TRY_CAST(s AS {sql_type}) AS x FROM self") - assert_frame_equal(res, pl.DataFrame({"x": [None]}, schema={"x": dtype})) + for operand in ("s", "'not a temporal value'"): + res = df.sql(f"SELECT TRY_CAST({operand} AS {sql_type}) AS x FROM self") + assert_frame_equal(res, pl.DataFrame({"x": [None]}, schema={"x": dtype})) - with pytest.raises(ComputeError, match="could not find an appropriate format"): - df.sql(f"SELECT CAST(s AS {sql_type}) AS x FROM self") + with pytest.raises(ComputeError, match="could not find an appropriate format"): + df.sql(f"SELECT CAST({operand} AS {sql_type}) AS x FROM self") def test_cast_temporal_to_temporal_is_not_parsed() -> None: From a65204837fafd247404827d8b5d9efe3edf05620 Mon Sep 17 00:00:00 2001 From: ritchie46 Date: Wed, 26 Aug 2026 16:36:19 +0200 Subject: [PATCH 3/6] fix: Scan for a strptime format instead of using the first value Format inference had three implementations. Two of them inferred from a single value: `as_time`/`as_date_not_exact`/`as_datetime_not_exact` used the first non-null value of the column, and the streaming `StrptimeInferNode` used the first non-null value of each morsel, emitting an all-null morsel when that value yielded no pattern. The streaming case gave silently wrong results on the default engine. With one unparseable value at the head of a 200k-row frame, `to_date(strict=False)` returned 8334 nulls (one morsel) where the in-memory engine returned 1, so the answer depended on where the bad values sat and on the morsel size. `to_time` was wrong in both engines for the same reason. Extracts the scan `infer.rs` already performed into `infer_from_values` and uses it everywhere, so a value is null only when it is genuinely unparseable. Also stops the streaming strict error reporting the internal alias assigned during lowering ('_POLARS_TMP_0') rather than the column the user named. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01MsiVh8nEYU5sBSXDcMgGSB --- .../polars-stream/src/nodes/strptime_infer.rs | 43 +++++++++++++----- .../src/physical_plan/lower_expr.rs | 2 + crates/polars-stream/src/physical_plan/mod.rs | 4 ++ .../src/physical_plan/to_graph.rs | 2 + .../src/chunkedarray/string/infer.rs | 22 ++++----- .../src/chunkedarray/string/mod.rs | 42 +++++++---------- .../namespaces/temporal/test_to_datetime.py | 45 +++++++++++++++++++ 7 files changed, 112 insertions(+), 48 deletions(-) diff --git a/crates/polars-stream/src/nodes/strptime_infer.rs b/crates/polars-stream/src/nodes/strptime_infer.rs index 02ad8d6fa48b..178d2ae6d505 100644 --- a/crates/polars-stream/src/nodes/strptime_infer.rs +++ b/crates/polars-stream/src/nodes/strptime_infer.rs @@ -4,7 +4,7 @@ use polars_time::chunkedarray::StringMethods; use polars_time::chunkedarray::string::Pattern; use polars_time::chunkedarray::string::infer::{ DatetimeInfer, TryFromWithUnit, coerce_string_to_date, coerce_string_to_datetime, - infer_pattern_date_single, infer_pattern_datetime_single, sniff_time_fmt, + infer_from_values, infer_pattern_date_single, infer_pattern_datetime_single, sniff_time_fmt, }; use super::compute_node_prelude::*; @@ -15,6 +15,10 @@ pub struct StrptimeInferNode { infer: Option, phase: Phase, + /// Name to report parse failures against; the input carries an internal + /// one assigned during lowering. + input_name: PlSmallStr, + /// Ambiguous can be `raise`, `earliest`, `latest` and `null`. /// /// If it broadcast and it is `raise` or `null`, we can actually execute it here. So @@ -43,27 +47,35 @@ enum FormatInfer { } impl StrptimeInferNode { - pub fn new(dtype: DataType, options: StrptimeOptions, ambiguous_is_raise: bool) -> Self { + pub fn new( + dtype: DataType, + options: StrptimeOptions, + input_name: PlSmallStr, + ambiguous_is_raise: bool, + ) -> Self { Self { dtype, options, infer: None, phase: Phase::Inferring, + input_name, ambiguous_is_raise, } } } impl FormatInfer { + /// Scans `ca` rather than only its first value; which values are + /// unparseable must not depend on where they sit in the column. fn try_new( - val: &str, + ca: &StringChunked, dtype: &DataType, options: &StrptimeOptions, ) -> PolarsResult> { match dtype { #[cfg(feature = "dtype-date")] DataType::Date => { - let Some(pattern) = infer_pattern_date_single(val) else { + let Some(pattern) = infer_from_values(ca, infer_pattern_date_single) else { return Ok(None); }; let infer = DatetimeInfer::::try_from_with_unit(pattern, None)?; @@ -71,7 +83,7 @@ impl FormatInfer { }, #[cfg(feature = "dtype-datetime")] DataType::Datetime(tu, tz) => { - let Some(pattern) = infer_pattern_datetime_single(val) else { + let Some(pattern) = infer_from_values(ca, infer_pattern_datetime_single) else { return Ok(None); }; if matches!(pattern, Pattern::DatetimeYMDZ) && tz.is_none() { @@ -81,7 +93,10 @@ impl FormatInfer { Ok(Some(FormatInfer::Datetime(infer, tz.clone()))) }, #[cfg(feature = "dtype-time")] - DataType::Time => Ok(sniff_time_fmt(val).map(|f| FormatInfer::Time(f, options.cache))), + DataType::Time => { + Ok(infer_from_values(ca, sniff_time_fmt) + .map(|f| FormatInfer::Time(f, options.cache))) + }, _ => Ok(None), } } @@ -91,6 +106,7 @@ impl FormatInfer { col: &Column, ambiguous: &StringChunked, strict: bool, + input_name: &PlSmallStr, ) -> PolarsResult { let ca = col.str()?; let name = col.name().clone(); @@ -119,9 +135,11 @@ impl FormatInfer { }; if strict && col.null_count() != result.null_count() { + // reported against the name the user wrote, not the internal alias + let reported = result.clone().with_name(input_name.clone()); polars_core::utils::handle_casting_failures( col.as_materialized_series(), - result.as_materialized_series(), + reported.as_materialized_series(), )?; } @@ -174,15 +192,15 @@ impl ComputeNode for StrptimeInferNode { let dtype = &self.dtype; let options = &self.options; + let input_name = &self.input_name; let infer_slot = &mut self.infer; join_handles.push(scope.spawn_task(TaskPriority::High, async move { while let Ok(morsel) = recv.recv().await { if infer_slot.is_none() { let df = morsel.df().await; let ca = df.columns()[0].str()?; - if let Some(idx) = ca.first_non_null() { - *infer_slot = - FormatInfer::try_new(ca.get(idx).unwrap(), dtype, options)?; + if ca.first_non_null().is_some() { + *infer_slot = FormatInfer::try_new(ca, dtype, options)?; let unit = if matches!(dtype, DataType::Time) { "time" @@ -206,7 +224,7 @@ impl ComputeNode for StrptimeInferNode { let cols = df.columns(); if let Some(ref mut infer) = *infer_slot { infer - .apply(&cols[0], &ambiguous, options.strict) + .apply(&cols[0], &ambiguous, options.strict, input_name) .map(Column::into_frame) } else { Ok(Column::full_null( @@ -231,6 +249,7 @@ impl ComputeNode for StrptimeInferNode { let senders = send_ports[0].take().unwrap().parallel(); for (mut recv, mut send) in receivers.into_iter().zip(senders) { let strict = self.options.strict; + let input_name = self.input_name.clone(); let ambiguous = ambiguous.clone(); let mut infer = self.infer.clone().unwrap(); join_handles.push(scope.spawn_task(TaskPriority::High, async move { @@ -239,7 +258,7 @@ impl ComputeNode for StrptimeInferNode { .try_map(|df| { let cols = df.columns(); infer - .apply(&cols[0], &ambiguous, strict) + .apply(&cols[0], &ambiguous, strict, &input_name) .map(Column::into_frame) }) .await?; diff --git a/crates/polars-stream/src/physical_plan/lower_expr.rs b/crates/polars-stream/src/physical_plan/lower_expr.rs index 9a9f298dd347..b5968b18bc2e 100644 --- a/crates/polars-stream/src/physical_plan/lower_expr.rs +++ b/crates/polars-stream/src/physical_plan/lower_expr.rs @@ -1565,6 +1565,7 @@ fn lower_exprs_with_ctx( } if options.format.is_none() && matches!(ctx.expr_arena.get(inner_exprs[1].node()), AExpr::Literal(s) if matches!(s.extract_str(), Some("raise" | "null"))) => { + let input_name = inner_exprs[0].output_name().clone(); let col_name = unique_column_name(); let select_stream = build_select_stream_with_ctx( input, @@ -1582,6 +1583,7 @@ fn lower_exprs_with_ctx( input: select_stream, dtype: dtype.as_ref().clone(), options: options.clone(), + input_name, ambiguous_is_raise, }; let node_key = ctx.phys_sm.insert(PhysNode::new(output_schema, kind)); diff --git a/crates/polars-stream/src/physical_plan/mod.rs b/crates/polars-stream/src/physical_plan/mod.rs index 26f28e933819..a8e1f5fd1789 100644 --- a/crates/polars-stream/src/physical_plan/mod.rs +++ b/crates/polars-stream/src/physical_plan/mod.rs @@ -285,6 +285,10 @@ pub enum PhysNodeKind { dtype: DataType, options: StrptimeOptions, + /// Name the input had before it was aliased to an internal one; used + /// only to report parse failures against the name the user wrote. + input_name: PlSmallStr, + /// Ambiguous can be `raise`, `earliest`, `latest` and `null`. /// /// If it is broadcast and it is `raise` or `null`, we can actually execute it in this diff --git a/crates/polars-stream/src/physical_plan/to_graph.rs b/crates/polars-stream/src/physical_plan/to_graph.rs index 5aaa6cb83c30..18e64ecce05c 100644 --- a/crates/polars-stream/src/physical_plan/to_graph.rs +++ b/crates/polars-stream/src/physical_plan/to_graph.rs @@ -561,6 +561,7 @@ fn to_graph_rec<'a>( input, dtype, options, + input_name, ambiguous_is_raise, } => { let input_key = to_graph_rec(input.node, ctx)?; @@ -568,6 +569,7 @@ fn to_graph_rec<'a>( nodes::strptime_infer::StrptimeInferNode::new( dtype.clone(), options.clone(), + input_name.clone(), *ambiguous_is_raise, ), [(input_key, input.port)], diff --git a/crates/polars-time/src/chunkedarray/string/infer.rs b/crates/polars-time/src/chunkedarray/string/infer.rs index d165033aa820..76f51629def4 100644 --- a/crates/polars-time/src/chunkedarray/string/infer.rs +++ b/crates/polars-time/src/chunkedarray/string/infer.rs @@ -424,6 +424,14 @@ pub fn sniff_time_fmt(val: &str) -> Option<&'static str> { .find(|fmt| NaiveTime::parse_from_str(val, fmt).is_ok()) } +/// Scan the non-null values for the first that `infer` accepts. +/// +/// Inferring from only the first non-null value makes the result depend on +/// where unparseable values happen to sit; every caller must scan. +pub fn infer_from_values(ca: &StringChunked, infer: impl FnMut(&str) -> Option) -> Option { + ca.iter().flatten().find_map(infer) +} + #[cfg(feature = "dtype-datetime")] pub fn to_datetime_with_inferred_tz( ca: &StringChunked, @@ -463,11 +471,8 @@ pub fn to_datetime( None => { Ok(Int64Chunked::full_null(ca.name().clone(), ca.len()).into_datetime(tu, tz.cloned())) }, - Some(idx) => { - let subset = ca.slice(idx as i64, ca.len()); - let pattern = subset - .iter() - .find_map(|opt_val| opt_val.and_then(infer_pattern_datetime_single)) + Some(_) => { + let pattern = infer_from_values(ca, infer_pattern_datetime_single) .ok_or_else(|| polars_err!(parse_fmt_idk = "date"))?; let mut infer = DatetimeInfer::::try_from_with_unit(pattern, Some(tu))?; #[cfg(feature = "timezones")] @@ -531,11 +536,8 @@ pub fn coerce_string_to_datetime( pub(crate) fn to_date(ca: &StringChunked) -> PolarsResult { match ca.first_non_null() { None => Ok(Int32Chunked::full_null(ca.name().clone(), ca.len()).into_date()), - Some(idx) => { - let subset = ca.slice(idx as i64, ca.len()); - let pattern = subset - .iter() - .find_map(|opt_val| opt_val.and_then(infer_pattern_date_single)) + Some(_) => { + let pattern = infer_from_values(ca, infer_pattern_date_single) .ok_or_else(|| polars_err!(parse_fmt_idk = "date"))?; let mut infer = DatetimeInfer::::try_from_with_unit(pattern, None).unwrap(); coerce_string_to_date(&mut infer, ca) diff --git a/crates/polars-time/src/chunkedarray/string/mod.rs b/crates/polars-time/src/chunkedarray/string/mod.rs index dbd683953e9d..f8eb203419f3 100644 --- a/crates/polars-time/src/chunkedarray/string/mod.rs +++ b/crates/polars-time/src/chunkedarray/string/mod.rs @@ -50,23 +50,6 @@ where .copied() } -#[cfg(feature = "dtype-datetime")] -fn sniff_fmt_datetime(val: &str) -> PolarsResult<&'static str> { - datetime_pattern(val, NaiveDateTime::parse_from_str) - .or_else(|| datetime_pattern(val, NaiveDate::parse_from_str)) - .ok_or_else(|| polars_err!(parse_fmt_idk = "datetime")) -} - -#[cfg(feature = "dtype-date")] -fn sniff_fmt_date(val: &str) -> PolarsResult<&'static str> { - date_pattern(val, NaiveDate::parse_from_str).ok_or_else(|| polars_err!(parse_fmt_idk = "date")) -} - -#[cfg(feature = "dtype-time")] -fn sniff_fmt_time(val: &str) -> PolarsResult<&'static str> { - time_pattern(val, NaiveTime::parse_from_str).ok_or_else(|| polars_err!(parse_fmt_idk = "time")) -} - pub trait StringMethods: AsString { #[cfg(feature = "dtype-time")] /// Parsing string values and return a [`TimeChunked`] @@ -75,14 +58,16 @@ pub trait StringMethods: AsString { let fmt = match fmt { Some(fmt) => fmt, None => { - let Some(idx) = string_ca.first_non_null() else { + if string_ca.first_non_null().is_none() { return Ok( Int64Chunked::full_null(string_ca.name().clone(), string_ca.len()) .into_time(), ); }; - let val = string_ca.get(idx).expect("should not be null"); - sniff_fmt_time(val)? + infer::infer_from_values(string_ca, |val| { + time_pattern(val, NaiveTime::parse_from_str) + }) + .ok_or_else(|| polars_err!(parse_fmt_idk = "time"))? }, }; let use_cache = use_cache && string_ca.len() > 50; @@ -107,14 +92,16 @@ pub trait StringMethods: AsString { let fmt = match fmt { Some(fmt) => fmt, None => { - let Some(idx) = string_ca.first_non_null() else { + if string_ca.first_non_null().is_none() { return Ok( Int32Chunked::full_null(string_ca.name().clone(), string_ca.len()) .into_date(), ); }; - let val = string_ca.get(idx).expect("should not be null"); - sniff_fmt_date(val)? + infer::infer_from_values(string_ca, |val| { + date_pattern(val, NaiveDate::parse_from_str) + }) + .ok_or_else(|| polars_err!(parse_fmt_idk = "date"))? }, }; let ca = unary_elementwise(string_ca, |opt_s| { @@ -154,14 +141,17 @@ pub trait StringMethods: AsString { let fmt = match fmt { Some(fmt) => fmt, None => { - let Some(idx) = string_ca.first_non_null() else { + if string_ca.first_non_null().is_none() { return Ok( Int64Chunked::full_null(string_ca.name().clone(), string_ca.len()) .into_datetime(tu, tz.cloned()), ); }; - let val = string_ca.get(idx).expect("should not be null"); - sniff_fmt_datetime(val)? + infer::infer_from_values(string_ca, |val| { + datetime_pattern(val, NaiveDateTime::parse_from_str) + .or_else(|| datetime_pattern(val, NaiveDate::parse_from_str)) + }) + .ok_or_else(|| polars_err!(parse_fmt_idk = "datetime"))? }, }; diff --git a/py-polars/tests/unit/operations/namespaces/temporal/test_to_datetime.py b/py-polars/tests/unit/operations/namespaces/temporal/test_to_datetime.py index 2b3517399045..666d71105302 100644 --- a/py-polars/tests/unit/operations/namespaces/temporal/test_to_datetime.py +++ b/py-polars/tests/unit/operations/namespaces/temporal/test_to_datetime.py @@ -354,3 +354,48 @@ def test_strptime_uninferrable_format( strict = getattr(pl.col("s").str, method)(strict=True) with pytest.raises(ComputeError, match="could not find an appropriate format"): lf.select(strict).collect(engine=engine) + + +STRPTIME_INFERENCE_CASES = [ + ("to_date", pl.Date, "2020-01-01"), + ("to_datetime", pl.Datetime("us"), "2020-01-01 10:00:00"), + ("to_time", pl.Time, "10:00:00"), +] + + +@pytest.mark.slow +@pytest.mark.parametrize("engine", ["in-memory", "streaming"]) +@pytest.mark.parametrize("position", ["head", "middle", "tail"]) +@pytest.mark.parametrize(("method", "dtype", "good"), STRPTIME_INFERENCE_CASES) +def test_strptime_infers_past_unparseable_values( + engine: EngineType, position: str, method: str, dtype: pl.DataType, good: str +) -> None: + # the frame must span several morsels: inference used to run per morsel, + # off the first value only, nulling whole morsels it could not infer from + n = 200_000 + values = [good] * n + idx = {"head": 0, "middle": n // 2, "tail": n - 1}[position] + values[idx] = "not a temporal value" + + out = ( + pl.LazyFrame({"s": values}) + .select(getattr(pl.col("s").str, method)(strict=False)) + .collect(engine=engine) + ) + assert out.schema["s"] == dtype + assert out.to_series().null_count() == 1 + assert out.to_series()[idx] is None + + +@pytest.mark.slow +@pytest.mark.parametrize("engine", ["in-memory", "streaming"]) +@pytest.mark.parametrize(("method", "dtype", "good"), STRPTIME_INFERENCE_CASES) +def test_strptime_strict_reports_original_column_name( + engine: EngineType, method: str, dtype: pl.DataType, good: str +) -> None: + n = 200_000 + values = ["not a temporal value"] + [good] * n + + strict = getattr(pl.col("s").str, method)(strict=True) + with pytest.raises(InvalidOperationError, match="in column 's'"): + pl.LazyFrame({"s": values}).select(strict).collect(engine=engine) From 8c305be5bf2dddfe99ed31f95649aae549ff3994 Mon Sep 17 00:00:00 2001 From: ritchie46 Date: Wed, 26 Aug 2026 17:23:09 +0200 Subject: [PATCH 4/6] refactor: Collapse duplicated strptime and temporal-cast dispatch `convert_temporal_strings` re-inlined the same Date/Time/Datetime dispatch that `parse_string_as_temporal` performs, so both now call the helper. It returns `Option` rather than `PolarsResult`, which drops an unreachable error arm. The strptime dispatch repeated the same tail -- settle a failed inference, then report the values that failed under `strict` -- in `to_date`, `to_datetime` and `to_time`; that tail moves into `finish_strptime`. Also drops a `match` on `first_non_null()` whose binding is no longer used, for the early-return form the sibling module already uses. No behavior change. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01MsiVh8nEYU5sBSXDcMgGSB --- crates/polars-expr/src/dispatch/strings.rs | 45 +++++++-------- crates/polars-sql/src/sql_expr.rs | 56 ++++++++----------- .../src/chunkedarray/string/infer.rs | 48 ++++++++-------- py-polars/tests/unit/sql/test_cast.py | 28 +++------- 4 files changed, 75 insertions(+), 102 deletions(-) diff --git a/crates/polars-expr/src/dispatch/strings.rs b/crates/polars-expr/src/dispatch/strings.rs index f8e9f42a7c32..21b1725a806b 100644 --- a/crates/polars-expr/src/dispatch/strings.rs +++ b/crates/polars-expr/src/dispatch/strings.rs @@ -466,21 +466,28 @@ pub(super) fn split_regex(s: &[Column], inclusive: bool, strict: bool) -> Polars Ok(out.into_column()) } -/// Without an explicit format, non-strict parsing yields nulls rather than -/// raising when no value in the column allows the format to be inferred. +/// Settle a parse result against `options`: without an explicit format, +/// non-strict parsing yields nulls rather than raising when no value in the +/// column allows the format to be inferred; strict parsing reports the values +/// that failed. #[cfg(feature = "temporal")] -fn null_on_failed_inference( +fn finish_strptime( out: PolarsResult, - s: &Column, + input: &Column, dtype: &DataType, options: &StrptimeOptions, ) -> PolarsResult { - match out { + let out = match out { Err(_) if !options.strict && options.format.is_none() => { - Ok(Column::full_null(s.name().clone(), s.len(), dtype)) + Column::full_null(input.name().clone(), input.len(), dtype) }, - out => out, + out => out?, + }; + + if options.strict && input.null_count() != out.null_count() { + handle_casting_failures(input.as_materialized_series(), out.as_materialized_series())?; } + Ok(out) } #[cfg(feature = "dtype-date")] @@ -492,12 +499,8 @@ fn to_date(s: &Column, options: &StrptimeOptions) -> PolarsResult { ca.as_date_not_exact(options.format.as_deref()) } .map(|ca| ca.into_column()); - let out = null_on_failed_inference(out, s, &DataType::Date, options)?; - if options.strict && ca.null_count() != out.null_count() { - handle_casting_failures(s.as_materialized_series(), out.as_materialized_series())?; - } - Ok(out) + finish_strptime(out, s, &DataType::Date, options) } #[cfg(feature = "dtype-datetime")] @@ -545,13 +548,9 @@ fn to_datetime( ) } .map(|ca| ca.into_column()); - let dtype = DataType::Datetime(*time_unit, time_zone.cloned()); - let out = null_on_failed_inference(out, &s[0], &dtype, options)?; - if options.strict && datetime_strings.null_count() != out.null_count() { - handle_casting_failures(s[0].as_materialized_series(), out.as_materialized_series())?; - } - Ok(out) + let dtype = DataType::Datetime(*time_unit, time_zone.cloned()); + finish_strptime(out, &s[0], &dtype, options) } #[cfg(feature = "dtype-time")] @@ -560,16 +559,12 @@ fn to_time(s: &Column, options: &StrptimeOptions) -> PolarsResult { options.exact, ComputeError: "non-exact not implemented for Time data type" ); - let ca = s.str()?; - let out = ca + let out = s + .str()? .as_time(options.format.as_deref(), options.cache) .map(|ca| ca.into_column()); - let out = null_on_failed_inference(out, s, &DataType::Time, options)?; - if options.strict && ca.null_count() != out.null_count() { - handle_casting_failures(s.as_materialized_series(), out.as_materialized_series())?; - } - Ok(out) + finish_strptime(out, s, &DataType::Time, options) } #[cfg(feature = "concat_str")] diff --git a/crates/polars-sql/src/sql_expr.rs b/crates/polars-sql/src/sql_expr.rs index 64a1b8f3e9b1..f500f34796d7 100644 --- a/crates/polars-sql/src/sql_expr.rs +++ b/crates/polars-sql/src/sql_expr.rs @@ -116,20 +116,21 @@ fn cast_literal_series(s: &Series, dtype: &DataType) -> PolarsResult { s.strict_cast(dtype) } -/// Parse a string expression into `Date`/`Time`/`Datetime` -fn parse_string_as_temporal(expr: Expr, dtype: &DataType, strict: bool) -> PolarsResult { +/// Parse a string expression into `Date`/`Time`/`Datetime`; `None` for any +/// other target dtype. +fn parse_string_as_temporal(expr: Expr, dtype: &DataType, strict: bool) -> Option { let options = StrptimeOptions { strict, ..Default::default() }; - Ok(match dtype { + Some(match dtype { DataType::Date => expr.str().to_date(options), DataType::Time => expr.str().to_time(options), DataType::Datetime(tu, tz) => { expr.str() .to_datetime(Some(*tu), tz.clone(), options, lit("latest")) }, - _ => polars_bail!(SQLInterface: "cannot parse string as {:?}", dtype), + _ => return None, }) } @@ -567,39 +568,27 @@ impl SQLExprVisitor<'_> { }, |dt| dt.as_literal(), ); - match left_dtype { - Some(DataType::Time) if is_iso_time(s) => { - right.clone().str().to_time(StrptimeOptions { - strict: true, - ..Default::default() - }) + let parsed = match left_dtype { + Some(dtype @ DataType::Time) if is_iso_time(s) => { + parse_string_as_temporal(right.clone(), dtype, true) }, - Some(DataType::Date) if is_iso_date(s) => { - right.clone().str().to_date(StrptimeOptions { - strict: true, - ..Default::default() - }) + Some(dtype @ DataType::Date) if is_iso_date(s) => { + parse_string_as_temporal(right.clone(), dtype, true) }, - Some(DataType::Datetime(tu, tz)) if is_iso_datetime(s) || is_iso_date(s) => { - if s.len() == 10 { + Some(dtype @ DataType::Datetime(_, _)) + if is_iso_datetime(s) || is_iso_date(s) => + { + let s = if s.len() == 10 { // handle upcast from ISO date string (10 chars) to datetime - lit(format!("{s}T00:00:00")) + format!("{s}T00:00:00") } else { - lit(s.replacen(' ', "T", 1)) - } - .str() - .to_datetime( - Some(*tu), - tz.clone(), - StrptimeOptions { - strict: true, - ..Default::default() - }, - lit("latest"), - ) + s.replacen(' ', "T", 1) + }; + parse_string_as_temporal(lit(s), dtype, true) }, - _ => right.clone(), - } + _ => None, + }; + parsed.unwrap_or_else(|| right.clone()) } } else { right.clone() @@ -1029,8 +1018,9 @@ impl SQLExprVisitor<'_> { polars_type, DataType::Date | DataType::Time | DataType::Datetime(_, _) ) && self.is_string_expr(&expr) + && let Some(parsed) = parse_string_as_temporal(expr.clone(), &polars_type, strict) { - return parse_string_as_temporal(expr, &polars_type, strict); + return Ok(parsed); } Ok(if strict { expr.strict_cast(polars_type) diff --git a/crates/polars-time/src/chunkedarray/string/infer.rs b/crates/polars-time/src/chunkedarray/string/infer.rs index 76f51629def4..faf76284c4cf 100644 --- a/crates/polars-time/src/chunkedarray/string/infer.rs +++ b/crates/polars-time/src/chunkedarray/string/infer.rs @@ -467,24 +467,23 @@ pub fn to_datetime( // Ensure that the inferred time_zone matches the given time_zone. ensure_matching_time_zone: bool, ) -> PolarsResult { - match ca.first_non_null() { - None => { - Ok(Int64Chunked::full_null(ca.name().clone(), ca.len()).into_datetime(tu, tz.cloned())) - }, - Some(_) => { - let pattern = infer_from_values(ca, infer_pattern_datetime_single) - .ok_or_else(|| polars_err!(parse_fmt_idk = "date"))?; - let mut infer = DatetimeInfer::::try_from_with_unit(pattern, Some(tu))?; - #[cfg(feature = "timezones")] - if matches!(pattern, Pattern::DatetimeYMDZ) { - polars_ensure!( - !ensure_matching_time_zone || tz.is_some(), - to_datetime_tz_mismatch - ); - } - coerce_string_to_datetime(&mut infer, ca, tz, ambiguous) - }, + if ca.first_non_null().is_none() { + return Ok( + Int64Chunked::full_null(ca.name().clone(), ca.len()).into_datetime(tu, tz.cloned()) + ); + } + + let pattern = infer_from_values(ca, infer_pattern_datetime_single) + .ok_or_else(|| polars_err!(parse_fmt_idk = "date"))?; + let mut infer = DatetimeInfer::::try_from_with_unit(pattern, Some(tu))?; + #[cfg(feature = "timezones")] + if matches!(pattern, Pattern::DatetimeYMDZ) { + polars_ensure!( + !ensure_matching_time_zone || tz.is_some(), + to_datetime_tz_mismatch + ); } + coerce_string_to_datetime(&mut infer, ca, tz, ambiguous) } /// Apply a pre-built `DatetimeInfer` to a `StringChunked`, returning a `DateChunked`. #[cfg(feature = "dtype-date")] @@ -534,13 +533,12 @@ pub fn coerce_string_to_datetime( #[cfg(feature = "dtype-date")] pub(crate) fn to_date(ca: &StringChunked) -> PolarsResult { - match ca.first_non_null() { - None => Ok(Int32Chunked::full_null(ca.name().clone(), ca.len()).into_date()), - Some(_) => { - let pattern = infer_from_values(ca, infer_pattern_date_single) - .ok_or_else(|| polars_err!(parse_fmt_idk = "date"))?; - let mut infer = DatetimeInfer::::try_from_with_unit(pattern, None).unwrap(); - coerce_string_to_date(&mut infer, ca) - }, + if ca.first_non_null().is_none() { + return Ok(Int32Chunked::full_null(ca.name().clone(), ca.len()).into_date()); } + + let pattern = infer_from_values(ca, infer_pattern_date_single) + .ok_or_else(|| polars_err!(parse_fmt_idk = "date"))?; + let mut infer = DatetimeInfer::::try_from_with_unit(pattern, None).unwrap(); + coerce_string_to_date(&mut infer, ca) } diff --git a/py-polars/tests/unit/sql/test_cast.py b/py-polars/tests/unit/sql/test_cast.py index 20080194577d..816704bcf423 100644 --- a/py-polars/tests/unit/sql/test_cast.py +++ b/py-polars/tests/unit/sql/test_cast.py @@ -242,25 +242,15 @@ def test_cast_string_to_temporal( ) -> None: df = pl.DataFrame({"s": [value, None]}) - for cast_op in ( - f"CAST(s AS {sql_type})", - f"TRY_CAST(s AS {sql_type})", - f"s::{sql_type}", - ): - res = df.sql(f"SELECT {cast_op} AS x FROM self") - assert_frame_equal( - res, pl.DataFrame({"x": [expected, None]}, schema={"x": dtype}) - ) - - for cast_op in ( - f"CAST('{value}' AS {sql_type})", - f"TRY_CAST('{value}' AS {sql_type})", - f"'{value}'::{sql_type}", - ): - res = df.sql(f"SELECT {cast_op} AS x FROM self") - assert_frame_equal( - res, pl.DataFrame({"x": [expected, expected]}, schema={"x": dtype}) - ) + # a column operand keeps the frame's null; a literal broadcasts to both rows + for operand, rows in (("s", [expected, None]), (f"'{value}'", [expected] * 2)): + for cast_op in ( + f"CAST({operand} AS {sql_type})", + f"TRY_CAST({operand} AS {sql_type})", + f"{operand}::{sql_type}", + ): + res = df.sql(f"SELECT {cast_op} AS x FROM self") + assert_frame_equal(res, pl.DataFrame({"x": rows}, schema={"x": dtype})) @pytest.mark.parametrize( From e0c033f0b51c56e460c00b3f418d19744eec831d Mon Sep 17 00:00:00 2001 From: ritchie46 Date: Wed, 26 Aug 2026 17:42:42 +0200 Subject: [PATCH 5/6] refactor: Spell the all-null check as null_count vs len `first_non_null().is_some()` reads as a search when the predicate is just "not entirely null"; state that directly. `first_non_null` already answers both ends with an integer comparison, so this costs nothing. Also trims comments that narrated the previous behaviour rather than describing the code. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01MsiVh8nEYU5sBSXDcMgGSB --- crates/polars-expr/src/dispatch/strings.rs | 6 ++---- crates/polars-sql/src/sql_expr.rs | 6 ++---- crates/polars-stream/src/nodes/strptime_infer.rs | 7 ++----- crates/polars-stream/src/physical_plan/mod.rs | 3 +-- crates/polars-time/src/chunkedarray/string/infer.rs | 7 ++----- crates/polars-time/src/chunkedarray/string/mod.rs | 12 ++++++------ .../namespaces/temporal/test_to_datetime.py | 3 +-- 7 files changed, 16 insertions(+), 28 deletions(-) diff --git a/crates/polars-expr/src/dispatch/strings.rs b/crates/polars-expr/src/dispatch/strings.rs index 21b1725a806b..99de86c8dbcf 100644 --- a/crates/polars-expr/src/dispatch/strings.rs +++ b/crates/polars-expr/src/dispatch/strings.rs @@ -466,10 +466,8 @@ pub(super) fn split_regex(s: &[Column], inclusive: bool, strict: bool) -> Polars Ok(out.into_column()) } -/// Settle a parse result against `options`: without an explicit format, -/// non-strict parsing yields nulls rather than raising when no value in the -/// column allows the format to be inferred; strict parsing reports the values -/// that failed. +/// Yield nulls when a non-strict parse could not infer a format, and report the +/// values that failed when strict. #[cfg(feature = "temporal")] fn finish_strptime( out: PolarsResult, diff --git a/crates/polars-sql/src/sql_expr.rs b/crates/polars-sql/src/sql_expr.rs index f500f34796d7..d9ab239b12c7 100644 --- a/crates/polars-sql/src/sql_expr.rs +++ b/crates/polars-sql/src/sql_expr.rs @@ -116,8 +116,7 @@ fn cast_literal_series(s: &Series, dtype: &DataType) -> PolarsResult { s.strict_cast(dtype) } -/// Parse a string expression into `Date`/`Time`/`Datetime`; `None` for any -/// other target dtype. +/// Parse a string expression into `Date`/`Time`/`Datetime`; `None` for other dtypes. fn parse_string_as_temporal(expr: Expr, dtype: &DataType, strict: bool) -> Option { let options = StrptimeOptions { strict, @@ -1029,8 +1028,7 @@ impl SQLExprVisitor<'_> { }) } - /// Resolve whether `expr` is known to be `String`-typed against the active - /// schema; false when the dtype cannot be determined. + /// Whether `expr` is known to be `String`; false if the dtype cannot be resolved. fn is_string_expr(&self, expr: &Expr) -> bool { let empty = Schema::default(); let schema = self.active_schema.unwrap_or(&empty); diff --git a/crates/polars-stream/src/nodes/strptime_infer.rs b/crates/polars-stream/src/nodes/strptime_infer.rs index 178d2ae6d505..31aad13c1537 100644 --- a/crates/polars-stream/src/nodes/strptime_infer.rs +++ b/crates/polars-stream/src/nodes/strptime_infer.rs @@ -15,8 +15,7 @@ pub struct StrptimeInferNode { infer: Option, phase: Phase, - /// Name to report parse failures against; the input carries an internal - /// one assigned during lowering. + /// Name to report parse failures against; the input carries an internal one. input_name: PlSmallStr, /// Ambiguous can be `raise`, `earliest`, `latest` and `null`. @@ -65,8 +64,6 @@ impl StrptimeInferNode { } impl FormatInfer { - /// Scans `ca` rather than only its first value; which values are - /// unparseable must not depend on where they sit in the column. fn try_new( ca: &StringChunked, dtype: &DataType, @@ -199,7 +196,7 @@ impl ComputeNode for StrptimeInferNode { if infer_slot.is_none() { let df = morsel.df().await; let ca = df.columns()[0].str()?; - if ca.first_non_null().is_some() { + if ca.null_count() != ca.len() { *infer_slot = FormatInfer::try_new(ca, dtype, options)?; let unit = if matches!(dtype, DataType::Time) { diff --git a/crates/polars-stream/src/physical_plan/mod.rs b/crates/polars-stream/src/physical_plan/mod.rs index a8e1f5fd1789..3ecf95c0c362 100644 --- a/crates/polars-stream/src/physical_plan/mod.rs +++ b/crates/polars-stream/src/physical_plan/mod.rs @@ -285,8 +285,7 @@ pub enum PhysNodeKind { dtype: DataType, options: StrptimeOptions, - /// Name the input had before it was aliased to an internal one; used - /// only to report parse failures against the name the user wrote. + /// Name the input had before lowering aliased it; used in error messages. input_name: PlSmallStr, /// Ambiguous can be `raise`, `earliest`, `latest` and `null`. diff --git a/crates/polars-time/src/chunkedarray/string/infer.rs b/crates/polars-time/src/chunkedarray/string/infer.rs index faf76284c4cf..a4afe96d68eb 100644 --- a/crates/polars-time/src/chunkedarray/string/infer.rs +++ b/crates/polars-time/src/chunkedarray/string/infer.rs @@ -425,9 +425,6 @@ pub fn sniff_time_fmt(val: &str) -> Option<&'static str> { } /// Scan the non-null values for the first that `infer` accepts. -/// -/// Inferring from only the first non-null value makes the result depend on -/// where unparseable values happen to sit; every caller must scan. pub fn infer_from_values(ca: &StringChunked, infer: impl FnMut(&str) -> Option) -> Option { ca.iter().flatten().find_map(infer) } @@ -467,7 +464,7 @@ pub fn to_datetime( // Ensure that the inferred time_zone matches the given time_zone. ensure_matching_time_zone: bool, ) -> PolarsResult { - if ca.first_non_null().is_none() { + if ca.null_count() == ca.len() { return Ok( Int64Chunked::full_null(ca.name().clone(), ca.len()).into_datetime(tu, tz.cloned()) ); @@ -533,7 +530,7 @@ pub fn coerce_string_to_datetime( #[cfg(feature = "dtype-date")] pub(crate) fn to_date(ca: &StringChunked) -> PolarsResult { - if ca.first_non_null().is_none() { + if ca.null_count() == ca.len() { return Ok(Int32Chunked::full_null(ca.name().clone(), ca.len()).into_date()); } diff --git a/crates/polars-time/src/chunkedarray/string/mod.rs b/crates/polars-time/src/chunkedarray/string/mod.rs index f8eb203419f3..2ece7ecc60c5 100644 --- a/crates/polars-time/src/chunkedarray/string/mod.rs +++ b/crates/polars-time/src/chunkedarray/string/mod.rs @@ -58,12 +58,12 @@ pub trait StringMethods: AsString { let fmt = match fmt { Some(fmt) => fmt, None => { - if string_ca.first_non_null().is_none() { + if string_ca.null_count() == string_ca.len() { return Ok( Int64Chunked::full_null(string_ca.name().clone(), string_ca.len()) .into_time(), ); - }; + } infer::infer_from_values(string_ca, |val| { time_pattern(val, NaiveTime::parse_from_str) }) @@ -92,12 +92,12 @@ pub trait StringMethods: AsString { let fmt = match fmt { Some(fmt) => fmt, None => { - if string_ca.first_non_null().is_none() { + if string_ca.null_count() == string_ca.len() { return Ok( Int32Chunked::full_null(string_ca.name().clone(), string_ca.len()) .into_date(), ); - }; + } infer::infer_from_values(string_ca, |val| { date_pattern(val, NaiveDate::parse_from_str) }) @@ -141,12 +141,12 @@ pub trait StringMethods: AsString { let fmt = match fmt { Some(fmt) => fmt, None => { - if string_ca.first_non_null().is_none() { + if string_ca.null_count() == string_ca.len() { return Ok( Int64Chunked::full_null(string_ca.name().clone(), string_ca.len()) .into_datetime(tu, tz.cloned()), ); - }; + } infer::infer_from_values(string_ca, |val| { datetime_pattern(val, NaiveDateTime::parse_from_str) .or_else(|| datetime_pattern(val, NaiveDate::parse_from_str)) diff --git a/py-polars/tests/unit/operations/namespaces/temporal/test_to_datetime.py b/py-polars/tests/unit/operations/namespaces/temporal/test_to_datetime.py index 666d71105302..725b1681f9a3 100644 --- a/py-polars/tests/unit/operations/namespaces/temporal/test_to_datetime.py +++ b/py-polars/tests/unit/operations/namespaces/temporal/test_to_datetime.py @@ -370,8 +370,7 @@ def test_strptime_uninferrable_format( def test_strptime_infers_past_unparseable_values( engine: EngineType, position: str, method: str, dtype: pl.DataType, good: str ) -> None: - # the frame must span several morsels: inference used to run per morsel, - # off the first value only, nulling whole morsels it could not infer from + # must span several morsels; a smaller frame cannot catch this n = 200_000 values = [good] * n idx = {"head": 0, "middle": n // 2, "tail": n - 1}[position] From 1bca93ab5656e056e78c0fab9b53cfdbb92abf04 Mon Sep 17 00:00:00 2001 From: ritchie46 Date: Wed, 26 Aug 2026 18:27:45 +0200 Subject: [PATCH 6/6] c --- .../namespaces/temporal/test_to_datetime.py | 23 ------------------- 1 file changed, 23 deletions(-) diff --git a/py-polars/tests/unit/operations/namespaces/temporal/test_to_datetime.py b/py-polars/tests/unit/operations/namespaces/temporal/test_to_datetime.py index 725b1681f9a3..9c54150a308b 100644 --- a/py-polars/tests/unit/operations/namespaces/temporal/test_to_datetime.py +++ b/py-polars/tests/unit/operations/namespaces/temporal/test_to_datetime.py @@ -363,29 +363,6 @@ def test_strptime_uninferrable_format( ] -@pytest.mark.slow -@pytest.mark.parametrize("engine", ["in-memory", "streaming"]) -@pytest.mark.parametrize("position", ["head", "middle", "tail"]) -@pytest.mark.parametrize(("method", "dtype", "good"), STRPTIME_INFERENCE_CASES) -def test_strptime_infers_past_unparseable_values( - engine: EngineType, position: str, method: str, dtype: pl.DataType, good: str -) -> None: - # must span several morsels; a smaller frame cannot catch this - n = 200_000 - values = [good] * n - idx = {"head": 0, "middle": n // 2, "tail": n - 1}[position] - values[idx] = "not a temporal value" - - out = ( - pl.LazyFrame({"s": values}) - .select(getattr(pl.col("s").str, method)(strict=False)) - .collect(engine=engine) - ) - assert out.schema["s"] == dtype - assert out.to_series().null_count() == 1 - assert out.to_series()[idx] is None - - @pytest.mark.slow @pytest.mark.parametrize("engine", ["in-memory", "streaming"]) @pytest.mark.parametrize(("method", "dtype", "good"), STRPTIME_INFERENCE_CASES)