Skip to content

Commit 7464d2c

Browse files
committed
fix(sql): refuse per-row sequence accessors instead of returning NULL
nextval/currval/setval only evaluate through SqlCatalog's sequence state at plan time, in a FROM-less SELECT or a column DEFAULT. A call over a FROM relation instead reached the row evaluator, which holds no sequence state and silently returned NULL for every row. Name the three accessor names once in a shared sequence_accessor module and use it from both the constant folder and the new resolver gate. TableScope::is_row_scope and ColumnScope::is_row_scope report whether an expression sits behind a relation; convert_function_depth refuses a sequence accessor there with the new SqlError::SequencePerRowUnsupported. Wire the refusal through to SQLSTATE 0A000 (feature_not_supported) on both the plan-error and pgwire error-mapping paths via a new crate::Error::FeatureNotSupported, and through error_classify for the native surface.
1 parent 3cbbb19 commit 7464d2c

12 files changed

Lines changed: 247 additions & 1 deletion

File tree

‎nodedb-sql/src/error.rs‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,22 @@ pub enum SqlError {
1919
#[error("function {name}(...) does not exist")]
2020
UndefinedFunction { name: String },
2121

22+
/// A sequence accessor appeared where every output row needs its own
23+
/// allocation, such as a SELECT list over a FROM clause.
24+
///
25+
/// Rendered as SQLSTATE `0A000` (feature_not_supported). Constant
26+
/// contexts evaluate the call for real: a FROM-less `SELECT`, a column
27+
/// `DEFAULT`, a `VALUES` list. The refusal keeps a per-row call from
28+
/// reaching the row evaluator, which has no sequence state and would
29+
/// return `NULL` for every row.
30+
#[error(
31+
"{name}(...) is not supported in a per-row context; \
32+
a SELECT list, WHERE clause, or SET clause over a FROM relation \
33+
evaluates once per row. Call it in a FROM-less SELECT or a column \
34+
DEFAULT instead"
35+
)]
36+
SequencePerRowUnsupported { name: String },
37+
2238
/// A statement names a database object that does not exist — a sequence,
2339
/// most commonly. Distinct from [`SqlError::UndefinedFunction`]: the
2440
/// function exists, the object it names does not. PostgreSQL rejects the

‎nodedb-sql/src/functions/mod.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ pub mod arg_types;
44
pub mod builtins;
55
pub mod fts_ops;
66
pub mod registry;
7+
pub mod sequence_accessor;
78

