Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
1f8b9b5
fix(pgwire): batch multi-row INSERT into one page
EnRaiha Sep 13, 2026
22cddf3
fix(pgwire): answer kv INSERT with the INSERT tag
EnRaiha Sep 13, 2026
2e09cf6
fix(strict-format): coerce ARRAY vector literals
EnRaiha Sep 13, 2026
3856431
fix(executor): keep value errors out of XX000
EnRaiha Sep 13, 2026
8dbc520
fix(insert-select): scan source in its own engine
EnRaiha Sep 13, 2026
408cbf7
fix(sql): declare the SEARCH distance column
EnRaiha Sep 13, 2026
fb36857
feat(kv): evaluate scan projections per row
EnRaiha Sep 13, 2026
4df41f8
fix(sql): raise errors on constant derived tables
EnRaiha Sep 13, 2026
3a06467
fix(sql): name unaliased window projections
EnRaiha Sep 13, 2026
6f35a91
fix(sql): evaluate windows in the window's order
EnRaiha Sep 13, 2026
680c94e
fix(typeguard): resolve in the session database
EnRaiha Sep 13, 2026
a02bd2d
fix(convert): keep the source primary key
EnRaiha Sep 13, 2026
f916ce0
fix(convert): refuse a column-referencing guard
EnRaiha Sep 13, 2026
5b0159d
fix(sql): track token consumption in the graph DSL
EnRaiha Sep 14, 2026
36ebe5b
fix(sql): serve per-row nextval as a control-plane stamp
EnRaiha Sep 14, 2026
b47b5e9
fix(timeseries): convert instants once at shaping
EnRaiha Sep 14, 2026
436ef9b
fix(sql): connect the per-row nextval stamp to every response path
EnRaiha Sep 14, 2026
9027fe8
fix(sql): scale declared instants on a star read
EnRaiha Sep 14, 2026
290b597
fix(sql): collapse the star-path guard into one condition
EnRaiha Sep 14, 2026
f82f4e7
test(cluster): set the provider-scan fields the constructor now requires
EnRaiha Sep 14, 2026
0efb14d
fix(pgwire): keep a shaper error's own sqlstate
EnRaiha Sep 14, 2026
6df905f
fix(sql): record a per-row stamp in the session currval
EnRaiha Sep 14, 2026
c6bd324
fix(sql): evaluate a from-less nextval projection
EnRaiha Sep 14, 2026
9718510
fix(sql): keep one cell per duplicate constant column
EnRaiha Sep 14, 2026
469f1b8
chore(deps): update rustls to 0.23.45 (RUSTSEC-2026-0285)
EnRaiha Sep 14, 2026
fdba3e3
fix(sql): un-batch a cross-shard materialized-sum page
EnRaiha Sep 15, 2026
9fbacd7
fix(sql): keep window rows lossless and harden scan decode errors
EnRaiha Sep 15, 2026
b8d7879
fix(sql): keep window rows on the Value codec in every scan path
EnRaiha Sep 15, 2026
2e7775b
test(sql): keep projection test diagnostics off value contents
EnRaiha Sep 15, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@ fn producer_plan(rows: &[&Row]) -> Vec<u8> {
limit: None,
offset: 0,
distinct: false,
computed_columns: Vec::new(),
window_functions: Vec::new(),
});
let plan = PhysicalPlan::Query(QueryOp::PartialAggregateState {
collection: QualifiedCollection::new(nodedb_types::id::DatabaseId::DEFAULT, ""),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@ fn provider_scan_plan(rows: &[&Row]) -> Vec<u8> {
limit: None,
offset: 0,
distinct: false,
computed_columns: Vec::new(),
window_functions: Vec::new(),
});
plan_wire::encode(&plan).expect("encode provider scan plan")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@ fn provider_scan_plan(rows: &[Vec<u8>]) -> Vec<u8> {
limit: None,
offset: 0,
distinct: false,
computed_columns: Vec::new(),
window_functions: Vec::new(),
});
plan_wire::encode(&plan).expect("encode provider scan plan")
}
Expand Down
8 changes: 8 additions & 0 deletions nodedb-physical/src/physical_plan/kv/op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,14 @@ pub enum KvOp {
/// See `Get::surrogate_ceiling`; drops entries above the ceiling.
#[serde(default)]
surrogate_ceiling: Option<u32>,
/// Output column names (same format as DocumentOp::Scan). Empty =
/// return the whole row document.
#[serde(default)]
projection: Vec<String>,
/// Serialized `Vec<ComputedColumn>` applied per row after the scan
/// (same format as DocumentOp::Scan). Empty = none.
#[serde(default)]
computed_columns: Vec<u8>,
},

