From a7658777a19fd5e4ac4973934427201e35166dc6 Mon Sep 17 00:00:00 2001 From: Moritz Hoffmann Date: Fri, 16 Oct 2026 13:47:51 +0200 Subject: [PATCH 1/2] expr: Evaluate binary function arguments through a named iterator The blanket `LazyBinaryFunc` impl evaluated both arguments into an array of results before handing them to `InputDatumType::try_from_iter`. This made every binary function evaluation about 12% slower on the `case_literal/if_chain` benchmarks. A named iterator that evaluates each argument on demand restores the previous speed and keeps a single iterator type per `Eval` implementor, so the generated code size stays the same. Closes: CPU-315 Co-Authored-By: Claude Opus 5.5 --- src/expr/src/scalar/func/binary.rs | 33 +++++++++++++++++++++++------- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/src/expr/src/scalar/func/binary.rs b/src/expr/src/scalar/func/binary.rs index 0937d97ec6bf7..6f617fc1e5c70 100644 --- a/src/expr/src/scalar/func/binary.rs +++ b/src/expr/src/scalar/func/binary.rs @@ -140,6 +140,23 @@ pub(crate) trait EagerBinaryFunc { } } +/// Evaluates the arguments of a binary function one at a time, in order. +struct EvalArgs<'a, 'd, E> { + exprs: std::array::IntoIter<&'a E, 2>, + datums: &'d [Datum<'a>], + temp_storage: &'a RowArena, +} + +impl<'a, 'd, E: Eval> Iterator for EvalArgs<'a, 'd, E> { + type Item = Result, EvalError>; + #[inline] + fn next(&mut self) -> Option { + self.exprs + .next() + .map(|expr| expr.eval(self.datums, self.temp_storage)) + } +} + impl LazyBinaryFunc for T { fn eval<'a>( &'a self, @@ -154,13 +171,15 @@ impl LazyBinaryFunc for T { }; // `try_from_iter` is generic over the iterator, so a closure over `exprs` gives // it a fresh type per function and per `Eval` implementor, and the unwrapping is - // emitted once for each. Passing the arguments through an array keeps the - // iterator type the same everywhere, so it is emitted once per input type. - let mut datums = [ - expr_a.eval(datums, temp_storage), - expr_b.eval(datums, temp_storage), - ] - .into_iter(); + // emitted once for each. A named iterator type is shared by all functions, so it + // is emitted once per input type and `Eval` implementor. Evaluating both + // arguments into an array first has the same instantiation count but measured + // about 12% slower on `case_literal/if_chain`. + let mut datums = EvalArgs { + exprs: [*expr_a, *expr_b].into_iter(), + datums, + temp_storage, + }; let input = match T::Input::try_from_iter(&mut datums) { // If we can convert to the input type then we call the function Ok(input) => input, From 6ef55d7b99f948595f4ca016eeb422151a6afab8 Mon Sep 17 00:00:00 2001 From: Moritz Hoffmann Date: Mon, 19 Oct 2026 09:59:21 +0200 Subject: [PATCH 2/2] test: Pin that an eval error wins over a NULL in binary functions Co-Authored-By: Claude Opus 5.5 --- test/sqllogictest/error_semantics.slt | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/sqllogictest/error_semantics.slt b/test/sqllogictest/error_semantics.slt index 48cb8f815bdd0..b0b14770c9c1c 100644 --- a/test/sqllogictest/error_semantics.slt +++ b/test/sqllogictest/error_semantics.slt @@ -87,6 +87,21 @@ select coalesce(a, a/b + 1, a/b + 2) from test; ---- 1 +# A binary function evaluates both arguments before propagating a NULL, so an error in +# either argument wins over a NULL in the other. The NULL comes from a column so the +# optimizer cannot fold it away. +statement ok +create table null_and_err (a int, b int); + +statement ok +insert into null_and_err values (NULL, 0); + +query error Evaluation error: division by zero +select a + 1/b from null_and_err; + +query error Evaluation error: division by zero +select 1/b + a from null_and_err; + # The following two results are probably wrong (but depends on what error semantics we agree on later). query error Evaluation error: division by zero select coalesce(a, (select a/b from test)) from test;