Skip to content

Commit 5f23c5b

Browse files
committed
classify sequence accessors in expression contexts as 0A000
Register nextval, currval and setval so the planner admits and types them, then make every evaluation that escapes the DEFAULT path fail loudly: the scalar evaluator raises a typed FeatureNotSupported error, const folding classifies it exhaustively, and the error plumbing maps it to SQLSTATE 0A000 end to end. Executor lanes that collapsed any evaluation error to division-by-zero now discriminate variants, so the typed code survives to the client. Adds the expression-context wire coverage, a parser corpus locking the tolerant byte-safe accessor recogniser, and a static gate script that keeps the DEFAULT-plane seam honest.
1 parent 29a2350 commit 5f23c5b

37 files changed

Lines changed: 616 additions & 18 deletions

File tree

‎.github/workflows/static-gates.yml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,3 +55,5 @@ jobs:
5555
run: |
5656
python3 scripts/ci/check_advisory_ignores.py --self-test
5757
python3 scripts/ci/check_advisory_ignores.py
58+
- name: Sequence-plane gate
59+
run: python3 scripts/ci/check_sequence_plane.py

‎nodedb-cluster/src/rpc_codec/data_plane_error.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,4 +112,8 @@ pub enum DataPlaneErrorCode {
112112
limit: u64,
113113
},
114114
DivisionByZero,
115+
/// Registered sequence accessor reached expression evaluation (0A000).
116+
FeatureNotSupported {
117+
name: String,
118+
},
115119
}

‎nodedb-query/src/expr/eval.rs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,16 @@ use super::types::SqlExpr;
2727
pub enum EvalError {
2828
#[error("division by zero")]
2929
DivisionByZero,
30+
/// Sequence accessors (`nextval`/`currval`/`setval`) are stateful and
31+
/// CP-side only. They are evaluated as column DEFAULTs by the plan
32+
/// converter, never by the row-scope scalar evaluator. Reaching this
33+
/// error means an accessor escaped the DEFAULT path (e.g. a bare
34+
/// `SELECT nextval('s')`), which must surface loudly as 0A000 — never as
35+
/// a silent `Null`.
36+
#[error(
37+
"sequence accessors are supported as column DEFAULTs (DEFAULT nextval('s')); SELECT-time evaluation is not yet wired"
38+
)]
39+
FeatureNotSupported { name: &'static str },
3040
}
3141

3242
/// Row scope for `SqlExpr::eval_scope`: how `Column(..)` and `OldColumn(..)`