/// Set or update TTL on an existing key.
Expand Down
20 changes: 20 additions & 0 deletions nodedb-physical/src/physical_plan/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,18 @@ pub enum QueryOp {
/// Output column names to keep. Empty = emit all columns.
#[serde(default)]
projection: Vec<String>,
/// Serialized `Vec<ComputedColumn>` applied per row after
/// projection-name extraction (same wire format as the engine
/// scans). Expression projections over materialized rows (derived
/// tables, constant subqueries) need these — name-only projection
/// would silently drop them.
#[serde(default)]
computed_columns: Vec<u8>,
/// Serialized `Vec<WindowFuncSpec>` evaluated per partition after
/// computed columns (window over derived-table rows — issue #295
/// Gap 3). Empty = no window functions.
#[serde(default)]
window_functions: Vec<u8>,
/// ORDER BY terms, each an expression. Empty = unordered.
#[serde(default)]
sort_keys: Vec<crate::physical_plan::SortKeySpec>,
Expand Down Expand Up @@ -118,6 +130,14 @@ pub enum QueryOp {
/// Output column names to keep. Empty = emit all columns.
#[serde(default)]
projection: Vec<String>,
/// Serialized `Vec<ComputedColumn>` applied per row (see
/// `ProviderScan::computed_columns`).
#[serde(default)]
computed_columns: Vec<u8>,
/// Serialized `Vec<WindowFuncSpec>` (see
/// `ProviderScan::window_functions`).
#[serde(default)]
window_functions: Vec<u8>,
/// ORDER BY terms, each an expression. Empty = unordered.
#[serde(default)]
sort_keys: Vec<crate::physical_plan::SortKeySpec>,
Expand Down
98 changes: 93 additions & 5 deletions nodedb-query/src/window/eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,13 @@ pub fn evaluate_window_functions(
let partitions = build_partitions(rows, &spec.partition_by)?;

for partition_indices in &partitions {
// Evaluate the partition in the window's own ORDER BY — the
// query's sort, when present, orders by different keys. Without
// this, row_number numbered by arrival order and the rank family
// compared peers in the wrong sequence.
let ordered =
super::helpers::ordered_partition_indices(rows, partition_indices, &spec.order_by)?;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Blocker 2. This fix lands in evaluate_window_functions, the JSON evaluator. After b8d78795 no Origin path calls it. document/read/scan.rs and provider_scan.rs go through evaluate_window_functions_on_msgpack_rows, which calls evaluate_window_functions_value. That evaluator builds partitions in arrival order in build_value_partitions. apply_v_row_number numbers by that order. The scan sorts rows by the query's ORDER BY first. SELECT id, row_number() OVER (ORDER BY n DESC) FROM s ORDER BY id is still numbered by id.

The resubmit for #295 puts a Value version of ordered_partition_indices in evaluate_window_functions_value. Then remove the JSON evaluator or route it through the same helper.

let partition_indices = &ordered;
match spec.func_name.as_str() {
"row_number" => apply_row_number(rows, partition_indices, &spec.alias),
"rank" => apply_rank(rows, partition_indices, &spec.alias, &spec.order_by)?,
Expand Down Expand Up @@ -134,6 +141,84 @@ mod tests {
assert_eq!(rows[4].1["rn"], json!(2));
}

/// `row_number() OVER (ORDER BY ...)` must number by the window's own
/// ordering, not by arrival order: arrival is salary 100, 120, 90; ORDER
/// BY salary DESC must give 120 → 1, 100 → 2, 90 → 3.
#[test]
fn row_number_follows_the_window_ordering() {
let mut rows = make_rows();
let spec = WindowFuncSpec {
alias: "rn".into(),
func_name: "row_number".into(),
args: vec![],
partition_by: vec![],
order_by: vec![(SqlExpr::Column("salary".into()), false)],
frame: WindowFrame::default(),
};
evaluate_window_functions(&mut rows, &[spec]).unwrap();
assert_eq!(rows[1].1["rn"], json!(1), "120 is the highest salary");
assert_eq!(rows[4].1["rn"], json!(2), "110 is second");
assert_eq!(rows[0].1["rn"], json!(3), "100 is third");
assert_eq!(rows[2].1["rn"], json!(4), "90 is fourth");
assert_eq!(rows[3].1["rn"], json!(5), "80 is fifth");
}

/// An ORDER BY key that divides by zero fails the query instead of being
/// skipped along with the ordering.
#[test]
fn row_number_ordering_error_propagates() {
let mut rows = make_rows();
let spec = WindowFuncSpec {
alias: "rn".into(),
func_name: "row_number".into(),
args: vec![],
partition_by: vec![],
order_by: vec![(
SqlExpr::BinaryOp {
left: Box::new(SqlExpr::Column("salary".into())),
op: crate::expr::BinaryOp::Div,
right: Box::new(SqlExpr::Literal(nodedb_types::Value::Integer(0))),
},
true,
)],
frame: WindowFrame::default(),
};
let err = evaluate_window_functions(&mut rows, &[spec])
.expect_err("ordering by a division by zero must fail");
assert!(
matches!(err, crate::expr::EvalError::DivisionByZero),
"unexpected error: {err:?}"
);
}

/// A singleton partition still evaluates its ordering key: the result
/// cannot change, but the error must be raised like anywhere else.
#[test]
fn singleton_partition_ordering_error_propagates() {
let mut rows = vec![("1".to_string(), json!({"n": 1}))];
let spec = WindowFuncSpec {
alias: "rn".into(),
func_name: "row_number".into(),
args: vec![],
partition_by: vec![],
order_by: vec![(
SqlExpr::BinaryOp {
left: Box::new(SqlExpr::Column("n".into())),
op: crate::expr::BinaryOp::Div,
right: Box::new(SqlExpr::Literal(nodedb_types::Value::Integer(0))),
},
true,
)],
frame: WindowFrame::default(),
};
let err = evaluate_window_functions(&mut rows, &[spec])
.expect_err("a division by zero in the ordering key must fail");
assert!(
matches!(err, crate::expr::EvalError::DivisionByZero),
"unexpected error: {err:?}"
);
}

#[test]
fn running_sum() {
let mut rows = make_rows();
Expand All @@ -146,11 +231,14 @@ mod tests {
frame: WindowFrame::default(),
};
evaluate_window_functions(&mut rows, &[spec]).unwrap();
assert_eq!(rows[0].1["running_total"], json!(100.0));
assert_eq!(rows[1].1["running_total"], json!(220.0));
assert_eq!(rows[2].1["running_total"], json!(310.0));
assert_eq!(rows[3].1["running_total"], json!(80.0));
assert_eq!(rows[4].1["running_total"], json!(190.0));
// Partition `eng` in salary order is 90, 100, 120 and `sales` is 80,
// 110. Arrival order (100, 120, 90 / 80, 110) is NOT the window's
// order, so the running total must follow the sorted sequence.
assert_eq!(rows[2].1["running_total"], json!(90.0), "eng 90 first");
assert_eq!(rows[0].1["running_total"], json!(190.0), "eng +100");
assert_eq!(rows[1].1["running_total"], json!(310.0), "eng +120");
assert_eq!(rows[3].1["running_total"], json!(80.0), "sales 80 first");
assert_eq!(rows[4].1["running_total"], json!(190.0), "sales +110");
}

#[test]
Expand Down
88 changes: 88 additions & 0 deletions nodedb-query/src/window/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,3 +82,91 @@ pub(super) fn order_keys_equal(
}
Ok(true)
}

/// Indices of one partition, ordered by a window spec's ORDER BY keys.
///
/// Window functions receive the sorted result set, but a window's own ORDER
/// BY is a different ordering from the query's — nothing upstream sorts by
/// it. Evaluating a partition in arrival order silently assigned
/// row numbers by arrival and compared peers in the wrong sequence, so every
/// ranking and running frame was wrong whenever the two orders differed.
///
/// Keys are evaluated once per row; a division/modulo-by-zero propagates as
/// `Err(EvalError::DivisionByZero)`. The sort is stable, so rows with equal
/// keys keep their arrival order.
pub(super) fn ordered_partition_indices(
rows: &[(String, serde_json::Value)],
indices: &[usize],
order_by: &[(SqlExpr, bool)],
) -> Result<Vec<usize>, crate::expr::EvalError> {
if order_by.is_empty() {
return Ok(indices.to_vec());
}
// Keys are evaluated even for a one-row partition: the ordering cannot
// change the result, but a key that divides by zero is still a statement
// error in PostgreSQL, and skipping the evaluation would answer where
// the same expression anywhere else fails.
let mut keyed: Vec<(usize, Vec<serde_json::Value>)> = Vec::with_capacity(indices.len());
for &i in indices {
let mut keys = Vec::with_capacity(order_by.len());
for (expr, _) in order_by {
keys.push(eval_expr_on_json(expr, &rows[i].1)?);
}
keyed.push((i, keys));
}
keyed.sort_by(|(_, a), (_, b)| compare_key_values(a, b, order_by));
Ok(keyed.into_iter().map(|(i, _)| i).collect())
}

/// Compare two pre-evaluated ORDER BY key vectors under their direction flags.
fn compare_key_values(
a: &[serde_json::Value],
b: &[serde_json::Value],
order_by: &[(SqlExpr, bool)],
) -> std::cmp::Ordering {
for ((va, vb), (_expr, ascending)) in a.iter().zip(b.iter()).zip(order_by.iter()) {
let ord = compare_with_direction(va, vb, *ascending);
if ord != std::cmp::Ordering::Equal {
return ord;
}
}
std::cmp::Ordering::Equal
}

/// Compare two key values, applying the window ORDER BY null convention:
/// NULLS LAST under ASC, NULLS FIRST under DESC.
fn compare_with_direction(
a: &serde_json::Value,
b: &serde_json::Value,
ascending: bool,
) -> std::cmp::Ordering {
use serde_json::Value as J;
use std::cmp::Ordering;

let base = match (a, b) {
(J::Null, J::Null) => Ordering::Equal,
(J::Null, _) => Ordering::Greater,
(_, J::Null) => Ordering::Less,
_ => json_rank(a).cmp(&json_rank(b)).then_with(|| {
if let (Some(x), Some(y)) = (as_f64(a), as_f64(b)) {
x.partial_cmp(&y).unwrap_or(Ordering::Equal)
} else {
a.to_string().cmp(&b.to_string())
}
}),
};
if ascending { base } else { base.reverse() }
}

/// Rank JSON kinds so mixed-type keys still compare deterministically.
fn json_rank(v: &serde_json::Value) -> u8 {
use serde_json::Value as J;
match v {
J::Null => 0,
J::Bool(_) => 1,
J::Number(_) => 2,
J::String(_) => 3,
J::Array(_) => 4,
J::Object(_) => 5,
}
}
Loading
Loading