89
pub use registry::{
910
ArgTypeSpec, FunctionCategory, FunctionMeta, FunctionRegistry, SearchTrigger, Version,
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
3+
//! The sequence-accessor function names, named once for every gate that
4+
//! treats them apart from other scalars.
5+
6+
/// The accessors the planner routes to `SqlCatalog` sequence state.
7+
pub const SEQUENCE_ACCESSORS: [&str; 3] = ["nextval", "currval", "setval"];
8+
9+
/// Whether `name` calls a sequence accessor. Comparison ignores ASCII case.
10+
pub fn is_sequence_accessor(name: &str) -> bool {
11+
SEQUENCE_ACCESSORS
12+
.iter()
13+
.any(|accessor| name.eq_ignore_ascii_case(accessor))
14+
}

‎nodedb-sql/src/planner/catalog_expr_fold.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ pub(super) fn eval_sequence_accessor(
5555
return Ok(None);
5656
};
5757
let lowered = name.to_ascii_lowercase();
58-
if !matches!(lowered.as_str(), "nextval" | "currval" | "setval") {
58+
if !crate::functions::sequence_accessor::is_sequence_accessor(&lowered) {
5959
return Ok(None);
6060
}
6161
let sequence = match args.first() {

‎nodedb-sql/src/resolver/columns.rs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,15 @@ impl TableScope {
123123
}
124124
}
125125

126+
/// Whether an expression here is evaluated once per row of some relation.
127+
///
128+
/// A scope with no relation of its own and no enclosing query stands
129+
/// behind a FROM-less `SELECT`, which produces exactly one row and
130+
/// evaluates its projection at plan time.
131+
pub fn is_row_scope(&self) -> bool {
132+
!self.tables.is_empty() || self.outer.is_some()
133+
}
134+
126135
/// A copy of this scope nested inside `outer`, for planning a correlated
127136
/// subquery body.
128137
pub fn nested_in(mut self, outer: TableScope) -> Self {

‎nodedb-sql/src/resolver/expr/functions.rs‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,16 @@ pub(super) fn convert_function_depth(
8686
return Err(SqlError::UndefinedFunction { name });
8787
}
8888

89+
// Per-row gate: a sequence accessor is evaluated at plan time, by
90+
// `planner::catalog_expr_fold` for a FROM-less SELECT and by
91+
// `planner::defaults` for a column DEFAULT. A call over a FROM relation
92+
// reaches the row evaluator instead, which holds no sequence state and
93+
// answers `NULL` for every row. Refuse it here so the statement fails
94+
// loudly at plan time.
95+
if scope.is_row_scope() && crate::functions::sequence_accessor::is_sequence_accessor(&name) {
96+
return Err(SqlError::SequencePerRowUnsupported { name });
97+
}
98+
8999
let args = collect_function_args(func, depth, scope)?;
90100

91101
let distinct = match &func.args {
@@ -297,4 +307,41 @@ mod tests {
297307
"quoted 'UPPER' must still resolve via case-insensitive registry lookup"
298308
);
299309
}
310+
311+
/// A FROM-less SELECT resolves against an empty relation set, where the
312+
/// planner evaluates the accessor through the catalog.
313+
#[test]
314+
fn a_sequence_accessor_resolves_without_a_relation() {
315+
let func = function_ast("SELECT nextval('s')");
316+
let mut depth = 0;
317+
let scope = crate::resolver::columns::TableScope::new();
318+
let expr = convert_function_depth(&func, &mut depth, &ColumnScope::Relations(&scope))
319+
.expect("a FROM-less sequence accessor must resolve");
320+
match expr {
321+
SqlExpr::Function { name, .. } => assert_eq!(name, "nextval"),
322+
other => panic!("expected SqlExpr::Function, got {other:?}"),
323+
}
324+
}
325+
326+
/// A relation in scope makes the call per-row, which the row evaluator
327+
/// cannot serve, so the resolver refuses it.
328+
#[test]
329+
fn a_sequence_accessor_over_a_relation_is_refused() {
330+
let mut depth = 0;
331+
let outer = crate::resolver::columns::TableScope::new();
332+
let scope = crate::resolver::columns::TableScope::new().nested_in(outer);
333+
for call in [
334+
"SELECT nextval('s')",
335+
"SELECT currval('s')",
336+
"SELECT setval('s', 1)",
337+
] {
338+
let func = function_ast(call);
339+
let err = convert_function_depth(&func, &mut depth, &ColumnScope::Relations(&scope))
340+
.unwrap_err();
341+
assert!(
342+
matches!(err, SqlError::SequencePerRowUnsupported { .. }),
343+
"expected SqlError::SequencePerRowUnsupported for {call}, got {err:?}"
344+
);
345+
}
346+
}
300347
}

‎nodedb-sql/src/resolver/scope.rs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,4 +31,15 @@ impl ColumnScope<'_> {
3131
Self::Relations(scope) => scope.check_name(table_ref, column),
3232
}
3333
}
34+
35+
/// Whether an expression here is evaluated once per row of some relation.
36+
///
37+
/// `Unchecked` stands behind stored DEFAULTs, index predicates, and the
38+
/// constant folders, none of which iterate rows.
39+
pub fn is_row_scope(&self) -> bool {
40+
match self {
41+
Self::Unchecked => false,
42+
Self::Relations(scope) => scope.is_row_scope(),
43+
}
44+
}
3445
}

‎nodedb/src/control/planner/plan_error_map.rs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,13 @@ pub(crate) fn map_plan_error(
3535
nodedb_sql::SqlError::UndefinedFunction { name } => {
3636
crate::Error::UndefinedFunction { name }
3737
}
38+
// A per-row sequence accessor is a refusal, not a syntax error, so it
39+
// keeps SQLSTATE `0A000` rather than the `42601` the fallback gives.
40+
nodedb_sql::SqlError::SequencePerRowUnsupported { .. } => {
41+
crate::Error::FeatureNotSupported {
42+
detail: error.to_string(),
43+
}
44+
}
3845
nodedb_sql::SqlError::UndefinedObject { kind, name } => {
3946
crate::Error::UndefinedObject { kind, name }
4047
}

‎nodedb/src/control/server/pgwire/types/error_map.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ pub fn error_to_sqlstate(err: &crate::Error) -> (&'static str, &'static str, Str
4545
it is hard-deleted"
4646
),
4747
),
48+
crate::Error::FeatureNotSupported { detail } => {
49+
("ERROR", sqlstate::FEATURE_NOT_SUPPORTED, detail.clone())
50+
}
4851
crate::Error::UndefinedFunction { name } => (
4952
"ERROR",
5053
sqlstate::UNDEFINED_FUNCTION,

‎nodedb/src/error/types.rs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,14 @@ pub enum Error {
276276
#[error("query plan error: {detail}")]
277277
PlanError { detail: String },
278278

279+
/// A statement asked for a SQL feature this server does not implement.
280+
/// Propagated from a planner refusal such as
281+
/// `SqlError::SequencePerRowUnsupported`; the pgwire layer renders this
282+
/// as SQLSTATE `0A000` (feature_not_supported), so a client stops rather
283+
/// than retries.
284+
#[error("{detail}")]
285+
FeatureNotSupported { detail: String },
286+
279287
/// A function call in a query names no registered scalar, aggregate, or
280288
/// window function. Propagated from `SqlError::UndefinedFunction`; the
281289
/// pgwire layer renders this as SQLSTATE `42883` (undefined_function).

0 commit comments

Comments
 (0)