‎nodedb-query/src/functions/eval.rs‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,19 @@ use super::{array, conditional, datetime, fts, id, json, math, string, system, t
2020
/// fallible arm doesn't force every scalar-function module to carry a
2121
/// `Result` it can never actually produce.
2222
pub fn eval_function(name: &str, args: &[Value]) -> Result<Value, EvalError> {
23+
// Sequence accessors are stateful (CP-side, DEFAULT-scoped). They are
24+
// never scalar-evaluable: if one reaches this dispatcher, it escaped the
25+
// DEFAULT path and must raise 0A000 instead of falling through to the
26+
// geo fallback's silent `Null`.
27+
let canonical = match name.to_ascii_lowercase().as_str() {
28+
"nextval" => Some("nextval"),
29+
"currval" => Some("currval"),
30+
"setval" => Some("setval"),
31+
_ => None,
32+
};
33+
if let Some(cname) = canonical {
34+
return Err(EvalError::FeatureNotSupported { name: cname });
35+
}
2336
if let Some(v) = string::try_eval(name, args) {
2437
return Ok(v);
2538
}
@@ -63,6 +76,29 @@ mod tests {
6376
eval_function(name, &args).unwrap()
6477
}
6578

79+
#[test]
80+
fn sequence_accessors_are_loud_not_null() {
81+
// Regression: accessors used to fall through to the geo fallback and
82+
// return Ok(Null). They must error as FeatureNotSupported (0A000).
83+
for name in ["nextval", "currval", "setval", "NEXTVAL"] {
84+
let err = eval_function(name, &[Value::String("s".into())]).unwrap_err();
85+
assert!(
86+
matches!(err, crate::expr::EvalError::FeatureNotSupported { .. }),
87+
"{name} must raise FeatureNotSupported, got {err:?}"
88+
);
89+
}
90+
}
91+
92+
#[test]
93+
fn non_sequence_unknown_still_nulls() {
94+
// The guard is scoped: unknown non-sequence names keep the legacy
95+
// geo-fallback behaviour (Ok(Null)), not an error.
96+
assert_eq!(
97+
eval_function("definitely_not_a_fn", &[]).unwrap(),
98+
Value::Null
99+
);
100+
}
101+
66102
#[test]
67103
fn mod_by_zero_errors() {
68104
let err = eval_function("mod", &[Value::Integer(5), Value::Integer(0)]).unwrap_err();

‎nodedb-sql/src/error.rs‎

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

22+
/// A registered sequence accessor (`nextval`/`currval`/`setval`) was
23+
/// const-folded or row-evaluated. Accessors are stateful and CP-side
24+
/// only — valid as column DEFAULTs, invalid in any SQL expression
25+
/// context. Maps to SQLSTATE 0A000 (feature not supported).
26+
#[error(
27+
"sequence accessors are supported as column DEFAULTs (DEFAULT nextval('s')); SELECT-time evaluation is not yet wired"
28+
)]
29+
FeatureNotSupported { name: String },
30+
2231
#[error("unknown column '{column}' in table '{table}'")]
2332
UnknownColumn { table: String, column: String },
2433

‎nodedb-sql/src/functions/builtins/scalars.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ mod math;
1212
mod misc;
1313
mod pg_fts;
1414
mod pg_json;
15+
mod sequence;
1516
mod spatial;
1617
mod string;
1718
mod vector;
@@ -22,6 +23,7 @@ pub(super) fn scalar_functions() -> Vec<FunctionMeta> {
2223
let mut fns = Vec::new();
2324
fns.extend(vector::vector_functions());
2425
fns.extend(spatial::spatial_functions());
26+
fns.extend(sequence::sequence_functions());
2527
fns.extend(datetime::datetime_functions());
2628
fns.extend(doc::doc_functions());
2729
fns.extend(string::string_functions());
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
3+
//! Sequence accessor registrations (`nextval`/`currval`/`setval`).
4+
//!
5+
//! These names are stateful and CP-side only: they evaluate as column
6+
//! DEFAULTs via the plan converter, never as SQL-expression scalars. The
7+
//! plan-time existence gate still needs them registered so that a bare
8+
//! expression use is planned (typed, arity-checked) and then fails LOUDLY
9+
//! at fold/row-eval time with 0A000 — instead of being rejected as
10+
//! "function does not exist" (42883) or silently NULLing at runtime.
11+
//!
12+
//! This list must stay in sync with the sequence guard arm in
13+
//! `nodedb_query::functions::eval_function`.
14+
15+
use nodedb_types::columnar::ColumnType;
16+
17+
use crate::functions::arg_types;
18+
use crate::functions::registry::{ArgTypeSpec, FunctionCategory::Scalar, FunctionMeta};
19+
20+
use super::super::helpers::{m, no_trigger};
21+
22+
static SEQ_1_ARGS: &[ArgTypeSpec] = &[arg_types::any("seq")];
23+
static SEQ_2_ARGS: &[ArgTypeSpec] = &[arg_types::any("seq"), arg_types::any("value")];
24+
25+
pub(super) fn sequence_functions() -> Vec<FunctionMeta> {
26+
vec![
27+
m(
28+
"nextval",
29+
Scalar,
30+
1,
31+
1,
32+
no_trigger(),
33+
Some(ColumnType::Int64),
34+
SEQ_1_ARGS,
35+
),
36+
m(
37+
"currval",
38+
Scalar,
39+
1,
40+
1,
41+
no_trigger(),
42+
Some(ColumnType::Int64),
43+
SEQ_1_ARGS,
44+
),
45+
m(
46+
"setval",
47+
Scalar,
48+
2,
49+
2,
50+
no_trigger(),
51+
Some(ColumnType::Int64),
52+
SEQ_2_ARGS,
53+
),
54+
]
55+
}
56+
57+
#[cfg(test)]
58+
mod tests {
59+
use crate::functions::registry::FunctionRegistry;
60+
61+
#[test]
62+
fn accessors_registered_for_plan_gate() {
63+
let reg = FunctionRegistry::new();
64+
for name in ["nextval", "currval", "setval"] {
65+
assert!(reg.lookup(name).is_some(), "{name} must be registered");
66+
}
67+
}
68+
69+
#[test]
70+
fn accessors_error_loudly_in_expression_eval() {
71+
// A1+A3 contract: registered, arity-checked, then loud 0A000 at
72+
// fold/row-eval — never a silent Null (parity invariant seed).
73+
for name in ["nextval", "currval"] {
74+
let err = nodedb_query::functions::eval_function(
75+
name,
76+
&[nodedb_types::Value::String("s".into())],
77+
)
78+
.unwrap_err();
79+
assert!(
80+
matches!(err, nodedb_query::EvalError::FeatureNotSupported { .. }),
81+
"{name} must raise FeatureNotSupported, got {err:?}"
82+
);
83+
}
84+
}
85+
}

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,11 @@ pub fn fold_function_call(name: &str, args: &[SqlExpr], registry: &FunctionRegis
342342
match nodedb_query::functions::eval_function(&name.to_lowercase(), &folded_args) {
343343
Ok(result) => Ok(Some(ndb_to_sql_value(result))),
344344
Err(nodedb_query::EvalError::DivisionByZero) => Err(SqlError::DivisionByZero),
345+
Err(nodedb_query::EvalError::FeatureNotSupported { name }) => {
346+
Err(SqlError::FeatureNotSupported {
347+
name: name.to_string(),
348+
})
349+
}
345350
}
346351
}
347352

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

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,3 +178,93 @@ pub fn sequence_accessor(expr: &str) -> Option<(SequenceAccessor, String)> {
178178
}
179179
Some((accessor, name.to_string()))
180180
}
181+
182+
#[cfg(test)]
183+
mod sequence_accessor_corpus {
184+
use super::{SequenceAccessor, looks_like_sequence_accessor, sequence_accessor};
185+
186+
fn name(expr: &str) -> Option<String> {
187+
sequence_accessor(expr).map(|(_, n)| n)
188+
}
189+
190+
fn acc(expr: &str) -> Option<SequenceAccessor> {
191+
sequence_accessor(expr).map(|(a, _)| a)
192+
}
193+
194+
#[test]
195+
fn canonical_forms() {
196+
assert_eq!(name("nextval('sq')").as_deref(), Some("sq"));
197+
assert_eq!(acc("nextval('sq')"), Some(SequenceAccessor::Nextval));
198+
assert_eq!(acc("currval('sq')"), Some(SequenceAccessor::Currval));
199+
assert_eq!(acc("setval('sq')"), Some(SequenceAccessor::Setval));
200+
}
201+
202+
#[test]
203+
fn case_and_quote_variants() {
204+
// ASCII case-insensitive on the accessor; name bytes preserved.
205+
assert_eq!(name("NEXTVAL('MySeq')").as_deref(), Some("MySeq"));
206+
assert_eq!(name("Currval('c')").as_deref(), Some("c"));
207+
// Double-quoted names are recognized too.
208+
assert_eq!(name("nextval(\"dq\")").as_deref(), Some("dq"));
209+
// Whitespace inside the parens is tolerated.
210+
assert_eq!(name("nextval( 'padded' )").as_deref(), Some("padded"));
211+
}
212+
213+
#[test]
214+
fn tolerant_raw_name_handling() {
215+
// Embedded quotes are preserved raw (byte-safe) — never sliced
216+
// against a case-folded copy, never panicked.
217+
assert_eq!(name("nextval('a''b')").as_deref(), Some("a''b"));
218+
// Unicode names survive untouched.
219+
assert_eq!(
220+
name("nextval('sekuensi\u{1F600}')").as_deref(),
221+
Some("sekuensi\u{1F600}")
222+
);
223+
// Bare identifier (unquoted) is tolerated by the recognizer; the
224+
// registry/convert layers decide loudness afterwards.
225+
assert_eq!(name("nextval(sq)").as_deref(), Some("sq"));
226+
}
227+
228+
#[test]
229+
fn non_accessor_shapes_are_rejected() {
230+
assert_eq!(name("nextval('')"), None, "empty name is malformed");
231+
// Tolerant by design: extra args ride along in the raw name and the
232+
// convert layer raises on registry lookup — still loud, never
233+
// silent. (Byte-safe: no slice against a case-folded copy.)
234+
assert_eq!(
235+
acc("nextval('s', 'x')"),
236+
Some(SequenceAccessor::Nextval),
237+
"two-arg form is tolerated and resolved loud later"
238+
);
239+
assert_eq!(
240+
name("nextval('s')::text"),
241+
None,
242+
"cast wrapper is not raw accessor"
243+
);
244+
assert_eq!(acc("lastval('s')"), None);
245+
assert_eq!(acc("nextvalx('s')"), None);
246+
assert_eq!(acc("xnextval('s')"), None);
247+
assert_eq!(
248+
acc("nextval ('s')"),
249+
None,
250+
"space before paren is not a call"
251+
);
252+
assert_eq!(acc("nextval"), None);
253+
assert_eq!(acc(""), None);
254+
assert_eq!(
255+
acc("'nextval('s')'"),
256+
None,
257+
"quoted string literal is not a call"
258+
);
259+
}
260+
261+
#[test]
262+
fn looks_like_matches_only_call_prefix() {
263+
assert!(looks_like_sequence_accessor("nextval('x')"));
264+
assert!(looks_like_sequence_accessor("SETVAL( 'x' )"));
265+
assert!(!looks_like_sequence_accessor("nextvalx('x')"));
266+
assert!(!looks_like_sequence_accessor("xnextval('x')"));
267+
assert!(!looks_like_sequence_accessor("nextval ('x')"));
268+
assert!(!looks_like_sequence_accessor("nextval"));
269+
}
270+
}

‎nodedb-types/src/error/code.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,9 @@ impl ErrorCode {
5757
pub const UNDEFINED_FUNCTION: Self = Self(1203);
5858
/// Expression evaluation divided or took a modulus by zero.
5959
pub const DIVISION_BY_ZERO: Self = Self(1204);
60+
/// A registered sequence accessor was evaluated in a SQL expression
61+
/// context (stateful accessors are DEFAULT-scoped only). SQLSTATE 0A000.
62+
pub const FEATURE_NOT_SUPPORTED: Self = Self(1208);
6063
/// A LIMIT/OFFSET/FETCH bound resolved outside `[0, usize::MAX]`.
6164
pub const INVALID_LIMIT_VALUE: Self = Self(1205);
6265
/// A column reference names no column of any relation in scope.

0 commit comments

Comments
 (0)