Refactor v32 cleanup migrated fields #3
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
- Comment out fields already migrated to AppStateContainer: - help_scroll (migrated to ScrollState) - input_scroll_offset (migrated to ScrollState) - scroll_offset (migrated to ScrollState) - undo_stack/redo_stack (migrated to UndoRedoState) - last_visible_rows (migrated to ScrollState) - jump_to_row_input (migrated to JumpToRowState) - Update remaining references to use AppStateContainer methods - All functionality preserved, just using centralized state This completes the V32 cleanup of redundant state fields.
- Remove old state struct definitions: - FilterState (migrated to AppStateContainer) - ColumnSearchState (migrated to AppStateContainer) - SearchState (migrated to AppStateContainer) - CompletionState (migrated to AppStateContainer) - Remove fallback_filter_state field and related methods - Update all references to use state_container methods - Fix filter highlighting to work with new FilterState structure This completes the state migration cleanup, removing all duplicate state management code in favor of the centralized AppStateContainer.
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 26, 2025
Allows users to reference SELECT clause aliases in GROUP BY, eliminating the need to repeat complex expressions and completing the trilogy of alias expansion (WHERE, GROUP BY, HAVING). Before: SELECT id % 3, COUNT(*) FROM t GROUP BY id % 3 -- Repetitive After: SELECT id % 3 as grp, COUNT(*) FROM t GROUP BY grp -- Clean! ## Implementation Added GroupByAliasExpander transformer to preprocessing pipeline: - Extracts all aliases from SELECT clause - Scans GROUP BY clause for column references - Replaces alias references with full expressions - Simple and focused: only expands simple column references Order in pipeline: 1. ExpressionLifter (window functions) 2. WhereAliasExpander (WHERE aliases) 3. GroupByAliasExpander (GROUP BY aliases - NEW) 4. HavingAliasTransformer (HAVING aggregates) 5. CTEHoister (CTE hoisting) 6. InOperatorLifter (IN optimization) ## Features - Works with arithmetic, modulo, functions, complex expressions - Handles multiple aliases in GROUP BY - Works seamlessly with HAVING and ORDER BY - Fixes previously failing tests that used GROUP BY aliases - No performance penalty (expansion at planning time) - No flags required - runs automatically ## Files Added - src/query_plan/group_by_alias_expander.rs (390 lines) * GroupByAliasExpander transformer implementation * 7 unit tests covering various expansion scenarios - tests/integration/test_group_by_alias_expansion.sh * 8 integration tests * Tests basic, complex, and combo scenarios with HAVING - scripts/demo_group_by_alias_expansion.sh * Interactive demo showcasing feature * Shows all three alias transformers working together ## Tests All tests passing: - 487 Rust unit tests (including 7 new transformer tests) - 397 integration tests - 511 Python tests - 8 new GROUP BY alias expansion integration tests Previously failing HAVING tests now work: SELECT id % 3 as grp, COUNT(*) FROM t GROUP BY grp HAVING COUNT(*) > 2 Performance: ~0.02ms transformation overhead ## Quick Wins Completed 1. ✅ HAVING auto-aliasing (aggregates in HAVING) 2. ✅ WHERE alias expansion (SELECT aliases in WHERE) 3. ✅ GROUP BY alias expansion (SELECT aliases in GROUP BY) The three core SQL clauses that need alias expansion are now complete! 🤖 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>
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.