vibesql_executor/select/executor/
builder.rs

1//! SelectExecutor construction and initialization
2
3use std::{
4    cell::{Cell, OnceCell, RefCell},
5    collections::HashMap,
6};
7
8use instant::Instant;
9
10use crate::{
11    errors::ExecutorError,
12    evaluator::compiled_pivot::PivotAggregateGroup,
13    limits::{MAX_MEMORY_BYTES, MEMORY_WARNING_BYTES},
14    memory::QueryArena,
15};
16
17/// Executes SELECT queries
18pub struct SelectExecutor<'a> {
19    pub(super) database: &'a vibesql_storage::Database,
20    pub(super) outer_row: Option<&'a vibesql_storage::Row>,
21    pub(super) outer_schema: Option<&'a crate::schema::CombinedSchema>,
22    /// All outer rows for outer-correlated aggregates (issue #4930).
23    /// When an aggregate in a scalar subquery references only outer columns,
24    /// it should aggregate over ALL outer rows, not just the current one.
25    pub(super) outer_rows: Option<&'a [vibesql_storage::Row]>,
26    /// Procedural context for stored procedure/function variable resolution
27    pub(super) procedural_context: Option<&'a crate::procedural::ExecutionContext>,
28    /// Trigger context for OLD/NEW pseudo-variable resolution.
29    /// Set when a SELECT runs inside a trigger body (e.g. the synthetic SELECT
30    /// used by UPDATE…FROM inside a trigger body, #5082).
31    pub(super) trigger_context: Option<&'a crate::trigger_execution::TriggerContext<'a>>,
32    /// CTE (Common Table Expression) context for accessing WITH clause results
33    /// Enables scalar subqueries to reference CTEs defined in the outer query
34    pub(super) cte_context: Option<&'a HashMap<String, super::super::cte::CteResult>>,
35    /// Subquery nesting depth (for preventing stack overflow)
36    pub(super) subquery_depth: usize,
37    /// Memory used by this query execution (in bytes)
38    pub(super) memory_used_bytes: Cell<usize>,
39    /// Flag to prevent logging the same warning multiple times
40    pub(super) memory_warning_logged: Cell<bool>,
41    /// Query start time (for timeout enforcement)
42    pub(crate) start_time: Instant,
43    /// Timeout in seconds (defaults to MAX_QUERY_EXECUTION_SECONDS)
44    pub timeout_seconds: u64,
45    /// Cache for aggregate results within a single group
46    /// Key: Hash of the aggregate expression (format: "{name}:{distinct}:{arg_debug}")
47    /// Value: Cached aggregate result
48    /// Scope: Per-group evaluation (cleared between groups)
49    /// Lazily initialized - only created when first aggregate is evaluated
50    pub(super) aggregate_cache: OnceCell<RefCell<HashMap<String, vibesql_types::SqlValue>>>,
51    /// Arena allocator for query-scoped allocations
52    /// Eliminates malloc/free overhead by using bump-pointer allocation
53    /// All allocations are freed when query completes
54    /// Lazily initialized - only created when first allocation is needed
55    pub(super) arena: OnceCell<RefCell<QueryArena>>,
56    /// Pivot aggregate group for batched SUM(CASE...) optimization
57    /// Detected once per query, executed once per group
58    /// Stores results directly in aggregate_cache
59    pub(super) pivot_group: RefCell<Option<PivotAggregateGroup>>,
60    /// Index of the "representative row" for aggregate context subqueries.
61    /// When evaluating correlated subqueries in aggregate SELECT lists, SQLite uses
62    /// the row that corresponds to the aggregate result (e.g., the row where the column
63    /// has its MAX value for MAX(col)). This field stores that row's index.
64    /// See issue #4683 for details.
65    pub(super) aggregate_representative_row_idx: RefCell<Option<usize>>,
66}
67
68impl<'a> SelectExecutor<'a> {
69    /// Create a new SELECT executor
70    ///
71    /// # Performance
72    ///
73    /// This constructor is optimized for OLTP workloads:
74    /// - Arena is lazily initialized (10MB allocation deferred until needed)
75    /// - Aggregate cache is lazily initialized (HashMap allocation deferred)
76    /// - Simple queries that don't use aggregates or complex allocations skip these costs
77    pub fn new(database: &'a vibesql_storage::Database) -> Self {
78        SelectExecutor {
79            database,
80            outer_row: None,
81            outer_schema: None,
82            outer_rows: None,
83            procedural_context: None,
84            trigger_context: None,
85            cte_context: None,
86            subquery_depth: 0,
87            memory_used_bytes: Cell::new(0),
88            memory_warning_logged: Cell::new(false),
89            start_time: Instant::now(),
90            timeout_seconds: crate::limits::MAX_QUERY_EXECUTION_SECONDS,
91            aggregate_cache: OnceCell::new(),
92            arena: OnceCell::new(),
93            pivot_group: RefCell::new(None),
94            aggregate_representative_row_idx: RefCell::new(None),
95        }
96    }
97
98    /// Create a new SELECT executor with CTE context
99    /// Used for INSERT ... SELECT with CTEs (WITH clause on INSERT statement)
100    pub fn new_with_cte(
101        database: &'a vibesql_storage::Database,
102        cte_context: &'a HashMap<String, super::super::cte::CteResult>,
103    ) -> Self {
104        Self::new_with_cte_and_depth(database, cte_context, 0)
105    }
106
107    /// Create a new SELECT executor with outer context for correlated subqueries
108    pub fn new_with_outer_context(
109        database: &'a vibesql_storage::Database,
110        outer_row: &'a vibesql_storage::Row,
111        outer_schema: &'a crate::schema::CombinedSchema,
112    ) -> Self {
113        SelectExecutor {
114            database,
115            outer_row: Some(outer_row),
116            outer_schema: Some(outer_schema),
117            outer_rows: None,
118            procedural_context: None,
119            trigger_context: None,
120            cte_context: None,
121            subquery_depth: 0,
122            memory_used_bytes: Cell::new(0),
123            memory_warning_logged: Cell::new(false),
124            start_time: Instant::now(),
125            timeout_seconds: crate::limits::MAX_QUERY_EXECUTION_SECONDS,
126            aggregate_cache: OnceCell::new(),
127            arena: OnceCell::new(),
128            pivot_group: RefCell::new(None),
129            aggregate_representative_row_idx: RefCell::new(None),
130        }
131    }
132
133    /// Create a new SELECT executor with explicit depth tracking
134    /// Used for non-correlated subqueries to propagate depth limit enforcement
135    pub fn new_with_depth(database: &'a vibesql_storage::Database, parent_depth: usize) -> Self {
136        SelectExecutor {
137            database,
138            outer_row: None,
139            outer_schema: None,
140            outer_rows: None,
141            procedural_context: None,
142            trigger_context: None,
143            cte_context: None,
144            subquery_depth: parent_depth + 1,
145            memory_used_bytes: Cell::new(0),
146            memory_warning_logged: Cell::new(false),
147            start_time: Instant::now(),
148            timeout_seconds: crate::limits::MAX_QUERY_EXECUTION_SECONDS,
149            aggregate_cache: OnceCell::new(),
150            arena: OnceCell::new(),
151            pivot_group: RefCell::new(None),
152            aggregate_representative_row_idx: RefCell::new(None),
153        }
154    }
155
156    /// Create a new SELECT executor with outer context and explicit depth
157    /// Used when creating subquery executors to track nesting depth
158    ///
159    /// # Note on Timeout Inheritance
160    ///
161    /// Currently subqueries get their own 60s timeout rather than sharing parent's timeout.
162    /// This means a query with N subqueries could run for up to N*60s instead of 60s total.
163    ///
164    /// However, this is acceptable for the initial fix because:
165    /// 1. The main regression (100% timeout) was caused by ZERO timeout enforcement
166    /// 2. Having per-subquery timeouts still prevents infinite loops (the core issue)
167    /// 3. Most problematic queries cause recursive subquery execution, which IS caught
168    /// 4. Threading timeout through evaluators requires extensive refactoring
169    ///
170    /// Future improvement: Add timeout fields to ExpressionEvaluator and pass through
171    /// See: <https://github.com/rjwalters/vibesql/issues/1012#subquery-timeout>
172    pub fn new_with_outer_context_and_depth(
173        database: &'a vibesql_storage::Database,
174        outer_row: &'a vibesql_storage::Row,
175        outer_schema: &'a crate::schema::CombinedSchema,
176        parent_depth: usize,
177    ) -> Self {
178        SelectExecutor {
179            database,
180            outer_row: Some(outer_row),
181            outer_schema: Some(outer_schema),
182            outer_rows: None,
183            procedural_context: None,
184            trigger_context: None,
185            cte_context: None,
186            subquery_depth: parent_depth + 1,
187            memory_used_bytes: Cell::new(0),
188            memory_warning_logged: Cell::new(false),
189            start_time: Instant::now(),
190            timeout_seconds: crate::limits::MAX_QUERY_EXECUTION_SECONDS,
191            aggregate_cache: OnceCell::new(),
192            arena: OnceCell::new(),
193            pivot_group: RefCell::new(None),
194            aggregate_representative_row_idx: RefCell::new(None),
195        }
196    }
197
198    /// Create a new SELECT executor with procedural context for stored procedures/functions
199    pub fn new_with_procedural_context(
200        database: &'a vibesql_storage::Database,
201        procedural_context: &'a crate::procedural::ExecutionContext,
202    ) -> Self {
203        SelectExecutor {
204            database,
205            outer_row: None,
206            outer_schema: None,
207            outer_rows: None,
208            procedural_context: Some(procedural_context),
209            trigger_context: None,
210            cte_context: None,
211            subquery_depth: 0,
212            memory_used_bytes: Cell::new(0),
213            memory_warning_logged: Cell::new(false),
214            start_time: Instant::now(),
215            timeout_seconds: crate::limits::MAX_QUERY_EXECUTION_SECONDS,
216            aggregate_cache: OnceCell::new(),
217            arena: OnceCell::new(),
218            pivot_group: RefCell::new(None),
219            aggregate_representative_row_idx: RefCell::new(None),
220        }
221    }
222
223    /// Create a new SELECT executor with trigger context for OLD/NEW pseudo-variable
224    /// resolution.
225    ///
226    /// Used by the synthetic SELECT inside `UPDATE … FROM …` when the UPDATE is
227    /// running inside a trigger body, so that `OLD.col` / `NEW.col` references in
228    /// the WHERE clause and SET expressions can resolve against the firing row.
229    /// (Issue #5082, Bucket B of #5073.)
230    pub fn new_with_trigger_context(
231        database: &'a vibesql_storage::Database,
232        trigger_context: &'a crate::trigger_execution::TriggerContext<'a>,
233    ) -> Self {
234        SelectExecutor {
235            database,
236            outer_row: None,
237            outer_schema: None,
238            outer_rows: None,
239            procedural_context: None,
240            trigger_context: Some(trigger_context),
241            cte_context: None,
242            subquery_depth: 0,
243            memory_used_bytes: Cell::new(0),
244            memory_warning_logged: Cell::new(false),
245            start_time: Instant::now(),
246            timeout_seconds: crate::limits::MAX_QUERY_EXECUTION_SECONDS,
247            aggregate_cache: OnceCell::new(),
248            arena: OnceCell::new(),
249            pivot_group: RefCell::new(None),
250            aggregate_representative_row_idx: RefCell::new(None),
251        }
252    }
253
254    /// Create a new SELECT executor with CTE context and depth tracking
255    /// Used for non-correlated subqueries that need access to parent CTEs
256    pub fn new_with_cte_and_depth(
257        database: &'a vibesql_storage::Database,
258        cte_context: &'a HashMap<String, super::super::cte::CteResult>,
259        parent_depth: usize,
260    ) -> Self {
261        SelectExecutor {
262            database,
263            outer_row: None,
264            outer_schema: None,
265            outer_rows: None,
266            procedural_context: None,
267            trigger_context: None,
268            cte_context: Some(cte_context),
269            subquery_depth: parent_depth + 1,
270            memory_used_bytes: Cell::new(0),
271            memory_warning_logged: Cell::new(false),
272            start_time: Instant::now(),
273            timeout_seconds: crate::limits::MAX_QUERY_EXECUTION_SECONDS,
274            aggregate_cache: OnceCell::new(),
275            arena: OnceCell::new(),
276            pivot_group: RefCell::new(None),
277            aggregate_representative_row_idx: RefCell::new(None),
278        }
279    }
280
281    /// Create a new SELECT executor with outer context, CTE context, and depth tracking
282    /// Used for correlated subqueries that need access to both outer row and parent CTEs
283    pub fn new_with_outer_and_cte_and_depth(
284        database: &'a vibesql_storage::Database,
285        outer_row: &'a vibesql_storage::Row,
286        outer_schema: &'a crate::schema::CombinedSchema,
287        cte_context: &'a HashMap<String, super::super::cte::CteResult>,
288        parent_depth: usize,
289    ) -> Self {
290        SelectExecutor {
291            database,
292            outer_row: Some(outer_row),
293            outer_schema: Some(outer_schema),
294            outer_rows: None,
295            procedural_context: None,
296            trigger_context: None,
297            cte_context: Some(cte_context),
298            subquery_depth: parent_depth + 1,
299            memory_used_bytes: Cell::new(0),
300            memory_warning_logged: Cell::new(false),
301            start_time: Instant::now(),
302            timeout_seconds: crate::limits::MAX_QUERY_EXECUTION_SECONDS,
303            aggregate_cache: OnceCell::new(),
304            arena: OnceCell::new(),
305            pivot_group: RefCell::new(None),
306            aggregate_representative_row_idx: RefCell::new(None),
307        }
308    }
309
310    /// Create a new SELECT executor with outer context, outer rows, and depth tracking
311    /// Used for outer-correlated aggregates (issue #4930) where aggregates in scalar
312    /// subqueries need access to ALL outer rows, not just the current one.
313    pub fn new_with_outer_rows_and_depth(
314        database: &'a vibesql_storage::Database,
315        outer_row: &'a vibesql_storage::Row,
316        outer_schema: &'a crate::schema::CombinedSchema,
317        outer_rows: &'a [vibesql_storage::Row],
318        parent_depth: usize,
319    ) -> Self {
320        SelectExecutor {
321            database,
322            outer_row: Some(outer_row),
323            outer_schema: Some(outer_schema),
324            outer_rows: Some(outer_rows),
325            procedural_context: None,
326            trigger_context: None,
327            cte_context: None,
328            subquery_depth: parent_depth + 1,
329            memory_used_bytes: Cell::new(0),
330            memory_warning_logged: Cell::new(false),
331            start_time: Instant::now(),
332            timeout_seconds: crate::limits::MAX_QUERY_EXECUTION_SECONDS,
333            aggregate_cache: OnceCell::new(),
334            arena: OnceCell::new(),
335            pivot_group: RefCell::new(None),
336            aggregate_representative_row_idx: RefCell::new(None),
337        }
338    }
339
340    /// Create a new SELECT executor with outer context, outer rows, CTE, and depth tracking
341    /// Used for outer-correlated aggregates (issue #4930) with CTE support
342    pub fn new_with_outer_rows_and_cte_and_depth(
343        database: &'a vibesql_storage::Database,
344        outer_row: &'a vibesql_storage::Row,
345        outer_schema: &'a crate::schema::CombinedSchema,
346        outer_rows: &'a [vibesql_storage::Row],
347        cte_context: &'a HashMap<String, super::super::cte::CteResult>,
348        parent_depth: usize,
349    ) -> Self {
350        SelectExecutor {
351            database,
352            outer_row: Some(outer_row),
353            outer_schema: Some(outer_schema),
354            outer_rows: Some(outer_rows),
355            procedural_context: None,
356            trigger_context: None,
357            cte_context: Some(cte_context),
358            subquery_depth: parent_depth + 1,
359            memory_used_bytes: Cell::new(0),
360            memory_warning_logged: Cell::new(false),
361            start_time: Instant::now(),
362            timeout_seconds: crate::limits::MAX_QUERY_EXECUTION_SECONDS,
363            aggregate_cache: OnceCell::new(),
364            arena: OnceCell::new(),
365            pivot_group: RefCell::new(None),
366            aggregate_representative_row_idx: RefCell::new(None),
367        }
368    }
369
370    /// Track memory allocation
371    pub(super) fn track_memory_allocation(&self, bytes: usize) -> Result<(), ExecutorError> {
372        let mut current = self.memory_used_bytes.get();
373        current += bytes;
374        self.memory_used_bytes.set(current);
375
376        // Log warning at threshold
377        if !self.memory_warning_logged.get() && current > MEMORY_WARNING_BYTES {
378            eprintln!(
379                "⚠️  Query memory usage: {:.2} GB",
380                current as f64 / 1024.0 / 1024.0 / 1024.0
381            );
382            self.memory_warning_logged.set(true);
383        }
384
385        // Hard limit
386        if current > MAX_MEMORY_BYTES {
387            return Err(ExecutorError::MemoryLimitExceeded {
388                used_bytes: current,
389                max_bytes: MAX_MEMORY_BYTES,
390            });
391        }
392
393        Ok(())
394    }
395
396    /// Track memory deallocation
397    #[cfg(test)]
398    pub(super) fn track_memory_deallocation(&self, bytes: usize) {
399        let current = self.memory_used_bytes.get();
400        self.memory_used_bytes.set(current.saturating_sub(bytes));
401    }
402
403    /// Override default timeout for this query (useful for testing)
404    pub fn with_timeout(mut self, seconds: u64) -> Self {
405        self.timeout_seconds = seconds;
406        self
407    }
408
409    /// Clear aggregate cache (should be called between group evaluations)
410    /// No-op if the cache has not been initialized (lazy initialization)
411    pub(super) fn clear_aggregate_cache(&self) {
412        if let Some(cache) = self.aggregate_cache.get() {
413            cache.borrow_mut().clear();
414        }
415    }
416
417    /// Get access to the aggregate cache, initializing it lazily if needed
418    pub(super) fn get_aggregate_cache(&self) -> &RefCell<HashMap<String, vibesql_types::SqlValue>> {
419        self.aggregate_cache.get_or_init(|| RefCell::new(HashMap::new()))
420    }
421
422    /// Get access to the query buffer pool for reducing allocations
423    pub(crate) fn query_buffer_pool(&self) -> &vibesql_storage::QueryBufferPool {
424        self.database.query_buffer_pool()
425    }
426
427    /// Check if query has exceeded timeout
428    /// Call this in hot loops to prevent infinite execution
429    pub fn check_timeout(&self) -> Result<(), crate::errors::ExecutorError> {
430        let elapsed = self.start_time.elapsed().as_secs();
431        if elapsed >= self.timeout_seconds {
432            return Err(crate::errors::ExecutorError::QueryTimeoutExceeded {
433                elapsed_seconds: elapsed,
434                max_seconds: self.timeout_seconds,
435            });
436        }
437        Ok(())
438    }
439
440    /// Get access to the query arena for allocations
441    /// The arena is lazily initialized on first access
442    #[allow(dead_code)]
443    pub(crate) fn arena(&self) -> &RefCell<QueryArena> {
444        self.arena.get_or_init(|| RefCell::new(QueryArena::new()))
445    }
446
447    /// Reset the arena for query reuse
448    /// Called at the start of each query execution
449    /// No-op if the arena has not been initialized (lazy initialization)
450    pub(super) fn reset_arena(&self) {
451        if let Some(arena) = self.arena.get() {
452            arena.borrow_mut().reset();
453        }
454    }
455
456    /// Reset the executor for reuse between queries
457    ///
458    /// This method prepares the executor for a new query execution by:
459    /// - Resetting the start time to now
460    /// - Clearing memory tracking counters
461    /// - Resetting the arena (if initialized)
462    /// - Clearing the aggregate cache (if initialized)
463    ///
464    /// # Performance
465    ///
466    /// Call this method to reuse an executor instead of creating a new one.
467    /// This avoids the allocation overhead of creating new HashMap and arena instances.
468    pub fn reset_for_reuse(&mut self) {
469        self.start_time = Instant::now();
470        self.memory_used_bytes.set(0);
471        self.memory_warning_logged.set(false);
472        self.subquery_depth = 0;
473        self.outer_row = None;
474        self.outer_schema = None;
475        self.procedural_context = None;
476        self.trigger_context = None;
477        self.cte_context = None;
478
479        // Reset arena if it was initialized (clears offset, keeps buffer allocation)
480        if let Some(arena) = self.arena.get() {
481            arena.borrow_mut().reset();
482        }
483
484        // Clear aggregate cache if it was initialized (clears entries, keeps HashMap allocation)
485        if let Some(cache) = self.aggregate_cache.get() {
486            cache.borrow_mut().clear();
487        }
488
489        // Clear pivot group
490        *self.pivot_group.borrow_mut() = None;
491
492        // Clear representative row index
493        *self.aggregate_representative_row_idx.borrow_mut() = None;
494    }
495
496    /// Set the pivot aggregate group for this query
497    ///
498    /// Called once during query planning when a pivot pattern is detected.
499    /// The pivot group is then executed once per group in aggregation.
500    pub(super) fn set_pivot_group(&self, group: PivotAggregateGroup) {
501        *self.pivot_group.borrow_mut() = Some(group);
502    }
503
504    /// Execute pivot aggregates for the current group and cache results
505    ///
506    /// This executes all pivot aggregates in a single pass over the rows,
507    /// storing results in the aggregate cache. Subsequent calls to evaluate
508    /// individual pivot aggregates will hit the cache.
509    pub(super) fn execute_pivot_aggregates(
510        &self,
511        group_rows: &[vibesql_storage::Row],
512    ) -> Result<(), ExecutorError> {
513        let pivot_group = self.pivot_group.borrow();
514        if let Some(ref pivot) = *pivot_group {
515            let results = pivot.execute(group_rows)?;
516
517            // Store all pivot results in the aggregate cache
518            let cache = self.get_aggregate_cache();
519            let mut cache_mut = cache.borrow_mut();
520            for (cache_key, value) in results {
521                cache_mut.insert(cache_key, value);
522            }
523        }
524        Ok(())
525    }
526
527    /// Check if a pivot group is set for this query
528    pub(super) fn has_pivot_group(&self) -> bool {
529        self.pivot_group.borrow().is_some()
530    }
531
532    /// Set the representative row index for aggregate context subquery evaluation.
533    ///
534    /// This is used to implement SQLite's behavior where correlated subqueries in
535    /// aggregate SELECT lists use the row that corresponds to the aggregate result.
536    /// For example, in `SELECT max(a), (SELECT d FROM t2 WHERE a=c) FROM t1`,
537    /// the subquery uses `a` from the row where `a` has its maximum value.
538    ///
539    /// # Arguments
540    /// * `idx` - The index of the representative row in the current group's rows
541    pub(super) fn set_aggregate_representative_row(&self, idx: Option<usize>) {
542        *self.aggregate_representative_row_idx.borrow_mut() = idx;
543    }
544
545    /// Get the representative row index for aggregate context subquery evaluation.
546    /// Returns None if no representative row has been set.
547    pub(super) fn get_aggregate_representative_row(&self) -> Option<usize> {
548        *self.aggregate_representative_row_idx.borrow()
549    }
550
551    /// Find the representative row based on aggregates in the SELECT list.
552    ///
553    /// For MAX(col) aggregates, returns the index of the row where col has its maximum value.
554    /// For MIN(col) aggregates, returns the index of the row where col has its minimum value.
555    /// For other aggregates or no aggregates, returns None (fallback to first row behavior).
556    ///
557    /// This implements SQLite's behavior where bare column references in aggregate queries
558    /// use values from the row that contributed to the aggregate result.
559    ///
560    /// # Arguments
561    /// * `select_list` - The expanded SELECT list to scan for aggregates
562    /// * `group_rows` - The rows in the current group
563    /// * `evaluator` - Expression evaluator for computing column values
564    pub(super) fn find_representative_row_index(
565        &self,
566        select_list: &[vibesql_ast::SelectItem],
567        group_rows: &[vibesql_storage::Row],
568        evaluator: &crate::evaluator::CombinedExpressionEvaluator,
569    ) -> Option<usize> {
570        use crate::select::grouping::compare_sql_values;
571        use vibesql_types::SqlValue;
572
573        if group_rows.is_empty() {
574            return None;
575        }
576
577        // Scan SELECT list for MAX or MIN aggregates on a column
578        for item in select_list {
579            if let vibesql_ast::SelectItem::Expression { expr, .. } = item {
580                if let vibesql_ast::Expression::AggregateFunction { name, args, .. } = expr {
581                    let name_upper = name.to_uppercase();
582
583                    // We only care about MAX/MIN aggregates on columns
584                    if (name_upper == "MAX" || name_upper == "MIN") && args.len() == 1 {
585                        // Find the row where the column has its max/min value
586                        let mut best_idx = 0;
587                        let mut best_val: Option<SqlValue> = None;
588
589                        for (idx, row) in group_rows.iter().enumerate() {
590                            if let Ok(val) = evaluator.eval(&args[0], row) {
591                                // Skip NULL values
592                                if matches!(val, SqlValue::Null) {
593                                    continue;
594                                }
595
596                                let is_better = match &best_val {
597                                    None => true,
598                                    Some(best) => {
599                                        let cmp = compare_sql_values(&val, best);
600                                        if name_upper == "MAX" {
601                                            cmp == std::cmp::Ordering::Greater
602                                        } else {
603                                            cmp == std::cmp::Ordering::Less
604                                        }
605                                    }
606                                };
607
608                                if is_better {
609                                    best_idx = idx;
610                                    best_val = Some(val);
611                                }
612                            }
613                        }
614
615                        // If we found a non-NULL value, use that row
616                        if best_val.is_some() {
617                            return Some(best_idx);
618                        }
619                    }
620                }
621            }
622        }
623
624        // No suitable aggregate found, return None (will fall back to first row)
625        None
626    }
627}
vibesql_executor/select/executor/builder.rs

vibesql_executor/select/executor/
builder.rs
