From 16994edd214e384fa079a53c2f4e81694d75a56e Mon Sep 17 00:00:00 2001 From: Moritz Hoffmann Date: Wed, 23 Sep 2026 21:32:29 +0200 Subject: [PATCH] expr: Convert CastRangeToString to the sqlfunc macro The impl was hand-written because it allocates its result, which the eager trait now covers: a `String` output is pushed into the arena by `OutputDatumType::into_result`, exactly as the body did by hand. The body also checked NULL itself, and under the macro that check has to come from the input type. The input is `ExcludeNull>`, because `stringify_datum` takes a raw `Datum` and a bare `Datum` accepts NULL. `ExcludeNull` reports `nullable() == false`, so the derived `propagates_nulls` is true, matching the impl's declaration, and its `try_from_result` turns `Datum::Null` into `Err(Ok(Datum::Null))`, which the evaluation layer returns as `Ok(Datum::Null)`. That is the manual check, so the manual check is gone. `introduces_nulls` derives false from the non-`Option` `String` inside the `Result`, matching the declaration. Per-override accounting: `eval` becomes the function body minus the NULL check. `output_sql_type` returned `String.nullable(input_type.nullable)`, which is what the derived `String` output plus the unary tail gives for a non-nullable input, so no output modifier is needed. `preserves_uniqueness = true` carries over. `propagates_nulls` and `introduces_nulls` are derived as above. `could_error` was not overridden and inherited `LazyUnaryFunc`'s conservative `true`, which the fallible `Result` output derives. `is_monotone` and `is_eliminable_cast` returned the trait defaults and are dropped, as is `inverse`, whose `TODO?` about `typeconv` now sits on the function. `Display` wrote a fixed string, so `sqlname` reproduces it. With the impl gone the file has no `Display` left, so `std::fmt` and the now unused `Eval`, `LazyUnaryFunc`, `RowArena` and `SqlColumnType` imports go with it. The registry reports only `sqlfunc_signature` moving, on the one record. Co-Authored-By: Claude Opus 5 (1M context) --- .../tests/snapshots/func_registry.json | 2 +- .../snapshots/func_registry_digests.json | 2 +- .../tests/snapshots/func_registry_source.json | 2 +- src/expr/src/scalar/func.rs | 1 - src/expr/src/scalar/func/impls/range.rs | 64 +++---------------- 5 files changed, 12 insertions(+), 59 deletions(-) diff --git a/src/compute-types/tests/snapshots/func_registry.json b/src/compute-types/tests/snapshots/func_registry.json index 275f28a81ef21..f9de3fd89ea72 100644 --- a/src/compute-types/tests/snapshots/func_registry.json +++ b/src/compute-types/tests/snapshots/func_registry.json @@ -360,7 +360,7 @@ "cast_pg_legacy_char_to_int32": {"variant":"CastPgLegacyCharToInt32","propagates_nulls":true,"introduces_nulls":false,"could_error":false,"preserves_uniqueness":true,"is_monotone":false,"is_eliminable_cast":false,"inverse":"cast_int32_to_pg_legacy_char","input_types":["PgLegacyChar:NotNull"],"output_type":"Int32:NotNull","sqlfunc_signature":"fn(PgLegacyChar) -> i32"}, "cast_pg_legacy_char_to_string": {"variant":"CastPgLegacyCharToString","propagates_nulls":true,"introduces_nulls":false,"could_error":true,"preserves_uniqueness":true,"is_monotone":false,"is_eliminable_cast":false,"inverse":"cast_string_to_pg_legacy_char","input_types":["PgLegacyChar:NotNull"],"output_type":"String:NotNull","sqlfunc_signature":"fn(PgLegacyChar) -> Result"}, "cast_pg_legacy_char_to_var_char": {"variant":"CastPgLegacyCharToVarChar","propagates_nulls":true,"introduces_nulls":false,"could_error":true,"preserves_uniqueness":true,"is_monotone":false,"is_eliminable_cast":false,"inverse":"cast_string_to_var_char","input_types":["PgLegacyChar:NotNull"],"output_type":"VarChar { max_length: None }:NotNull","sqlfunc_signature":"fn(PgLegacyChar) -> Result, EvalError>"}, - "cast_range_to_string": {"variant":"CastRangeToString","propagates_nulls":true,"introduces_nulls":false,"could_error":true,"preserves_uniqueness":true,"is_monotone":false,"is_eliminable_cast":false,"inverse":null,"input_types":["Range { element_type: Int32 }:NotNull"],"output_type":"String:NotNull","sqlfunc_signature":null}, + "cast_range_to_string": {"variant":"CastRangeToString","propagates_nulls":true,"introduces_nulls":false,"could_error":true,"preserves_uniqueness":true,"is_monotone":false,"is_eliminable_cast":false,"inverse":null,"input_types":["Range { element_type: Int32 }:NotNull"],"output_type":"String:NotNull","sqlfunc_signature":"fn(ExcludeNull>) -> Result"}, "cast_record1_to_record2": {"variant":"CastRecord1ToRecord2","propagates_nulls":true,"introduces_nulls":false,"could_error":true,"preserves_uniqueness":false,"is_monotone":false,"is_eliminable_cast":false,"inverse":null,"input_types":["Record { fields: [(ColumnName(\"a\"), SqlColumnType { scalar_type: Int32, nullable: false }), (ColumnName(\"b\"), SqlColumnType { scalar_type: String, nullable: true })], custom_id: None }:NotNull"],"output_type":"Record { fields: [(ColumnName(\"a\"), SqlColumnType { scalar_type: Int32, nullable: false }), (ColumnName(\"b\"), SqlColumnType { scalar_type: String, nullable: true })], custom_id: None }:NotNull","sqlfunc_signature":null}, "cast_record_to_string": {"variant":"CastRecordToString","propagates_nulls":true,"introduces_nulls":false,"could_error":true,"preserves_uniqueness":true,"is_monotone":false,"is_eliminable_cast":false,"inverse":null,"input_types":["Record { fields: [(ColumnName(\"a\"), SqlColumnType { scalar_type: Int32, nullable: false }), (ColumnName(\"b\"), SqlColumnType { scalar_type: String, nullable: true })], custom_id: None }:NotNull"],"output_type":"String:NotNull","sqlfunc_signature":null}, "cast_reg_class_to_oid": {"variant":"CastRegClassToOid","propagates_nulls":true,"introduces_nulls":false,"could_error":false,"preserves_uniqueness":true,"is_monotone":false,"is_eliminable_cast":false,"inverse":"cast_oid_to_reg_class","input_types":["RegClass:NotNull"],"output_type":"Oid:NotNull","sqlfunc_signature":"fn(RegClass) -> Oid"}, diff --git a/src/compute-types/tests/snapshots/func_registry_digests.json b/src/compute-types/tests/snapshots/func_registry_digests.json index 7bb90ffb39c26..aa62ea47967fe 100644 --- a/src/compute-types/tests/snapshots/func_registry_digests.json +++ b/src/compute-types/tests/snapshots/func_registry_digests.json @@ -1,3 +1,3 @@ { - "1": "sha256:1813d7979ea54244af7ca66259984733b1b6b77865264c479337ce36ce4d2da8" + "1": "sha256:e1d5a5f3547a40492745310bbbdb515dd6eaf99a3c2c2bccf10edbbf1317e9f9" } diff --git a/src/compute-types/tests/snapshots/func_registry_source.json b/src/compute-types/tests/snapshots/func_registry_source.json index 676b1b80497a6..bcba52ada46e7 100644 --- a/src/compute-types/tests/snapshots/func_registry_source.json +++ b/src/compute-types/tests/snapshots/func_registry_source.json @@ -360,7 +360,7 @@ "cast_pg_legacy_char_to_int32": {"display":"\"char\"_to_integer","sqlfunc_decl":"#[sqlfunc(sqlname = \"\\\"char\\\"_to_integer\", preserves_uniqueness = true, inverse = super::CastInt32ToPgLegacyChar)] fn cast_pg_legacy_char_to_int32(a: PgLegacyChar) -> i32","body_fingerprint":"1976be6ad62975f3"}, "cast_pg_legacy_char_to_string": {"display":"\"char\"_to_text","sqlfunc_decl":"#[sqlfunc(sqlname = \"\\\"char\\\"_to_text\", preserves_uniqueness = true, inverse = super::CastStringToPgLegacyChar)] fn cast_pg_legacy_char_to_string(a: PgLegacyChar) -> Result","body_fingerprint":"a6ca26834f5d02bc"}, "cast_pg_legacy_char_to_var_char": {"display":"\"char\"_to_varchar","sqlfunc_decl":"#[sqlfunc(sqlname = \"\\\"char\\\"_to_varchar\", preserves_uniqueness = true, inverse = super::CastStringToVarChar {fail_on_len: false, length: Some(VarCharMaxLength::try_from(1).unwrap()),})] fn cast_pg_legacy_char_to_var_char(a: PgLegacyChar) -> Result, EvalError>","body_fingerprint":"757b2f161f95b71c"}, - "cast_range_to_string": {"display":"rangetostr","sqlfunc_decl":null,"body_fingerprint":null}, + "cast_range_to_string": {"display":"rangetostr","sqlfunc_decl":"#[sqlfunc(CastRangeToString, sqlname = \"rangetostr\", preserves_uniqueness = true)] fn cast_range_to_string<'a>(&self, a: ExcludeNull>) -> Result","body_fingerprint":"57de61553fc8ac26"}, "cast_record1_to_record2": {"display":"record1torecord2","sqlfunc_decl":null,"body_fingerprint":null}, "cast_record_to_string": {"display":"recordtostr","sqlfunc_decl":null,"body_fingerprint":null}, "cast_reg_class_to_oid": {"display":"regclasstooid","sqlfunc_decl":"#[sqlfunc(sqlname = \"regclasstooid\", preserves_uniqueness = true, inverse = super::CastOidToRegClass)] fn cast_reg_class_to_oid(a: RegClass) -> Oid","body_fingerprint":"0c8b6d41527aa7c1"}, diff --git a/src/expr/src/scalar/func.rs b/src/expr/src/scalar/func.rs index 75dd572a1302c..da6c2df7adb5e 100644 --- a/src/expr/src/scalar/func.rs +++ b/src/expr/src/scalar/func.rs @@ -142,7 +142,6 @@ func_name! { CastListToJsonb => "cast_list_to_jsonb", CastListToString => "cast_list_to_string", CastMapToString => "cast_map_to_string", - CastRangeToString => "cast_range_to_string", CastRecord1ToRecord2 => "cast_record1_to_record2", CastRecordToString => "cast_record_to_string", CastStringToArray => "cast_string_to_array", diff --git a/src/expr/src/scalar/func/impls/range.rs b/src/expr/src/scalar/func/impls/range.rs index 6355d6d568d4e..64ebeb3472c58 100644 --- a/src/expr/src/scalar/func/impls/range.rs +++ b/src/expr/src/scalar/func/impls/range.rs @@ -7,15 +7,13 @@ // the Business Source License, use of this software will be governed // by the Apache License, Version 2.0. -use std::fmt; - use mz_expr_derive::sqlfunc; use mz_repr::adt::range::Range; -use mz_repr::{Datum, RowArena, SqlColumnType, SqlScalarType}; +use mz_repr::{Datum, ExcludeNull, SqlScalarType}; use serde::{Deserialize, Serialize}; -use crate::scalar::func::{LazyUnaryFunc, stringify_datum}; -use crate::{Eval, EvalError}; +use crate::EvalError; +use crate::scalar::func::stringify_datum; #[derive( Ord, @@ -32,56 +30,12 @@ pub struct CastRangeToString { pub ty: SqlScalarType, } -impl LazyUnaryFunc for CastRangeToString { - fn eval<'a>( - &'a self, - datums: &[Datum<'a>], - temp_storage: &'a RowArena, - a: &'a impl Eval, - ) -> Result, EvalError> { - let a = a.eval(datums, temp_storage)?; - if a.is_null() { - return Ok(Datum::Null); - } - let mut buf = String::new(); - stringify_datum(&mut buf, a, &self.ty)?; - Ok(Datum::String(temp_storage.push_string(buf))) - } - - fn output_sql_type(&self, input_type: SqlColumnType) -> SqlColumnType { - SqlScalarType::String.nullable(input_type.nullable) - } - - fn propagates_nulls(&self) -> bool { - true - } - - fn introduces_nulls(&self) -> bool { - false - } - - fn preserves_uniqueness(&self) -> bool { - true - } - - fn inverse(&self) -> Option { - // TODO? if typeconv was in expr, we could determine this - None - } - - fn is_monotone(&self) -> bool { - false - } - - fn is_eliminable_cast(&self) -> bool { - false - } -} - -impl fmt::Display for CastRangeToString { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - f.write_str("rangetostr") - } +// TODO? if typeconv was in expr, we could determine the inverse of this cast +#[sqlfunc(CastRangeToString, sqlname = "rangetostr", preserves_uniqueness = true)] +fn cast_range_to_string<'a>(&self, a: ExcludeNull>) -> Result { + let mut buf = String::new(); + stringify_datum(&mut buf, *a, &self.ty)?; + Ok(buf) } // The monotone claim survives this function mapping empty and