refactor: Decompose query_parser.py into query_parser_helpers package - #67
Merged
Conversation
Step 1 of query_parser.py decomposition: - Add query_parser_helpers/ package with __init__.py - Create stub files for all 10 helper classes: - FromClauseParser, JoinHandler, SpecialSourcesHandler - SetOperationsParser, PivotUnpivotParser, MergeParser - WindowFunctionsParser, GroupingParser - RecursiveCTEParser, SubqueryParser - Include dependency graph documentation in __init__.py - Define __all__ exports for better IDE support This prepares the structure for composition-based decomposition following the hybrid approach from the architectural plan.
…SourcesHandler Step 2 of query_parser.py decomposition: - Create FromClauseParser (~330 lines) for FROM clause orchestration - Create SpecialSourcesHandler (~516 lines) for UNNEST, TVF, VALUES, LATERAL - Update _parse_from_sources to delegate to FromClauseParser.parse() - Remove old nested function implementation (~750 lines) query_parser.py: 2,313 → 1,567 lines (32% reduction) All 1052 tests pass.
Step 3 of query_parser.py decomposition: - Create WindowFunctionsParser (~320 lines) for window function parsing - Extract 7 window-related methods: - _parse_window_functions, _parse_single_window, _parse_window_spec - _parse_order_by_column, _parse_frame_spec, _format_frame_boundary - _extract_window_columns - Update _parse_window_functions to delegate to WindowFunctionsParser.parse() query_parser.py: 1,567 → 1,282 lines (18% further reduction) Total reduction from original: 2,313 → 1,282 lines (45% reduction) All 1052 tests pass.
Step 4 of query_parser.py decomposition: - Implemented RecursiveCTEParser with full recursive CTE parsing logic (~287 lines) - Handles WITH RECURSIVE common table expressions - Includes is_recursive_cte() check and parse() for base/recursive components - Encapsulates _find_self_reference and _extract_select_column_names helper methods - Updated query_parser.py to delegate via _is_recursive_cte() and _parse_recursive_cte() - All 1052 tests pass - query_parser.py reduced from 1282 to 1042 lines (~240 lines removed)
Step 5 of query_parser.py decomposition - implement helper stubs: - SubqueryParser (~185 lines): WHERE/HAVING/SELECT subqueries, QUALIFY clause - GroupingParser (~155 lines): GROUPING SETS, CUBE, ROLLUP - SetOperationsParser (~168 lines): UNION, INTERSECT, EXCEPT - PivotUnpivotParser (~228 lines): PIVOT and UNPIVOT operations - MergeParser (~196 lines): MERGE INTO statements All helpers follow composition pattern with parent parser reference. All 1052 tests pass.
Step 6 of query_parser.py decomposition - delegate to helper classes: - SetOperationsParser: UNION, INTERSECT, EXCEPT - PivotUnpivotParser: PIVOT and UNPIVOT operations - MergeParser: MERGE INTO statements - GroupingParser: GROUPING SETS, CUBE, ROLLUP - SubqueryParser: WHERE/HAVING/SELECT subqueries, QUALIFY clause query_parser.py reduced from 2,313 lines to 391 lines (83% reduction) All 1052 tests pass Helper classes initialized in __init__: - _from_clause, _special_sources, _window_functions, _recursive_cte - _set_operations, _pivot_unpivot, _merge, _grouping, _subqueries
JOIN parsing is fully handled by FromClauseParser (_process_join_source and the joins loop in parse()). JoinHandler was scaffolding created in step 1 of the decomposition and never implemented — process_joins() only raised NotImplementedError, and its signature omitted the preceding_tables argument the real JOIN path requires. - Delete query_parser_helpers/join_handler.py - Drop the import and __all__ entry from the package - Correct the dependency-graph docstring No call sites existed; 1052 tests pass unchanged.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Decomposes
query_parser.pyfrom 2,498 → 558 lines (78% reduction) by extracting nine helper classes into a newquery_parser_helpers/package, wired in via composition.This continues the systematic file-decomposition plan (v2.1) that followed the architectural review Items 7–10.
pipeline.pyandlineage_builder.pyare untouched by this PR.SpecialSourcesHandlerWindowFunctionsParserFromClauseParserRecursiveCTEParserWITH RECURSIVEbase/recursive componentsPivotUnpivotParserMergeParserMERGE INTOSubqueryParserSetOperationsParserGroupingParserEach helper receives a reference to the parent
RecursiveQueryParserfor shared state. No public API changes —RecursiveQueryParserkeeps the same surface and delegates internally.Notes for reviewers
This work was originally developed on an older base and has been rebased onto current
main. Two features that landed onmainin the interim sit inside code paths that moved into helpers, so they were ported by hand rather than resolved mechanically:7b98e6e) —ranking_window_columnspopulation moved intoWindowFunctionsParser.parse()d1a51c1, refs MERGE condition columns missing from column lineage (Gaps 3, 9, 10) #63) —match_filter_columnsfor literal-bound MERGE ON predicates moved intoMergeParser.parse(), including the new config keyBoth are covered by existing assertions (
test_merge_statements.py:159,test_subquery_dedup_qualify.pyGroup 5), which would fail if either port had been dropped.Also included: removal of the
JoinHandlerstub. It was scaffolded in the first commit but never implemented —process_joins()only raisedNotImplementedError, and its signature omitted thepreceding_tablesargument the real path requires. JOIN parsing lives inFromClauseParser(_process_join_source). No call sites existed.One incidental fix:
ListandTuplerestored to thetypingimports, still required by_extract_join_predicate_columns.Test plan
origin/mainbaseline captured before any changesruff checkandruff format --checkcleantytype checker clean (via pre-commit)