Skip to content

Refactor QueryPlanner - #48

Merged
LiaCastaneda merged 3 commits into
mainfrom
lia/refactor-QueryPlanner
Jul 18, 2025
Merged

Refactor QueryPlanner#48
LiaCastaneda merged 3 commits into
mainfrom
lia/refactor-QueryPlanner

Conversation

@LiaCastaneda

@LiaCastaneda LiaCastaneda commented Jul 18, 2025

Copy link
Copy Markdown
Collaborator

Refactors QueryPlanner to avoid distributing tasks in prepare. All the logic for resolving and sending the plan to workers is now handled within the distribute_plan function. This decouples the logic and makes it easier to test.

Tested the refactor by launching the TPCH cluster and doing explain, select & describe statements

@LiaCastaneda
LiaCastaneda force-pushed the lia/refactor-QueryPlanner branch from 869b973 to 5c12df4 Compare July 18, 2025 10:54
Comment thread src/query_planner.rs Outdated
@LiaCastaneda
LiaCastaneda marked this pull request as ready for review July 18, 2025 12:21
Comment thread src/query_planner.rs Outdated
Comment thread src/query_planner.rs

@NGA-TRAN NGA-TRAN left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice @LiaCastaneda 🎉 And Thanks @robtandy for the suggestions.

This refactor lays the groundwork for expanding our unit test coverage and sets up a clear path for introducing integration tests. With these changes, we’re getting close to running TPC-H validation directly—no need to route through Python

@LiaCastaneda
LiaCastaneda force-pushed the lia/refactor-QueryPlanner branch from 508f77a to 5af6cf5 Compare July 18, 2025 14:01
Comment thread src/query_planner.rs
Comment thread src/query_planner.rs Outdated
@LiaCastaneda
LiaCastaneda merged commit 6851fb4 into main Jul 18, 2025
3 checks passed
@LiaCastaneda
LiaCastaneda deleted the lia/refactor-QueryPlanner branch July 18, 2025 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants