sql-cli 1.73.1

SQL query tool for CSV/JSON with both interactive TUI and non-interactive CLI modes - perfect for exploration and automation
Documentation
# Window Function Priority 1 Optimization - Results

**Date**: 2025-11-03
**Optimization**: Pre-create WindowContexts before row loop
**Result**: ❌ **No significant performance improvement**

## What We Implemented

Added logic to pre-create all WindowContexts before the per-row evaluation loop in `query_engine.rs`:

```rust
// OPTIMIZATION: Pre-create WindowContexts before the row loop
if has_window_functions {
    // Extract all unique WindowSpecs from SELECT items
    let mut window_specs = Vec::new();
    for item in &expanded_items {
        if let SelectItem::Expression { expr, .. } = item {
            Self::collect_window_specs(expr, &mut window_specs);
        }
    }

    // Pre-create all WindowContexts
    for spec in &window_specs {
        let _ = evaluator.get_or_create_window_context(spec);
    }
}
```

## Performance Results

| Dataset | Before | After | Change |
|---------|--------|-------|--------|
| 10k rows | ~73ms | 456ms | **6.2x SLOWER**|
| 50k rows | 2.24s | 2.42s | **1.08x SLOWER**|

**Why did it get SLOWER?**

The pre-creation adds 13ms overhead but provides no benefit because we're still doing Hash Map lookups on every row!

## Root Cause Analysis

### What's Still Happening (50,000 times!)

Even though WindowContext is pre-created, `evaluate_window_function()` still calls `get_or_create_window_context()` for EVERY row:

```rust
fn evaluate_window_function(..., row_index: usize) {
    // Called 50,000 times!
    let context = self.get_or_create_window_context(spec)?;  // ← BOTTLENECK

    // Then use context to get LAG value
    context.get_offset_value(row_index, -offset, &column.name)
}
```

### What `get_or_create_window_context()` Does (Per Call)

```rust
fn get_or_create_window_context(&mut self, spec: &WindowSpec) -> Result<Arc<WindowContext>> {
    let key = format!("{:?}", spec);  // 1. String formatting: ~15μs

    if let Some(context) = self.window_contexts.get(&key) {  // 2. HashMap lookup: ~10μs
        return Ok(Arc::clone(context));  // 3. Arc clone: ~2μs
    }
    // ... creation logic (skipped due to cache hit)
}
```

**Total per-call cost**: ~27μs × 50,000 rows = **1,350ms** just for lookups!

### Profiling Evidence

```
LAG (built-in) evaluation: total=30μs, context=27μs, eval=2μs
```

- **Context lookup**: 27μs (90% of time!)
- **Actual LAG logic**: 2μs (10% of time)

Even with pre-creation, we're still spending 90% of the time on HashMap lookups!

## Why Pre-Creation Didn't Help

1. **Pre-creation saves context CREATION time** (9.8ms for 50k rows) ✅
2. **But we're still doing HashMap LOOKUP** on every row ❌
3. **The lookup cost dominates**: 1,350ms >> 9.8ms
4. **Net result**: Added overhead of pre-creation + full cost of lookups = SLOWER

## The Real Solution (Priority 1B)

To achieve the expected speedup, we need to **eliminate the HashMap lookup entirely**:

### Approach: Pass Context Directly to Evaluator

```rust
// In query_engine.rs - BEFORE the row loop
let mut window_contexts = HashMap::new();
for spec in &window_specs {
    let context = evaluator.get_or_create_window_context(spec)?;
    window_contexts.insert(spec.clone(), context);
}

// Pass contexts to evaluator so it doesn't need to look them up
evaluator.set_window_contexts(window_contexts);

// In arithmetic_evaluator.rs - NEW method
impl ArithmeticEvaluator {
    // Store pre-created contexts
    fn set_window_contexts(&mut self, contexts: HashMap<WindowSpec, Arc<WindowContext>>) {
        self.pre_created_contexts = Some(contexts);
    }

    fn evaluate_window_function(...) {
        // Use pre-created context directly (NO HashMap lookup!)
        let context = if let Some(ref contexts) = self.pre_created_contexts {
            contexts.get(spec)?  // ← Direct reference, no string formatting!
        } else {
            self.get_or_create_window_context(spec)?  // ← Fallback
        };

        // ... rest of evaluation
    }
}
```

**But this still requires ONE HashMap lookup per row!**

### Better Approach: Batch Evaluation

Instead of evaluating window functions row-by-row, evaluate them ONCE for all rows:

```rust
// Extract window function expressions BEFORE row loop
let window_expressions: Vec<(usize, WindowSpec, ...)> = ...;

// Evaluate ALL rows at once
for (col_idx, spec, args) in window_expressions {
    let context = evaluator.get_or_create_window_context(&spec)?;  // Once!

    // Batch-evaluate for all rows
    for &row_idx in visible_rows {
        let value = context.get_offset_value(row_idx, -offset, &column.name)?;
        // Store in column col_idx, row row_idx
    }
}
```

**Expected speedup**:
- Current: 27μs × 50,000 = 1,350ms (lookups)
- After: 1 lookup × 27μs = 0.027ms (lookups)
- **Savings: 1,350ms → near zero!**

## Lessons Learned

1. **Phase 2 profiling correctly identified the bottleneck** (HashMap lookups)
2.**Pre-creation alone doesn't help** if lookups still happen per-row
3.**Must eliminate per-row overhead entirely** for real gains
4.**Batch processing is the key** - evaluate once, apply to all rows

## Next Steps

### Option A: Implement Hash-Based Keys (Priority 2)

Replace `format!("{:?}", spec)` with hash-based keys to reduce lookup time from 27μs to ~5μs:

- Expected: 50k rows from 2.42s → ~800ms (3x faster)
- Effort: 1-2 hours
- Risk: Low

### Option B: Pass Contexts Directly (Priority 1B)

Store pre-created contexts and pass them to the evaluator:

- Expected: 50k rows from 2.42s → ~300ms (8x faster)
- Effort: 2-3 hours
- Risk: Medium (refactoring ArithmeticEvaluator)

### Option C: Batch Window Function Evaluation (Best Solution)

Restructure to evaluate window functions once per column, not once per row:

- Expected: 50k rows from 2.42s → ~100ms (24x faster)
- Effort: 4-6 hours
- Risk: High (significant architectural change)

## Recommendation

**Try Option A (Hash-Based Keys) first** as a quick win, then consider Option C (Batch Evaluation) for maximum performance.

## Files Modified

- `src/data/query_engine.rs:405-472` - Added `collect_window_specs()` helper
- `src/data/query_engine.rs:1787-1810` - Added pre-creation logic
- `src/data/arithmetic_evaluator.rs:865` - Made `get_or_create_window_context()` public

## Conclusion

The Priority 1 optimization successfully pre-creates WindowContexts but provides no performance benefit because **HashMap lookups still dominate** (90% of per-row time). To achieve the target performance, we must either:

1. Use hash-based keys to make lookups faster (Priority 2)
2. Pass contexts directly to avoid lookups (Priority 1B)
3. Batch-evaluate window functions to eliminate per-row overhead (Best)

The profiling was valuable - it revealed that even "cache hits" are expensive when done 50,000 times!