R13 slice 3: one evaluator construction path, case-insensitive mode honoured

docs/ENGINE_REFACTORING.md

@@ -472,7 +472,8 @@ feature work**, and so we can tell the difference between "this is awkward" and
 
 ### R13 — Two expression evaluators, every boolean operator implemented twice
 - **Status:** 🟡 IN PROGRESS — filed 2026-09-13 out of [P46](SQL_PARITY.md#p46);
-  **slices 1 and 2 done 2026-09-13**, slice 3 next.
+  **slices 1 and 2 done 2026-09-13, slice 3 (with 3b) done 2026-09-14**, slice 4
+  next.
   **The active workstream:** parity fixes that touch expression evaluation land
   as slices of this entry, not as patches.
 - **Where:** `src/data/arithmetic_evaluator.rs` (`ArithmeticEvaluator`, value →
@@ -565,6 +566,37 @@ feature work**, and so we can tell the difference between "this is awkward" and
   (both NULL → not-true, already correct), so nothing downstream had to change.
   **Two evaluators now agree on all 36 predicates, but there are still two** —
   slices 3–6 are what make that agreement structural rather than coincidental.
+- **Slice 3 result (2026-09-14).** Two no-op commits, then a pinned semantic
+  one (3b), each checked against a byte-identical parity report.
+  - *Registries built once.* The function, both aggregate and the window
+    registries are immutable after construction, so a process-wide `OnceLock`
+    builds them and every `ArithmeticEvaluator` takes `Arc` clones. GROUP BY was
+    paying for it per group and per aggregate: over `trades_100000.csv`, net of
+    ~3 s load, a high-cardinality GROUP BY went 3.4 s → 1.9 s and GROUP BY +
+    HAVING 2.5 s → 0.8 s. `estimate_group_cardinality` also built one per row.
+  - *One constructor.* `new` / `with_date_notation` /
+    `with_date_notation_and_registry` differed only by a date-notation string
+    stored in a field **nothing read** — dates parse through the global
+    notation. So "pass date notation in" turned out to be "delete it": the
+    field, the constructors, `apply_group_by_expressions`' parameter and
+    `QueryEngine`'s own unread field went. Table aliases needed nothing:
+    alias-qualified operands already resolve in every clause probed.
+  - *3b — case-insensitivity.* The one context gap with a visible answer: under
+    `--case-insensitive`, `WHERE status = 'ACTIVE'` matched `Active` but
+    `SELECT status = 'ACTIVE'`, HAVING, aggregate arguments and CASE WHEN did
+    not — nor WHERE itself once the predicate sat under a CASE, which WHERE
+    hands to its inner value evaluator. Slice 1 had not pinned it (the parity
+    harness cannot: DuckDB has no such mode), so 3b pinned first — a
+    case-insensitive run of the matrix (10 predicates, expectations from DuckDB
+    with `COLLATE NOCASE` / `ILIKE`) plus five clause-level cases through
+    `QueryEngine::with_case_insensitive(true)` — recording 11 + 5 divergences.
+    The fix, `ArithmeticEvaluator::with_case_insensitive` handed in at all 16
+    construction sites (comparisons, `IN`, `BETWEEN`, `LIKE`), flipped exactly
+    those and nothing else; parity report byte-identical.
+  - *Still case-sensitive outside WHERE, not pinned:* simple
+    `CASE x WHEN 'v'` (its own `values_equal`, with its own numeric rules — goes
+    with slice 4) and method calls such as `.Contains()` routed to registry
+    functions (slice 5).
 - **Slices, in the R10 pattern — no-op slices kept apart from the one that
   changes answers:**
   1. ✅ **Pin the divergences, no engine change.** *(Done 2026-09-13.)* A per-operator matrix run through
@@ -579,7 +611,7 @@ feature work**, and so we can tell the difference between "this is awkward" and
      false for predicates. Closes [P48](SQL_PARITY.md#p48) and the HAVING / SELECT
      rows above. Check `CASE WHEN` (UNKNOWN takes ELSE) and every `to_bool` /
      `is_truthy` caller. Acceptance: exactly the slice-1 cases flip, nothing else.
-  3. **One construction path.** Registries built once and shared (`Arc`), context
+  3. ✅ **One construction path.** *(Done 2026-09-14 — see result above.)* Registries built once and shared (`Arc`), context
      — case sensitivity, date notation, table aliases — passed in rather than
      defaulted per site. No-op; acceptance is parity *exactly* unchanged, and it
      fixes the case-insensitivity gap as a side effect only if slice 1 pinned it.
@@ -630,7 +662,7 @@ R8 legacy WHERE ──── independent; stage 2 is self-contained, do it in a
 R10 Trilean ──────── DONE; closed P18/P19 (parity 125 → 129)
 R11 ORDER BY resolver ─ independent; small, but a behaviour change — wants its own parity run
 R12 aggregate registries ─ independent; step 1 is a provable no-op, do it before the next aggregate fix
-R13 one evaluator ─── ACTIVE from 2026-09-13; slices 1 (pin) and 2 (3VL) DONE → 3 (construction) → 4–6 (retire WHERE arms)
+R13 one evaluator ─── ACTIVE from 2026-09-13; slices 1 (pin), 2 (3VL), 3 (construction + case mode) DONE → 4–6 (retire WHERE arms)
 ```
 
 **A note on ordering, from the P18/P19 work being next.** The WHERE evaluator
@@ -670,3 +702,4 @@ AGREE count — which makes it safe to land well before the semantics change.
 | 2026-09-13 | R13 filed and made the active workstream: two evaluators with divergent NULL semantics, three truth collapses. Probing found four live clause-dependent divergences (HAVING, SELECT `IN`, JOIN NULL keys, P48) | — |
 | 2026-09-13 | R13 slice 1: evaluator matrix (36 predicates × both evaluators × 5 NULL-bearing rows, DuckDB-derived expectations). WHERE 36/36, value evaluator 4/36. P48 widened; P50–P52 filed; six corpus cases, AGREE unchanged | — |
 | 2026-09-13 | R13 slice 2: value evaluator three-valued; `Trilean::from_value`/`to_value`; `to_bool` removed. Matrix value column 4/36 → 36/36 in one change. Closes P48, P50 — 168 → **174 AGREE** | — |
+| 2026-09-14 | R13 slice 3: registries built once per process; one `ArithmeticEvaluator` constructor, dead date-notation plumbing deleted — parity byte-identical, high-cardinality GROUP BY ~2× faster. 3b: case-insensitive mode pinned (11 matrix + 5 clause cases) then handed in at every construction site; all flipped, parity byte-identical | — |

docs/SQL_PARITY.md

@@ -92,7 +92,7 @@ Suggested fix order, by silent blast radius:
 | ~~9d~~ | ~~[P37](#p37) window in `WHERE` returns 0 rows~~ | ✅ **Fixed 2026-09-05** — corpus count unchanged, and that is the finding: the case is `OURS_ONLY` before *and* after, so the harness cannot see this fix or a future regression of it (first entry of that kind — the regression test is a Rust module). The filed root cause was wrong: `ExpressionLifter` *does* lift from `WHERE`. The real defect was one arm in the WHERE evaluator answering FALSE for any bare value used as a predicate — `WHERE true` returned zero rows too. It did **not** close [P15](#p15), which needs the opposite change |
 | ~~9e~~ | ~~[P41](#p41) `MODE` tie-break is random per run~~ | ✅ **Fixed 2026-09-06** — 152 → **156 AGREE** (four new cases). Small, as predicted, but not where it was filed: the named `ModeState` was a *shadowed* implementation and fixing it moved nothing. Reference does specify a rule and it is **first-occurrence**, not the "smallest value wins" this row proposed. Unblocked both example files, now FORMAL. Spun off [P42](#p42), [P43](#p43), [R12](ENGINE_REFACTORING.md#r12) |
 | ~~9g~~ | ~~[P46](#p46) column-vs-column `WHERE` returns 0 rows~~ | ✅ **Fixed 2026-09-13** — 157 → **168 AGREE**. Wider than filed: *every* non-literal right-hand operand read as NULL (comparisons, `IN` items, `BETWEEN` bounds), and a literal on the left errored (`WHERE 1=0`). One resolver for all operands closed it. Did **not** need [P48](#p48) alongside, as this row predicted — the fix kept the comparison in the WHERE evaluator. Spun off [P49](#p49) (correlated outer refs bind to the inner table), accepted knowingly |
-| **NEXT** | [R13](ENGINE_REFACTORING.md#r13) one expression evaluator — **the active workstream from 2026-09-13** | **Slices 1–2 done 2026-09-13** (value evaluator three-valued, 168 → 174 AGREE, closed P48/P50); slice 3 (one construction path) next. Not a finding but the reason several are cheap to fix only once. WHERE and the value evaluator implement every boolean operator separately with different NULL rules, so the same predicate answers differently in WHERE, HAVING, SELECT and JOIN ON. **Working rule:** a parity fix that touches expression evaluation lands as an R13 slice, not a patch. [P52](#p52) (NULL join keys) is slice 6 |
+| **NEXT** | [R13](ENGINE_REFACTORING.md#r13) one expression evaluator — **the active workstream from 2026-09-13** | **Slices 1–2 done 2026-09-13** (value evaluator three-valued, 168 → 174 AGREE, closed P48/P50); **slice 3 done 2026-09-14** (one construction path; case-insensitive mode now honoured outside WHERE, parity unmoved); slice 4 (WHERE arms delegate, one operator per commit) next. Not a finding but the reason several are cheap to fix only once. WHERE and the value evaluator implement every boolean operator separately with different NULL rules, so the same predicate answers differently in WHERE, HAVING, SELECT and JOIN ON. **Working rule:** a parity fix that touches expression evaluation lands as an R13 slice, not a patch. [P52](#p52) (NULL join keys) is slice 6 |
 | then | [P47](#p47) `MIN`/`MAX` ranked by type | Silent and cheap, and outside the evaluator (aggregate registries — confirm the live one first, [R12](ENGINE_REFACTORING.md#r12)), so it can land alongside R13 without breaking the working rule |
 | then | [P14](#p14), [P20](#p20), [P23](#p23) | Smaller, self-contained, decisions already taken. Was row 9b, then NEXT until the 2026-09-12 findings displaced it. **Check each against the R13 working rule first** — P20 (`\|\|` with NULL) is an operator in the value evaluator |
 | 9f | [P42](#p42) `MODE` is numeric-only | Companion to [R12](ENGINE_REFACTORING.md#r12), and cheap if taken with it: the shadowed implementation already handles non-numerics and preserves type, so the fix is largely to stop the live path throwing away what it knows. Also buys the corpus its clearest tie-break case |

src/data/arithmetic_evaluator.rs

@@ -1,4 +1,3 @@
-use crate::config::global::get_date_notation;
 use crate::data::data_view::DataView;
 use crate::data::datatable::{DataTable, DataValue};
 use crate::data::trilean::Trilean;
@@ -12,52 +11,58 @@ use crate::sql::window_context::WindowContext;
 use crate::sql::window_functions::{ExpressionEvaluator, WindowFunctionRegistry};
 use anyhow::{anyhow, Result};
 use std::collections::{HashMap, HashSet};
-use std::sync::Arc;
+use std::sync::{Arc, OnceLock};
 use std::time::Instant;
 use tracing::{debug, info};
 
+/// The registries every evaluator reads. None of them change after
+/// construction, so the process builds them once and every evaluator shares
+/// them, rather than each construction site rebuilding all four (R13 slice 3).
+struct EvaluatorRegistries {
+    function: Arc<FunctionRegistry>,
+    aggregate: Arc<AggregateRegistry>,
+    new_aggregate: Arc<AggregateFunctionRegistry>,
+    window: Arc<WindowFunctionRegistry>,
+}
+
+fn shared_registries() -> &'static EvaluatorRegistries {
+    static REGISTRIES: OnceLock<EvaluatorRegistries> = OnceLock::new();
+    REGISTRIES.get_or_init(|| EvaluatorRegistries {
+        function: Arc::new(FunctionRegistry::new()),
+        aggregate: Arc::new(AggregateRegistry::new()),
+        new_aggregate: Arc::new(AggregateFunctionRegistry::new()),
+        window: Arc::new(WindowFunctionRegistry::new()),
+    })
+}
+
 /// Evaluates SQL expressions to compute `DataValues` (for SELECT clauses)
 /// This is different from `RecursiveWhereEvaluator` which returns boolean
 pub struct ArithmeticEvaluator<'a> {
     table: &'a DataTable,
-    _date_notation: String,
     function_registry: Arc<FunctionRegistry>,
     aggregate_registry: Arc<AggregateRegistry>, // Old registry (being phased out)
     new_aggregate_registry: Arc<AggregateFunctionRegistry>, // New registry
     window_function_registry: Arc<WindowFunctionRegistry>,
     visible_rows: Option<Vec<usize>>, // For aggregate functions on filtered views
     window_contexts: HashMap<u64, Arc<WindowContext>>, // Cache window contexts by hash
     table_aliases: HashMap<String, String>, // Map alias -> table name for qualified columns
+    case_insensitive: bool,           // The engine's string-comparison mode, as WHERE honours it
 }
 
 impl<'a> ArithmeticEvaluator<'a> {
     #[must_use]
     pub fn new(table: &'a DataTable) -> Self {
+        let registries = shared_registries();
         Self {
             table,
-            _date_notation: get_date_notation(),
-            function_registry: Arc::new(FunctionRegistry::new()),
-            aggregate_registry: Arc::new(AggregateRegistry::new()),
-            new_aggregate_registry: Arc::new(AggregateFunctionRegistry::new()),
-            window_function_registry: Arc::new(WindowFunctionRegistry::new()),
-            visible_rows: None,
-            window_contexts: HashMap::new(),
-            table_aliases: HashMap::new(),
-        }
-    }
-
-    #[must_use]
-    pub fn with_date_notation(table: &'a DataTable, date_notation: String) -> Self {
-        Self {
-            table,
-            _date_notation: date_notation,
-            function_registry: Arc::new(FunctionRegistry::new()),
-            aggregate_registry: Arc::new(AggregateRegistry::new()),
-            new_aggregate_registry: Arc::new(AggregateFunctionRegistry::new()),
-            window_function_registry: Arc::new(WindowFunctionRegistry::new()),
+            function_registry: Arc::clone(&registries.function),
+            aggregate_registry: Arc::clone(&registries.aggregate),
+            new_aggregate_registry: Arc::clone(&registries.new_aggregate),
+            window_function_registry: Arc::clone(&registries.window),
             visible_rows: None,
             window_contexts: HashMap::new(),
             table_aliases: HashMap::new(),
+            case_insensitive: false,
         }
     }
 
@@ -68,30 +73,19 @@ impl<'a> ArithmeticEvaluator<'a> {
         self
     }
 
-    /// Set table aliases for qualified column resolution
+    /// Compare strings case-insensitively, as the engine's case-insensitive
+    /// mode does for WHERE: comparisons, IN, BETWEEN and LIKE.
     #[must_use]
-    pub fn with_table_aliases(mut self, aliases: HashMap<String, String>) -> Self {
-        self.table_aliases = aliases;
+    pub fn with_case_insensitive(mut self, case_insensitive: bool) -> Self {
+        self.case_insensitive = case_insensitive;
         self
     }
 
+    /// Set table aliases for qualified column resolution
     #[must_use]
-    pub fn with_date_notation_and_registry(
-        table: &'a DataTable,
-        date_notation: String,
-        function_registry: Arc<FunctionRegistry>,
-    ) -> Self {
-        Self {
-            table,
-            _date_notation: date_notation,
-            function_registry,
-            aggregate_registry: Arc::new(AggregateRegistry::new()),
-            new_aggregate_registry: Arc::new(AggregateFunctionRegistry::new()),
-            window_function_registry: Arc::new(WindowFunctionRegistry::new()),
-            visible_rows: None,
-            window_contexts: HashMap::new(),
-            table_aliases: HashMap::new(),
-        }
+    pub fn with_table_aliases(mut self, aliases: HashMap<String, String>) -> Self {
+        self.table_aliases = aliases;
+        self
     }
 
     /// Find a column name similar to the given name using edit distance
@@ -166,8 +160,8 @@ impl<'a> ArithmeticEvaluator<'a> {
                 let val = self.evaluate(expr, row_index)?;
                 let lo = self.evaluate(lower, row_index)?;
                 let hi = self.evaluate(upper, row_index)?;
-                let ge = compare_trilean(&val, &lo, ">=");
-                let le = compare_trilean(&val, &hi, "<=");
+                let ge = compare_trilean(&val, &lo, ">=", self.case_insensitive);
+                let le = compare_trilean(&val, &hi, "<=", self.case_insensitive);
                 Ok(ge.and(le).to_value())
             }
             // Logical negation as a value-producing expression. Reached, e.g., by
@@ -385,7 +379,7 @@ impl<'a> ArithmeticEvaluator<'a> {
             // the clause that filters on the result collapses it (R13). See
             // tests/evaluator_matrix_tests.rs for the per-operator contract.
             ">" | "<" | ">=" | "<=" | "=" | "!=" | "<>" => {
-                Ok(compare_trilean(&left_val, &right_val, op).to_value())
+                Ok(compare_trilean(&left_val, &right_val, op, self.case_insensitive).to_value())
             }
             // IS NULL / IS NOT NULL are the sanctioned NULL tests, so two-valued.
             "IS NULL" => Ok(DataValue::Boolean(matches!(left_val, DataValue::Null))),
@@ -397,8 +391,12 @@ impl<'a> ArithmeticEvaluator<'a> {
                 if matches!(left_val, DataValue::Null) || matches!(right_val, DataValue::Null) {
                     return Ok(DataValue::Null);
                 }
-                let text = self.value_to_string(&left_val);
-                let pattern = self.value_to_string(&right_val);
+                let mut text = self.value_to_string(&left_val);
+                let mut pattern = self.value_to_string(&right_val);
+                if self.case_insensitive {
+                    text = text.to_lowercase();
+                    pattern = pattern.to_lowercase();
+                }
                 let matches = self.sql_like_match(&text, &pattern);
                 Ok(DataValue::Boolean(matches))
             }
@@ -1545,7 +1543,7 @@ impl<'a> ArithmeticEvaluator<'a> {
         let mut result = Trilean::False;
         for v in values {
             let item = self.evaluate(v, row_index)?;
-            result = result.or(compare_trilean(&val, &item, "="));
+            result = result.or(compare_trilean(&val, &item, "=", self.case_insensitive));
             if result.is_true() {
                 break;
             }
@@ -1665,12 +1663,16 @@ impl<'a> ArithmeticEvaluator<'a> {
 /// for predicates is what made `SELECT x = NULL` TRUE for NULL rows (P48, the
 /// value-evaluator twin of P18). Same rule as the WHERE evaluator's
 /// `compare_trilean`, which R13 slice 4 will make the only copy.
-fn compare_trilean(left: &DataValue, right: &DataValue, op: &str) -> Trilean {
+fn compare_trilean(
+    left: &DataValue,
+    right: &DataValue,
+    op: &str,
+    case_insensitive: bool,
+) -> Trilean {
     if matches!(left, DataValue::Null) || matches!(right, DataValue::Null) {
         return Trilean::Unknown;
     }
-    // Case sensitivity is not yet threaded into this evaluator - R13 slice 3.
-    Trilean::from_bool(compare_with_op(left, right, op, false))
+    Trilean::from_bool(compare_with_op(left, right, op, case_insensitive))
 }
 
 /// The truth value of an operand of AND / OR / NOT. A value with no truth value