Repository navigation
feat(query): evaluate derived-table expressions and per-row sequence calls - #340
Merged
Merged
Conversation
Add wire-level tests for SELECT over derived tables covering constant, aggregate, grouped, and UNION ALL bodies: computed-column projection, aggregate-of-aggregate, window functions, and division-by-zero propagation through projections, aggregate arguments, GROUP BY keys, and window PARTITION BY.
Move bridge-expression conversion, CTE inlining, and sort-key conversion into separate files under expr/, with mod.rs limited to module declarations and re-exports.
…erScan Carry serialized computed-column and window-function specs on the ScanProvider and Scan QueryOp variants, threading the new fields through every planner conversion path, exchange resolution, clone rewriting, and the cluster shuffle test fixtures that construct these plan nodes. Add provider_scan_compute to decode the row set to JSON, evaluate window functions over the full set, apply computed columns per row, and re-encode to msgpack; the executor runs this step after sort and before distinct, skipping it entirely when both byte slices are empty so a plain relational scan stays on the zero-decode msgpack path. Reorder the ProviderScan pipeline so offset runs after sort instead of before, matching ORDER BY ... OFFSET semantics.
Window functions previously ranked/aggregated rows in row-array arrival order, ignoring the spec's ORDER BY when it differed from how rows were already sorted. build_partitions and build_value_partitions now sort each partition's row indices by the spec's ORDER BY (with PostgreSQL-style NULL placement) after grouping, while leaving the row array itself untouched. The Value-native partition builder moves into its own value_partition module alongside the shared null_order helper.
Add a window_functions field to SqlPlan::Subquery and thread it through catalog folding/validation, the plan visitor, and CTE inlining, so a derived table or CTE reference carrying window specs merges or wraps correctly instead of dropping them silently. Subquery-to-physical-plan lowering now serializes computed columns and window specs for the tail, selecting qualified vs unqualified column references based on whether the body emits merged join/lateral documents. The ProviderScan executor evaluates windows and computed columns before sort instead of after, so ORDER BY can reference a window or computed alias.
Extend input-sourced aggregation, previously restricted to catalog
ProviderScan inputs, to any body with no routing collection: a
derived-table subquery, a constant result, or a UNION ALL. The
planner now routes such bodies through a shared input_sourced
lowering that materializes the body and builds a coordinator-local
Aggregate task, factored out of the catalog and single-collection
paths via build_input_sourced_aggregate_task.
extract_collection_name and join_side_collection return Option<String>
instead of an empty-string sentinel to distinguish a scan-shaped input
from one with no routing collection at all. The exchange resolver
gains an Aggregate{input: Some} arm that materializes the child
(reusing the PostProcess gather path, generalized as
materialize_child_rows) and runs it once on the coordinator's owning
core when the child itself reads no per-shard collection. The
executor's aggregate handler now fails the statement on a child error
instead of masking it as zero rows, and treats an undecodable
non-empty payload as an internal error rather than an empty result.
Add a coordinator-resolved SetOp physical plan node so UNION/UNION ALL/INTERSECT/EXCEPT bodies can sit under a subquery tail (ORDER BY, DISTINCT, OFFSET/LIMIT, aggregate) instead of only being reachable at the top level. Body-to-single-plan lowering is factored out of convert_subquery into a shared convert_body_to_single_plan, which now also recognizes a set-operation body and lowers it to the new SetOp node instead of rejecting multi-task bodies outright. The exchange resolver gains a SetOp arm that materializes each branch task, extracted alongside a shared materialize_child_rows helper, and dispatches to shared merge logic for UNION/UNION ALL/INTERSECT/EXCEPT. That merge logic — previously private to the pgwire response path — moves into a new set_op_merge module (union dedup, INTERSECT/EXCEPT row matching, and the row-key normalization they share) so both the pgwire routing path and the exchange resolver call the same implementation instead of duplicating it. Constant-result rows and cell values are now built as typed nodedb_types::Value/HashMap instead of a serde_json::Value tree before encoding to msgpack, matching how other response paths carry shaped rows.
A sequence accessor (nextval/currval) in a SELECT list over a FROM relation becomes Projection::CpComputed instead of being refused. The Control Plane evaluates it once per output row after the Data Plane returns the fetched columns, then drops the base columns it read and writes the alias. Every other row-scope clause (WHERE, ORDER BY, GROUP BY, HAVING, JOIN ON, SET, aggregate/window arguments, subqueries) still refuses the call, since the row evaluator has no sequence state there. TableScope tracks whether it backs the statement's output SELECT and whether it currently allows a Control-Plane function; neither flag is inherited by a nested scope, so a correlated subquery or a non-output SELECT keeps refusing. Aggregate projections that carry a Control-Plane-computed item are wrapped so ORDER BY/LIMIT keep operating on the underlying aggregate output. Plan cache eligibility now treats a Control-Plane-computed projection as data-dependent, since it allocates on every execution. Wire tests replace the old blanket-refusal cases with coverage for the allowed SELECT-list position and the row-scope clauses that still refuse.
KvOp::Scan carries projection and serialized computed-column bytes through the clone-source rewriter, planner, and native/RESP plan builders. The kv scan handler applies them to raw msgpack rows before sort, matching the document/columnar/timeseries scan handlers instead of returning NULL for computed expressions.
…ping Thread session sequence access (nextval/currval) through every path that shapes rows: kv/document/columnar scan responses, the HTTP materialized query route, native and pgwire dispatch/streaming, and RETURNING. shape_decoded_rows and shape_returning_rows take a sequences: Option<&dyn SequenceAccess> and stamp a projection's cp_computed columns onto the flat rows after redaction and before projection, so a SELECT-list sequence accessor resolves the same way regardless of which server path served the query. A caller with computed columns in scope but no session access fails the statement instead of shipping NULL under the alias. OutputSchema gains a cp_computed field populated wherever the planner builds an output schema, and a new nodedb::control::sequence::access module (SequenceAccess trait, SessionSequenceAccess) backs it, sharing error mapping extracted into sequence::error_map from the catalog adapter's existing sequence lookup. The HTTP materialized-query handler and the pgwire dispatch loop, both of which need the new sequence wiring, split from a single oversized file into a directory of focused modules (request parsing, row shaping, encoding for the former; task setup, the run loop, and finish handling for the latter).
Previously RETURNING only accepted a bare column list or star. The clause now resolves through the same path as a top-level SELECT list, via the new resolve_returning_items in nodedb-sql, so computed expressions and sequence accessors work in RETURNING and are evaluated per row by the Control Plane. Split the monolithic shared/returning.rs into a directory module (clause, strip, inject) grouped by concern, kept protocol-neutral across the pgwire planner, the neutral DDL UPSERT path, and the prepared-statement Describe path.
Extend keyed and predicate-filtered KV delete/update ops with a returning spec and RLS filter bytes, so the executor can read the stored pre-image (delete) or post-image (update) of every affected row and project it per spec instead of a bare row count. Thread the new fields through the physical plan, planner RLS injection, WAL replication encode/decode, response shaping, and transaction staging. Split the KV scan and transfer dispatch arms out of dispatch.rs into their own files to keep it within size limits alongside the new params.
Add if_present to KvOp::FieldSet so SQL UPDATE and the RESP hash-set family diverge on a missing key: UPDATE reports UPDATE 0 (RETURNING yields no rows) instead of materializing a row from nothing, while HSET keeps creating it. Thread the flag through the physical plan, planner conversion, WAL encode/decode, replication encode/decode, the executor's live and transaction-staged field-set paths, and WAL replay, so every read of the flag agrees on the same decision. Switch HSET's per-field encoding from JSON to msgpack so RESP writes and the SQL UPDATE lowering feed the field-set merge the same wire format.
HSET, SET, DEL, and GETSET matched on Status::Ok (or on Ok(_) alone) and fell back to a zero-ish reply (0 fields added, OK, 0 deleted, nil) whenever the dispatch payload didn't decode as expected, masking a rejected write behind a misleading success reply. Route every kv write dispatch through payload_or_typed_error so a rejected write returns the RESP error it actually is.
Cover CREATE/DROP/ALTER SEQUENCE, SHOW SEQUENCES, DESCRIBE SEQUENCE, the nextval/currval/setval functions, their error SQLSTATEs, and where a sequence accessor is allowed to run per query context.
farhan-syah
changed the base branch from
fix/constant-derived-expression-errors
to
main
September 18, 2026 09:46
farhan-syah
changed the title
# Conflicts: # nodedb-sql/src/planner/select/select_stmt.rs
farhan-syah
added
the
run-ci
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.
Closes #295. Closes #314.
One branch, two issues: #295 builds the Control-Plane / materialized-row evaluation seam; #314 rides on it.
#295 — expressions over derived tables
The reported symptom was
22012folding to NULL overFROM (SELECT 1 AS x) s. The class is wider: any computed column, window function, or aggregate over a non-scan derived body (constant, aggregate, grouped, UNION) silently returned NULL, zero rows, or aggregated the base table.ProviderScan/PostProcessSqlPlan::Subquerywindow_functionsfield;inline_cteno longer drops projections, windows, or inner aliases, and no longer merges an outerWHEREinto an innerLIMIT/DISTINCT/ORDER BYscanAggregate { input }, materialized on the coordinator; the Data Plane propagates a child error instead of aggregating zero rowsQueryOp::SetOp; the merge is hoisted out of pgwire intocontrol/server/set_op_merge/SELECT 1reportsint8ORDER BYOFFSETapplies afterORDER BY; coordinator-local bodies dispatch once instead of broadcasting to every core#314 — per-row sequence accessors
nextval/currval/setvalin a top-level SELECT list and in RETURNING evaluate once per output row, in output order, on the Control Plane after the rows are final. NewProjection::CpComputed,OutputSchema.cp_computed, and a stamp stage in the response shaper shared by pgwire, native, and HTTP; streaming is gated. Every other row-scope clause keeps0A000. Such plans are never cached.RETURNING items resolve through the real parser: bare columns, aliases, arithmetic, function calls, sequence accessors.
Defects found and fixed on the way
1 / (id - 2)was NULL on every row)KvOp::Scancarriescomputed_columns; the handler evaluates per rowUPDATE/DELETE … RETURNINGsilently dropped the clauseFieldSet/PredicateUpdate/Delete/PredicateDeletecarryreturning; post-image for UPDATE, pre-image for DELETEUPDATEon an absent KV key created the rowFieldSet.if_present; SQL UPDATE isUPDATE 0, RESPHSETstill createsHSETwrote nothing and replied0SET/DEL/GETSET/HSEThid a rejected write behindOK/0/nilTests
nodedb/tests/wire/cases/sql_subquery_from.rs— 16 derived-table casessql_sequence_row_scope.rs,sql_sequence_row_scope_refusals.rs,pgwire_returning_dml_kv.rs— newkv_sql_select.rs,pgwire_returning_dml.rs,resp_row_level_security.rs— additionsFull suite green at branch head: stage 1 16529, stage 2 369.
Docs
docs/query-language.md: new Sequences section; RETURNING section states expression support.Not in this PR
GAP_FREEis stored but inert:GapFreeManager::reservehas no caller. Wiring needs a decision on block-vs-fail-fast for the synchronous plan-time path.SUM(int)renders15.0on every path.UNION/INTERSECT/EXCEPT.42803).nodedb-liteneeds arms forQueryOp::SetOp,Projection::CpComputed, and aSubqueryVisitArgs.window_functionsdestructure; it has local uncommitted kv-adapter edits to review.