BE-641: Restructure the SQL AST into statement/clause/expression modules#9020
BE-641: Restructure the SQL AST into statement/clause/expression modules#9020TimDiekmann wants to merge 3 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
PR SummaryLow Risk Overview Clause-shaped types that lived under the old The public Reviewed by Cursor Bugbot for commit 4a6acd2. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
Pull request overview
Reorganizes the PostgreSQL query AST into dedicated statement, clause, and expression modules without changing SQL compilation behavior.
Changes:
- Introduces the
query::asthierarchy and updates imports/re-exports. - Splits expression primitives and relocates window expressions.
- Moves
SelectCompilertests beside the compiler implementation.
Reviewed changes
Copilot reviewed 19 out of 29 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
query/table.rs |
Updates AST imports. |
query/statement/select.rs |
Removes the former select module and embedded tests. |
query/mod.rs |
Registers and re-exports the new AST structure. |
query/expression/mod.rs |
Removes the legacy expression module layout. |
query/compile/tests.rs |
Houses relocated compiler tests. |
query/compile/mod.rs |
Registers tests and updates AST imports. |
query/ast/table_reference.rs |
Relocates table-reference nodes. |
query/ast/statement/select.rs |
Houses the select statement AST. |
query/ast/statement/mod.rs |
Defines statement exports. |
query/ast/statement/insert.rs |
Updates insert-node imports. |
query/ast/mod.rs |
Defines the unified AST export surface. |
query/ast/identifier.rs |
Relocates identifier handling. |
query/ast/expression/window.rs |
Relocates window expressions. |
query/ast/expression/variadic.rs |
Relocates variadic expressions. |
query/ast/expression/unary.rs |
Relocates unary expressions. |
query/ast/expression/mod.rs |
Organizes expression nodes. |
query/ast/expression/function.rs |
Extracts function expressions. |
query/ast/expression/constant.rs |
Extracts constant expressions. |
query/ast/expression/binary.rs |
Relocates binary expressions. |
query/ast/column_reference.rs |
Updates table-reference imports. |
query/ast/clause/with.rs |
Updates CTE clause imports. |
query/ast/clause/where_clause.rs |
Updates expression transpiler imports. |
query/ast/clause/table_sample.rs |
Relocates table-sampling clauses. |
query/ast/clause/select_list.rs |
Updates select-list imports. |
query/ast/clause/order_by.rs |
Relocates ordering clauses. |
query/ast/clause/mod.rs |
Defines clause exports. |
query/ast/clause/join_type.rs |
Relocates join types. |
query/ast/clause/group_by.rs |
Relocates grouping clauses. |
query/ast/clause/from_item.rs |
Updates FROM-item module imports. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9020 +/- ##
==========================================
- Coverage 59.73% 59.39% -0.34%
==========================================
Files 1371 1373 +2
Lines 135461 134345 -1116
Branches 6066 6066
==========================================
- Hits 80911 79796 -1115
+ Misses 53618 53617 -1
Partials 932 932
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Benchmark results
|
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| resolve_policies_for_actor | user: empty, selectivity: high, policies: 2002 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: medium, policies: 1002 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: high, policies: 3314 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: medium, policies: 1527 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: high, policies: 2078 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: medium, policies: 1033 | Flame Graph |
policy_resolution_medium
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| resolve_policies_for_actor | user: empty, selectivity: high, policies: 102 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: medium, policies: 52 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: high, policies: 269 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: medium, policies: 108 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: high, policies: 133 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: medium, policies: 63 | Flame Graph |
policy_resolution_none
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| resolve_policies_for_actor | user: empty, selectivity: high, policies: 2 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: medium, policies: 2 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: high, policies: 8 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: medium, policies: 3 | Flame Graph |
policy_resolution_small
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| resolve_policies_for_actor | user: empty, selectivity: high, policies: 52 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: empty, selectivity: medium, policies: 26 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: high, policies: 94 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: seeded, selectivity: medium, policies: 27 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: high, policies: 66 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: low, policies: 1 | Flame Graph | |
| resolve_policies_for_actor | user: system, selectivity: medium, policies: 29 | Flame Graph |
read_scaling_complete
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| entity_by_id;one_depth | 1 entities | Flame Graph | |
| entity_by_id;one_depth | 10 entities | Flame Graph | |
| entity_by_id;one_depth | 25 entities | Flame Graph | |
| entity_by_id;one_depth | 5 entities | Flame Graph | |
| entity_by_id;one_depth | 50 entities | Flame Graph | |
| entity_by_id;two_depth | 1 entities | Flame Graph | |
| entity_by_id;two_depth | 10 entities | Flame Graph | |
| entity_by_id;two_depth | 25 entities | Flame Graph | |
| entity_by_id;two_depth | 5 entities | Flame Graph | |
| entity_by_id;two_depth | 50 entities | Flame Graph | |
| entity_by_id;zero_depth | 1 entities | Flame Graph | |
| entity_by_id;zero_depth | 10 entities | Flame Graph | |
| entity_by_id;zero_depth | 25 entities | Flame Graph | |
| entity_by_id;zero_depth | 5 entities | Flame Graph | |
| entity_by_id;zero_depth | 50 entities | Flame Graph |
read_scaling_linkless
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| entity_by_id | 1 entities | Flame Graph | |
| entity_by_id | 10 entities | Flame Graph | |
| entity_by_id | 100 entities | Flame Graph | |
| entity_by_id | 1000 entities | Flame Graph | |
| entity_by_id | 10000 entities | Flame Graph |
representative_read_entity
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/block/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/book/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/building/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/organization/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/page/v/2
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/person/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/playlist/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/song/v/1
|
Flame Graph | |
| entity_by_id | entity type ID: https://blockprotocol.org/@alice/types/entity-type/uk-address/v/1
|
Flame Graph |
representative_read_entity_type
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| get_entity_type_by_id | Account ID: bf5a9ef5-dc3b-43cf-a291-6210c0321eba
|
Flame Graph |
representative_read_multiple_entities
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| entity_by_property | traversal_paths=0 | 0 | |
| entity_by_property | traversal_paths=255 | 1,resolve_depths=inherit:1;values:255;properties:255;links:127;link_dests:126;type:true | |
| entity_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:0;links:0;link_dests:0;type:false | |
| entity_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:0;links:1;link_dests:0;type:true | |
| entity_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:2;links:1;link_dests:0;type:true | |
| entity_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:2;properties:2;links:1;link_dests:0;type:true | |
| link_by_source_by_property | traversal_paths=0 | 0 | |
| link_by_source_by_property | traversal_paths=255 | 1,resolve_depths=inherit:1;values:255;properties:255;links:127;link_dests:126;type:true | |
| link_by_source_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:0;links:0;link_dests:0;type:false | |
| link_by_source_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:0;links:1;link_dests:0;type:true | |
| link_by_source_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:0;properties:2;links:1;link_dests:0;type:true | |
| link_by_source_by_property | traversal_paths=2 | 1,resolve_depths=inherit:0;values:2;properties:2;links:1;link_dests:0;type:true |
scenarios
| Function | Value | Mean | Flame graphs |
|---|---|---|---|
| full_test | query-limited | Flame Graph | |
| full_test | query-unlimited | Flame Graph | |
| linked_queries | query-limited | Flame Graph | |
| linked_queries | query-unlimited | Flame Graph |
🌟 What is the purpose of this PR?
First step of BE-641 (making every Postgres statement we need expressible in typed Rust): restructure the SQL AST inside the postgres-store so statements, clauses, and expressions live in dedicated modules and the compiler is clearly separated from the AST it builds. Pure relocation — no behavior, signature, or logic changes. This clears the ground for the follow-up that extends the AST (free CTE names,
MATERIALIZED, set operations, subquery predicates), which in turn unblocks the fetch-keys-then-hydrate query shape (BE-598).🔗 Related links
🔍 What does this change?
Best reviewed commit by commit — the first commit is pure file moves (GitHub shows them as renames), the second moves content between files:
query/{expression,statement}/→query/ast/{statement,clause,expression}/, splitting the formerexpression/module (which mostly contained clauses) into the classic statement / clause / expression triad.compile.rsbecomescompile/mod.rs.conditional.rs, whose name hid the centralExpressionenum) is split intoexpression/{mod,function,constant}.rs.SelectCompilerintegration tests that lived instatement/select.rsmove tocompile/tests.rs, next to the code they test.select.rsis now just theSelectStatementnode.WindowStatementmoves fromstatement/toexpression/(it is only used insideExpression::Window).query/mod.rsis preserved, so no consumers (includinghashql-eval, which emits through this AST) needed changes.GroupByExpression/OrderByExpressionare newly part of the flat re-exports — they were previously only reachable via nested module paths.Pre-Merge Checklist 🚀
🚢 Has this modified a publishable library?
This PR:
📜 Does this require a change to the docs?
The changes in this PR:
🕸️ Does this require a change to the Turbo Graph?
The changes in this PR:
🐾 Next steps
MATERIALIZED/NOT MATERIALIZED, set operations (UNION/INTERSECT/EXCEPT), andEXISTS/IN (SELECT …)predicatesSelectCompilerinto a compositional consumer of the AST (incl. reusing relations instead of duplicate self-joins), then the BE-598 three-phase query shape🛡 What tests cover this?
SelectCompilerintegration tests) runs unchanged and green — no assertions were modifiedhashql-evalcompiles against the unchanged re-export surface❓ How to test this?
cargo nextest run -p hash-graph-postgres-store --libcargo clippy -p hash-graph-postgres-store --all-features --all-targets