Repository navigation
fix(query): evaluate derived-table expressions instead of folding to NULL - #339
Closed
farhan-syah wants to merge 7 commits into
Closed
farhan-syah wants to merge 7 commits into
farhan-syah wants to merge 7 commits into
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.
Member
Author
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.
Symptom
22012overFROM (SELECT 1 AS x) sfolds to NULL. The real class is wider: any computed column, window function, or aggregate over a non-scan derived body (constant, aggregate, grouped, UNION) silently returns NULL, zero rows, or aggregates the base table instead of the derived body.Fix
ProviderScan/PostProcessSqlPlan::Subquerywindow_functionsfieldAggregate { input }, materialized on the coordinatorQueryOp::SetOp; merge logic hoisted out of pgwire into shared codeSELECT 1reportsint8ORDER BYOFFSETORDER BYon the provider pathTests
16 new wire tests in
nodedb/tests/wire/cases/sql_subquery_from.rs. Full suite green at branch head: stage 1 16440, stage 2 369.Not in this PR
SUM(int)renders15.0on every pathUNION/INTERSECT/EXCEPTnodedb-liteneeds aQueryOp::SetOparm and aSubqueryVisitArgs.window_functionsdestructure