BE-641: Extend the SQL AST along gram.y and make invalid SQL unrepresentable#9033
BE-641: Extend the SQL AST along gram.y and make invalid SQL unrepresentable#9033TimDiekmann wants to merge 20 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #9033 +/- ##
==========================================
+ Coverage 59.19% 59.31% +0.11%
==========================================
Files 1389 1393 +4
Lines 135001 135212 +211
Branches 6183 6183
==========================================
+ Hits 79910 80196 +286
+ Misses 54151 54086 -65
+ Partials 940 930 -10
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:
|
…h infallible single setters
a2a0e6d to
0a85270
Compare
PR SummaryMedium Risk Overview
Reviewed by Cursor Bugbot for commit 6753875. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 644b7be. Configure here.
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?
Extends the SQL statement AST in
hash-graph-postgres-storeso that every Postgres statement we need is expressible in typed Rust, modelled strictly along Postgres' grammar (gram.y): at most the grammar per node — invalid SQL is unrepresentable — with completeness added on demand. On top of the structural work, theSelectCompilerno longer uses the AST as mutable working state: it keeps its own bookkeeping and assembles the statement incompile(), which is the groundwork for the BE-598 query-shape changes.Stacked on #9020 (module restructure); only the commits of this branch are new.
🔗 Related links
🚫 Blocked by
🔍 What does this change?
Grammar-faithful AST (per node: at most gram.y):
WITH: CTEs with plain [TableName]s,MATERIALIZED/NOT MATERIALIZEDhints, optional column lists, andWITH RECURSIVE; the clause holds aNonEmptyVec<CommonTableExpression>SimpleSelect(simple_select),SelectClause(set-operation tree:UNION/INTERSECT/EXCEPT [ALL], operands parenthesized so Postgres re-parses the exact tree),SelectStatement(select_no_parens:WITH/ORDER BY/LIMIT/OFFSET) — clause placement rules are now type-structural (noORDER BYon a bare set-operation operand)GROUP BY [ALL|DISTINCT] grouping_element [, …]withGroupingElement::{Empty, Expressions}(ROLLUP/CUBE/GROUPING SETS fenced off),HAVING, hardenedWindowDefinition(PARTITION BYonly, non-empty),TABLESAMPLEwith finiteSamplePercentageWHEREisOption<Expression>(the grammar's[ WHERE condition ]);ORDER BYuses AST-ownSortBy/SortDirection/NullsOrder(gram.ysortby), decoupling the AST from store typesNonEmptyVec<T>as the list-invariant helper (grow-only, no shrink API,Deref<[T]>); target lists, CTE lists,DISTINCT ON,PARTITION BY, and sort lists are non-empty by constructionORDER BYwas emitted beforeGROUP BY(never hit because no statement set both)Builder composition:
#[builder(derive(Into))]conversion chains: builders flow intoimpl Intoslots,.build()only remains at the outermost edge;NonEmptyVecmembers use plain#[builder(into)]setters (single element and ready-made vec convert infallibly, a rawVecparses visibly viaNonEmptyVec::try_from)Compiler decomposition:
SelectCompilerkeepsselects/from/quantifier/with/conditions/cursor/sort_by/limitas its own fields and assembles the statement incompile(); join bookkeeping lives in mirroredCompiledJoinrecords instead of reading the statement tree back, andTable::aliased_nameis the single owner of alias-name manglingExpressiontree;nulls: Nonenow mirrors Postgres' actual default (ASC →NULLS LAST, DESC →NULLS FIRST) — previously unhinted ascending sorts over nullable keys silently lost theirNULLrows during pagination. ProvablyNOT NULLsort keys (verified whitelistColumn::NON_NULL_COLUMNS) skip the null-handling arms, keeping the hot default-sort predicates bareIdentifiers panic at construction, parameters with pending conversions returnPendingParameterConversioninstead ofunimplemented!, and a second embeddings distance filter is rejected via an explicit flag (previously it silently reinstalled the subquery with mismatched parameters)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:
WHEREconjuncts are now individually parenthesized and the cursor continuation is an expression tree. Semantically identical (validated by executing the hashql E2E suite and the integration tests against real Postgres), butpg_stat_statementswill mint new query IDs — dashboards comparing against old fingerprints reset.WHEREbuilder (empty conditions combined with a cursor producedWHERE AND (…)/ a danglingWHERE).LIMIT/OFFSETas expressions (LIMIT $1),FETCH, locking clauses,VALUES/TABLE/parenthesized set-operation operands, the statement-levelWINDOWclause, ROLLUP/CUBE/GROUPING SETS,ORDER BY … USING op,SEARCH/CYCLE.FromItemstill allows aUSINGjoin with an empty column list (degrades toON TRUE, documented); the structural fix is bundled with the BE-528column_definitionswork, which touches the same grammar production.🐾 Next steps
EXISTS,IN (SELECT …)) as the last planned AST blockcolumn_definitionsfor typed JSON +AliasClausepairing /USINGnon-emptinessColumn::nullable()or generate the schema mirror from migrations🛡 What tests cover this?
hash-graph-postgres-store(golden SQL for every compiler shape, allcursor_conditionbranches — previously untested —, per-clause transpile tests,NonEmptyVecinvariants)tests/schema.rs: verifiesColumn::NON_NULL_COLUMNSagainstinformation_schemaof the live databaseNULLS FIRST/LASTmixes, cursor loops against real data)❓ How to test this?
cargo nextest run --package hash-graph-postgres-store --libcargo nextest run --package hashql-eval(needs Docker for testcontainers)cargo nextest run --package hash-graph-postgres-store --test schemaandcargo nextest run -p hash-graph-integration --test postgres -E 'test(sorting)'📹 Demo