Remember a failed evaluation in the applyFunction cache and renumber the test

Address review: after a throw, `applyFunction` left no cache entry behind, so every
later range of the part asking about the same (function, column) pair re-ran the
whole-column execution and threw again before `applyMonotonicFunctionsChainToRange`
turned it into "unknown". The entry is now published as a failure sentinel (a null
column under the pair's name) and the lookup short-circuits on it: `applyFunction`
returns `std::nullopt`, and the caller answers "unknown" for the range at once.

Measured on a 64-mark part with `index_granularity = 1` (`system.errors` for
`ILLEGAL_DIVISION`): 69 throws per query before, 1 after; 138 vs 2 for two atoms.

The test moves from `05086` (taken on master) to `05227`.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

src/Storages/MergeTree/KeyCondition.cpp

@@ -1650,12 +1650,16 @@ static Field applyFunctionForField(
 }
 
 /// applyFunction will execute the function with one `field` or the column which `field` refers to.
-static FieldRef applyFunction(const FunctionBasePtr & func, const DataTypePtr & current_type, const FieldRef & field)
+///
+/// Returns `std::nullopt` when an earlier call already failed to evaluate the same (function, column)
+/// pair: the cache remembers the failure, so the caller can answer "unknown" for the range at once
+/// instead of re-running the throwing expression over the whole column.
+static std::optional<FieldRef> applyFunction(const FunctionBasePtr & func, const DataTypePtr & current_type, const FieldRef & field)
 {
     chassert(func != nullptr);
     /// Fallback for fields without block reference.
     if (field.isExplicit())
-        return applyFunctionForField(func, current_type, field);
+        return FieldRef(applyFunctionForField(func, current_type, field));
 
     /// We will cache the function result inside `field.columns`, because this function will call many times
     /// from many fields from same column. When the column is huge, for example there are thousands of marks, we need a cache.
@@ -1674,7 +1678,15 @@ static FieldRef applyFunction(const FunctionBasePtr & func, const DataTypePtr &
             result_idx = i;
     }
 
-    if (result_idx == columns->size())
+    if (result_idx < columns->size())
+    {
+        /// The entry is a failure sentinel: a previous evaluation of this pair threw, and the cache is
+        /// shared by every range of the part, so every later range asking about it gets the same
+        /// "unknown" without paying for the whole-column execution and the exception again.
+        if (!(*columns)[result_idx].column)
+            return std::nullopt;
+    }
+    else
     {
         /// When cache is missed, we calculate the whole column where the field comes from. This will avoid repeated calculation.
         ColumnsWithTypeAndName args{(*columns)[field.column_idx]};
@@ -1690,14 +1702,25 @@ static FieldRef applyFunction(const FunctionBasePtr & func, const DataTypePtr &
         /// keeps this function's own result type and representation.
         ///
         /// Compute before publishing the cache entry. The function is evaluated on values the analysis
-        /// substitutes, so it can fail for one of them - `intDiv(1, a - 1)` divides by zero at `a = 1` -
-        /// and a placeholder entry with a null column left behind by a throw would be found by the
-        /// lookup above on the next call for the same (function, column) pair and dereferenced.
-        auto result_column = func->execute(args, func->getResultType(), args.front().column->size(), /* dry_run = */ false);
+        /// substitutes, so it can fail for one of them - `intDiv(1, a - 1)` divides by zero at `a = 1`.
+        /// A throw publishes a sentinel entry with a null column instead of the result, so that the
+        /// lookup above short-circuits every later call for the same (function, column) pair. The
+        /// caller decides which exceptions it may swallow; the ones it rethrows abort the query, and
+        /// the sentinel they leave behind is never consulted.
+        ColumnPtr result_column;
+        try
+        {
+            result_column = func->execute(args, func->getResultType(), args.front().column->size(), /* dry_run = */ false);
+        }
+        catch (...)
+        {
+            field.columns->emplace_back(ColumnWithTypeAndName{nullptr, func->getResultType(), result_name});
+            throw;
+        }
         field.columns->emplace_back(ColumnWithTypeAndName{result_column, func->getResultType(), result_name});
     }
 
-    return {field.columns, field.row_idx, result_idx};
+    return FieldRef(field.columns, field.row_idx, result_idx);
 }
 
 /// Sequentially applies functions to the column, returns `true`
@@ -5736,13 +5759,19 @@ std::optional<Range> KeyCondition::applyMonotonicFunctionsChainToRange(
             {
                 if (!key_range.left.isNull())
                 {
-                    key_range.left = applyFunction(func, current_type, key_range.left);
+                    auto transformed = applyFunction(func, current_type, key_range.left);
+                    if (!transformed)
+                        return {};
+                    key_range.left = std::move(*transformed);
                     key_range.left_included = true;
                 }
 
                 if (!key_range.right.isNull())
                 {
-                    key_range.right = applyFunction(func, current_type, key_range.right);
+                    auto transformed = applyFunction(func, current_type, key_range.right);
+                    if (!transformed)
+                        return {};
+                    key_range.right = std::move(*transformed);
                     key_range.right_included = true;
                 }
             }

tests/queries/0_stateless/05227_partition_pruning_unevaluable_subexpression.sql

@@ -25,8 +25,10 @@ SELECT 'repeated failing chain';
 -- The transformed key is cached per `(function, index column)` and the entry is shared by every
 -- range of a part, so a range whose endpoint cannot be evaluated is followed by ranges that look
 -- that entry up. Publishing the entry before computing it left a null column behind on the way out,
--- and the next lookup dereferenced it. Two atoms over the same chain, and a primary key that puts
--- the failing boundary in the middle of many ranges, exercise that lookup.
+-- and the next lookup dereferenced it. Now the entry left behind by a throw is a failure sentinel,
+-- so every later range of the part gets "unknown" from the cache instead of re-running the
+-- throwing expression over the whole column. Two atoms over the same chain, and a primary key that
+-- puts the failing boundary in the middle of many ranges, exercise that lookup.
 SELECT count() FROM t_pruning_eval_error WHERE p != 1 AND intDiv(1, p - 1) > 0 AND intDiv(1, p - 1) < 100 SETTINGS use_skip_indexes = 0;
 DROP TABLE IF EXISTS t_pruning_eval_error_repeat;
 CREATE TABLE t_pruning_eval_error_repeat (k Int64) ENGINE = MergeTree ORDER BY k SETTINGS index_granularity = 1;