Refactor v18b search state only #1
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ontainer (V18b) Successfully migrated SearchState from EnhancedTuiApp to use AppStateContainer: 🔧 Changes Made: - Commented out `search_state: SearchState` field in EnhancedTuiApp struct - Removed SearchState initialization in constructor - Migrated all SearchState assignments to use `state_container.search_mut()` - Updated field mappings to match AppStateContainer SearchState structure: * `current_match: Option<(usize, usize)>` → `current_match: usize` * Removed `match_index` field (not used in AppStateContainer) * Added `is_active: bool` field for proper state management 🛡️ Compatibility: - Added comprehensive fallback handlers with warning messages - Maintains backward compatibility when state_container is not available - All existing search functionality preserved 🧪 Verification: - All compilation errors resolved - Binary builds successfully - Application starts without crashes - Integration test created and passes all checks - 38 calls to state_container.search() API confirmed ✅ Results: - No duplicate SearchState instances - Single source of truth achieved through AppStateContainer - Clean architecture maintained - Ready for V18c ColumnSearchState migration Testing confirmed cross-platform compatibility maintained. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
The fuzzy filter was applying on every keystroke, causing UI stickiness with large datasets (100k+ rows). Each character typed would trigger a full scan of all rows. Changes: - Removed apply_fuzzy_filter() call from KeyCode::Char handler - Removed apply_fuzzy_filter() call from KeyCode::Backspace handler - Let the existing debounced search mechanism handle filter application - Only clear filter immediately when pattern becomes empty This allows typing to remain responsive even with large datasets, as the expensive filter operation only runs after the debounce period expires. The Enter key still applies the filter immediately for explicit user action.
TimelordUK
added a commit
that referenced
this pull request
Sep 13, 2025
- Added option to open HTML table in browser for proper copying - Browser preview shows styled table that can be copied and pasted - Works with Gmail, Outlook, Teams - preserves formatting - Added 'yb' keymap to open in browser directly - Updated export menu with browser option as #1 choice - Includes helpful tip in browser view about copying The browser approach solves the issue where pasting raw HTML into Gmail shows code instead of a rendered table.
TimelordUK
added a commit
that referenced
this pull request
Oct 1, 2025
Fixed both cursor detection and query generation to handle WEB CTEs:
- Search phase: Case-insensitive matching for 'name AS ('
- Generation phase: Added pattern for 'WEB name AS (' lines
- Handles both 'WITH WEB name AS (' and standalone 'WEB name AS ('
This should fix the issue where WEB CTEs were skipped, causing
enriched to be detected as CTE #1 instead of CTE #2.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
TimelordUK
added a commit
that referenced
this pull request
Oct 4, 2025
Critical fix for CTE extraction - previously only extracted single CTE which wouldn't execute due to missing dependencies. ## Problem When testing CTE #3 in a chain: WITH WEB raw_trades AS (...), -- CTE 1 filtered AS (...), -- CTE 2 (uses raw_trades) aggregated AS (...) -- CTE 3 (uses filtered) SELECT * FROM final; Old behavior: Extract ONLY aggregated → fails (missing filtered, raw_trades) ## Solution Now extracts ALL CTEs up to and including target: WITH WEB raw_trades AS (...), filtered AS (...), aggregated AS (...) SELECT * FROM aggregated; -- Testable! ## Changes - src/analysis/mod.rs: Rewrote extract_cte() to include all dependencies - Adds WITH clause with all CTEs from index 0 to target index - Appends SELECT * FROM target for execution - Properly formats WEB CTEs with METHOD, HEADERS, BODY, FORMAT ## Testing Verified with work-like scenario: - WEB CTE + 3 chained Standard CTEs - Extracting CTE #2 includes CTE #1 (WEB dependency) - Extracting CTE #3 includes CTE #1 + #2 (all dependencies) - All extracted queries are now executable This matches user's work requirement: "extract up to and including cte3" so the query can actually run and show results. 🤖 Generated with Claude Code https://claude.com/claude-code Co-Authored-By: Claude <noreply@anthropic.com>
TimelordUK
added a commit
that referenced
this pull request
Oct 12, 2025
…y-aware execution Problem: When using \sx (execute statement at cursor) in a script with multiple GO-separated statements, the statement numbering was incorrect. If cursor was on line 10 and the first GO was on line 8, it would report "Executing statement #1 with 0 dependencies" when it should be "Executing statement #2 with 1 dependencies". Root Cause: The old logic counted GO separators BEFORE the cursor line: - Counted GOs from line 1 to cursor_line-1 - This meant cursor on line 10 with GO on line 8 = statement 2 (incorrect) - But actually, lines 1-8 are statement 1, lines 9-11 are statement 2 The confusion was: GO ENDS the statement, not starts it. Solution: Changed from counting GOs before cursor to finding which statement BLOCK contains cursor: - Statement 1: lines 1 to first GO - Statement 2: after first GO to second GO - etc. Now correctly identifies cursor position within statement blocks. Testing: Script with cursor on line 10 (SELECT * FROM #secmaster): - Line 8: GO (ends statement 1) - Lines 9-11: statement 2 - Correctly reports: "Executing statement #2 with 1 dependencies" - Execution order: [1, 2] ✅ 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
TimelordUK
added a commit
that referenced
this pull request
Oct 25, 2025
Implements automatic aliasing for aggregate functions in HAVING clauses, showcasing the preprocessing pipeline framework with a real-world use case. Problem Solved: Users previously had to manually alias aggregates: SELECT region, COUNT(*) as cnt FROM sales GROUP BY region HAVING cnt > 5 Now they can write natural SQL: SELECT region, COUNT(*) FROM sales GROUP BY region HAVING COUNT(*) > 5 The preprocessor automatically transforms it! Implementation: - HavingAliasTransformer - New transformer in pipeline * Detects aggregates without aliases in SELECT * Generates unique aliases (__agg_1, __agg_2, etc.) * Rewrites HAVING to use column references * Recursive expression rewriting for complex HAVING clauses - Parser Changes: * Removed validation that rejected aggregates in HAVING * Now allows aggregates (transformer handles them) * Updated test to expect success instead of error - Pipeline Integration: * Added to standard pipeline after ExpressionLifter * Order: ExpressionLifter → HavingAliasTransformer → CTEHoister → InOperatorLifter * Runs in ~0.01ms (negligible overhead) Features: - Supports COUNT, SUM, AVG, MIN, MAX, COUNT_DISTINCT - Handles complex HAVING expressions (AND, OR, comparisons) - Preserves existing aliases (doesn't overwrite user-provided names) - Normalizes expressions for reliable matching Testing: - 5 unit tests for transformer logic - 6 integration tests with real queries - All 474 existing tests still pass - Demo script showcasing the feature Performance: - Transformation overhead: ~0.01ms - No impact on queries without HAVING - Skips queries with existing aliases Files Created: - src/query_plan/having_alias_transformer.rs (348 lines) - tests/integration/test_having_auto_alias.sh - scripts/demo_having_auto_alias.sh Files Modified: - src/query_plan/mod.rs - Added to pipeline - src/sql/recursive_parser.rs - Removed aggregate validation - src/sql/parser/tests.rs - Updated test expectations Demo: ./scripts/demo_having_auto_alias.sh This demonstrates the power of the preprocessing framework - adding sophisticated SQL features without touching the executor! 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
TimelordUK
added a commit
that referenced
this pull request
Oct 26, 2025
Extends the AST preprocessing pipeline to work with script execution (-f flag), enabling all Quick Win features (WHERE/GROUP BY/HAVING alias expansion) in SQL scripts with GO separators. ## Problem Scripts executed with -f flag were bypassing the preprocessing pipeline, so Quick Win features (alias expansion) only worked in query mode (-q) and interactive mode. This made scripts inconsistent with other execution modes. ## Solution Added preprocessing pipeline to script execution in execute_script(): - Parse each statement in the script - Apply full preprocessing pipeline if statement has FROM clause - Fall back to original query if preprocessing fails - Preserve special row-iteration semantics for queries without FROM ## Implementation Modified src/non_interactive.rs execute_script(): - Check if statement has FROM clause - If yes: apply preprocessing pipeline, then format AST back to SQL - If no: preserve original behavior (special semantics) - Handle INTO clause removal after preprocessing Pipeline runs transformers in order: 1. ExpressionLifter 2. WhereAliasExpander 3. GroupByAliasExpander 4. HavingAliasTransformer 5. CTEHoister 6. InOperatorLifter ## New Reference File Added examples/expansion_transformers.sql: - Comprehensive reference showing all supported alias expansion features - Documents Quick Wins #1, #2, #3 - Shows working examples for each transformer - Documents future enhancements (CASE, BETWEEN, IN with aliases) - Includes implementation notes and testing commands - Can be run directly: sql-cli examples/expansion_transformers.sql ## Tests All tests passing: - 487 Rust unit tests - 397 integration tests - 511 Python tests - expansion_transformers.sql runs successfully with all features working ## Benefits - Scripts now have feature parity with query and interactive modes - Users can write cleaner SQL scripts with alias expansion - Example files demonstrate best practices - Consistent behavior across all execution modes 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
TimelordUK
added a commit
that referenced
this pull request
Nov 1, 2025
Fixes critical regression where temp tables created with SELECT INTO were not persisting between statements in dependency-aware execution (\sx mode in nvim plugin). ## Problem After the execution mode unification refactor (commit e3ec558), temp tables stopped working in --execute-statement mode. When executing: ```sql SELECT * FROM web_cte INTO #temp; GO SELECT * FROM #temp; GO ``` Statement #2 would return DUMMY table instead of the temp table data. ## Root Cause The StatementExecutor.execute() method was removing the INTO clause during preprocessing (line 175) before execution, so the QueryEngine never knew to store the result as a temp table. The INTO clause was being stripped by IntoClauseRemover but the result was never captured. ## Solution Modified StatementExecutor.execute() to: 1. Capture INTO table name BEFORE preprocessing removes it (Step 0) 2. Execute the query normally 3. If INTO clause was present, materialize the result and store it in the ExecutionContext's temp_tables registry (Step 4) This ensures temp tables persist within a single --execute-statement invocation across multiple dependent statements. ## Changes **src/execution/statement_executor.rs:** - Added Step 0: Capture into_table_name before preprocessing - Added Step 4: Materialize and store result if INTO clause was present - Uses QueryEngine.materialize_view() to convert DataView to DataTable - Stores in context.temp_tables for subsequent statement access ## Testing All 10 statement_executor tests pass. User scenario now works: 1. \sx on statement #1: Creates #temp with 72,888 rows ✓ 2. \sx on statement #2: Queries #temp, returns actual data ✓ Fixes regression introduced in execution mode unification. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.