Refactor v29 column search verification #2
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
- Remove all duplicate column search operations from buffer - Migrate all column search functionality to AppStateContainer exclusively - Fix Results title corruption with robust string building - Add compact number formatting (10k, 1.5M, etc.) - Add cursor coordinates (x,y) to status line in Results mode - Remove unused ColumnSearchState from buffer.rs - Clean up all buffer column search methods from BufferAPI trait The column search state is now completely centralized in AppStateContainer with zero duplication. All operations flow through proper state management. Display improvements: - Results title no longer gets corrupted when applying WHERE conditions - Compact row counts for immediate readability (10.1k vs 10100) - Cursor position shown as (column,row) in status line for precise navigation
When column search finds a match, we must update BOTH the AppStateContainer and the buffer's current column to keep them in sync. This ensures the cursor actually moves to the matched column visually, not just in state. Previously, typing \orderid would find the column but not move the cursor to it. Now the cursor properly jumps to the first matching column (externalOrderId).
Add sample data files for testing financial queries: - instruments.csv/json: Reference data for financial instruments - vwap_example_fills.json: VWAP trade execution fills - vwap_example_orders.json: VWAP order data These provide realistic financial data for testing complex queries involving instruments, trades, and VWAP calculations.
- Add get_column_names() method to BufferAPI trait - Implement method in both Buffer (JSON) and DataTableBuffer (CSV) - Update search_columns() to use unified column retrieval method - Column search now works correctly with all data sources - Cursor properly moves to matched columns when searching
- Add ensure_visible() call in set_current_column() method - This fixes the Windows issue where column search finds matches but doesn't scroll to show them - The viewport now properly scrolls horizontally to display the matched column
- Add production_vwap_final test data for realistic trading scenarios - Include supporting generation scripts - Add architecture documentation for trading simulation
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 12, 2025
Fixed three critical issues with the execute-statement-at-cursor (\sx) feature: 1. Statement counting bug: Changed from counting GO separators before cursor to finding which statement block contains the cursor. GO ends a statement, not starts it, so cursor position must be compared to GO locations to determine which statement block the cursor is in. 2. Case-insensitive GO matching: Added .upper() before regex match to accept both 'GO' and 'go' as statement terminators in Neovim plugin. 3. Data file requirement: Removed mandatory data file requirement for --execute-statement mode. Now uses DUAL table when no data file is provided, allowing WEB CTEs and queries without data files to work correctly. Testing: - Verified with lowercase 'go' separators: correctly identifies statement numbers - Verified with WEB CTE scripts: no longer requires CSV data file - Verified dependency analysis: correctly shows "Executing statement #2 with 1 dependencies" Files changed: - nvim-plugin/lua/sql-cli/executor.lua: Fixed statement counting logic and case-insensitive GO matching - src/main.rs: Use DUAL table when no data file provided 🤖 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
Allows users to reference SELECT clause aliases in WHERE conditions, eliminating the need to repeat complex expressions. Before: SELECT a, a * 2 as double_a FROM t WHERE a * 2 > 10 -- Repetitive After: SELECT a, a * 2 as double_a FROM t WHERE double_a > 10 -- Clean! ## Implementation Added WhereAliasExpander transformer to preprocessing pipeline: - Extracts all aliases from SELECT clause - Recursively scans WHERE clause for column references - Replaces alias references with full expressions - Handles complex expressions (CASE, IN, functions, etc.) Order in pipeline: 1. ExpressionLifter (window functions) 2. WhereAliasExpander (NEW - alias expansion) 3. HavingAliasTransformer (aggregate aliases) 4. CTEHoister (CTE hoisting) 5. InOperatorLifter (IN optimization) ## Features - Works with arithmetic, string functions, complex expressions - Handles multiple aliases in complex WHERE conditions - Same performance as writing full expression (expansion at planning time) - No flags required - runs automatically ## Files Added - src/query_plan/where_alias_expander.rs (530 lines) * WhereAliasExpander transformer implementation * 7 unit tests covering various expansion scenarios - tests/integration/test_where_alias_expansion.sh * 8 integration tests * Tests basic, complex, and edge cases - scripts/demo_where_alias_expansion.sh * Interactive demo showcasing feature * Shows preprocessing pipeline in action ## Tests All tests passing: - 480 Rust unit tests (including 7 new transformer tests) - 397 integration tests - 511 Python tests - 8 new WHERE alias expansion integration tests Performance: ~0.02ms transformation overhead 🤖 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
Marked ORDER BY aggregate expressions as completed with full details. Updated transformer count from 6 to 7, reorganized priority matrix. **Completed Features:** - ✅ ORDER BY aggregate expressions (OrderByAliasTransformer) - ✅ Parser extended to support expressions in ORDER BY - ✅ Works in all execution modes (including --execute-statement) - ✅ 8 comprehensive examples with formal test coverage **Updated Priority Matrix:** - Moved ORDER BY to completed (was priority #2) - Re-prioritized remaining features - Added status column (Done, Next, Ready, Complex) **Next Priorities:** 1. DISTINCT in aggregates (high value, medium effort) 2. QUALIFY clause (nice syntax, easy effort) 3. ILIKE (trivial effort) Transformer count: 7 active (was 6) 🤖 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.