Skip to content

Commit 62dacd9

Browse files
committed
fix(sql): evaluate sequence-backed DEFAULT expressions on typed engines
`DEFAULT nextval('name')` passed DDL and then silently produced NULL on every insert: the pure default evaluator does not recognize sequence accessors, so the expression evaluated to "no value" and the column was omitted — including primary keys (#294). The sequence machinery itself exists on the control plane (SequenceRegistry with catalog persistence and range allocation); this wires the SQL DEFAULT path to it. - SequenceRegistry is threaded from SharedState through QueryContext into ConvertContext (for_state / for_state_with_lease and both planning sites); sub-planner contexts keep None. - evaluate_sequence_default recognizes DEFAULT nextval('name'), advances the CP-side registry, and raises a loud PlanError for unknown sequences — a DDL-accepted DEFAULT never silently vanishes into NULL again. - The INSERT/UPSERT expanded_rows loop prefers the sequence-aware evaluator over the pure stateless one. Verified: document_strict DEFAULT nextval fills distinct ids (1,2,3) across inserts; unknown sequence raises naming the sequence; uuid defaults unchanged. Out of scope here (tracked): SELECT nextval/currval evaluation, kv/columnar-family defaults, schemaless document column defaults (dropped at catalog adapter — pre-existing), currval/setval session semantics.
1 parent bb76352 commit 62dacd9

15 files changed

Lines changed: 211 additions & 7 deletions

File tree

‎nodedb/src/control/planner/context/query/context.rs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,11 @@ pub struct QueryContext {
4040
/// `QueryContext::new()` test fixtures that never lower to
4141
/// surrogate-bearing variants.
4242
pub(super) surrogate_assigner: Option<Arc<crate::control::surrogate::SurrogateAssigner>>,
43+
/// Sequence registry — `Some` when the planner has access to
44+
/// `SharedState` (production path). SQL sequence accessors
45+
/// (`nextval`/`currval`/`setval`) and sequence-backed DEFAULT
46+
/// expressions evaluate through this; `None` for sub-planners.
47+
pub(super) sequence_registry: Option<Arc<crate::control::sequence::SequenceRegistry>>,
4348
/// Cluster mode flag — `true` when the node has a live cluster
4449
/// topology. Passed into `ConvertContext` so array converters can
4550
/// emit `ClusterArray` variants instead of local `Array` variants.
@@ -109,6 +114,7 @@ impl QueryContext {
109114
array_catalog: None,
110115
wal: None,
111116
surrogate_assigner: None,
117+
sequence_registry: None,
112118
cluster_enabled: false,
113119
bitemporal_retention_registry: None,
114120
max_vector_dim: std::sync::atomic::AtomicU32::new(0),
@@ -140,6 +146,7 @@ impl QueryContext {
140146
Some(Arc::clone(&state.retention_policy_registry)),
141147
);
142148
ctx.surrogate_assigner = Some(Arc::clone(&state.surrogate_assigner));
149+
ctx.sequence_registry = Some(Arc::clone(&state.sequence_registry));
143150
ctx.cluster_enabled = state.cluster_topology.is_some();
144151
ctx.bitemporal_retention_registry = Some(Arc::clone(&state.bitemporal_retention_registry));
145152
// max_vector_dim starts at 0 (unlimited); connection handlers call
@@ -174,6 +181,7 @@ impl QueryContext {
174181
array_catalog: Some(state.array_catalog.clone()),
175182
wal: Some(Arc::clone(&state.wal)),
176183
surrogate_assigner: Some(Arc::clone(&state.surrogate_assigner)),
184+
sequence_registry: Some(Arc::clone(&state.sequence_registry)),
177185
cluster_enabled: state.cluster_topology.is_some(),
178186
bitemporal_retention_registry: Some(Arc::clone(&state.bitemporal_retention_registry)),
179187
// max_vector_dim is tenant-specific; callers supply it via
@@ -215,6 +223,7 @@ impl QueryContext {
215223
array_catalog: None,
216224
wal: None,
217225
surrogate_assigner: None,
226+
sequence_registry: None,
218227
cluster_enabled: false,
219228
bitemporal_retention_registry: None,
220229
max_vector_dim: std::sync::atomic::AtomicU32::new(0),

‎nodedb/src/control/planner/context/query/planning.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ impl QueryContext {
147147
.map(|i| Arc::clone(&i.credentials)),
148148
wal: self.wal.clone(),
149149
surrogate_assigner: self.surrogate_assigner.clone(),
150+
sequence_registry: self.sequence_registry.clone(),
150151
cluster_enabled: self.cluster_enabled,
151152
bitemporal_retention_registry: self.bitemporal_retention_registry.clone(),
152153
max_vector_dim: self
@@ -412,6 +413,7 @@ impl QueryContext {
412413
.map(|i| Arc::clone(&i.credentials)),
413414
wal: self.wal.clone(),
414415
surrogate_assigner: self.surrogate_assigner.clone(),
416+
sequence_registry: self.sequence_registry.clone(),
415417
cluster_enabled: self.cluster_enabled,
416418
bitemporal_retention_registry: self.bitemporal_retention_registry.clone(),
417419
max_vector_dim: self

‎nodedb/src/control/planner/sql_plan_convert/array_fn_convert/aggregate.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ mod tests {
144144
credentials: None,
145145
wal: None,
146146
surrogate_assigner: None,
147+
sequence_registry: None,
147148
cluster_enabled,
148149
bitemporal_retention_registry: None,
149150
max_vector_dim: 0,

‎nodedb/src/control/planner/sql_plan_convert/array_fn_convert/slice.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,7 @@ mod tests {
246246
credentials: None,
247247
wal: None,
248248
surrogate_assigner: None,
249+
sequence_registry: None,
249250
cluster_enabled,
250251
bitemporal_retention_registry: None,
251252
max_vector_dim: 0,

‎nodedb/src/control/planner/sql_plan_convert/convert.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,9 @@ pub struct ConvertContext {
6262
pub credentials: Option<Arc<CredentialStore>>,
6363
/// LSN allocator for array Put/Delete dispatches.
6464
pub wal: Option<Arc<WalManager>>,
65+
/// Sequence registry for SQL sequence accessors and sequence-backed
66+
/// DEFAULT expressions. `None` for sub-planner contexts.
67+
pub sequence_registry: Option<Arc<crate::control::sequence::SequenceRegistry>>,
6568
/// CP-side surrogate assigner — bound to the same `Arc` held on
6669
/// `SharedState`. Threaded into INSERT/UPSERT/KV-INSERT converters
6770
/// to bind `(collection, pk_bytes)` → `Surrogate` before the op
@@ -281,6 +284,7 @@ mod tests {
281284
credentials: None,
282285
wal: None,
283286
surrogate_assigner: Some(assigner),
287+
sequence_registry: None,
284288
cluster_enabled: false,
285289
bitemporal_retention_registry: None,
286290
max_vector_dim: 0,

‎nodedb/src/control/planner/sql_plan_convert/dml/insert.rs‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -233,13 +233,22 @@ pub(in super::super) fn convert_insert(
233233
}
234234
let mut expanded = row.clone();
235235
for (col_name, default_expr) in column_defaults {
236-
if !expanded.iter().any(|(k, _)| k == col_name)
237-
&& let Some(val) = super::super::value::evaluate_default_expr(default_expr)
238-
.map_err(|e| crate::Error::PlanError {
239-
detail: format!("default for column '{col_name}': {e}"),
240-
})?
241-
{
242-
expanded.push((col_name.clone(), nodedb_value_to_sql(val)));
236+
if !expanded.iter().any(|(k, _)| k == col_name) {
237+
// Sequence-backed defaults advance CP-side registry state;
238+
// the pure evaluator would fold them to nothing (NULL).
239+
let maybe_val: Option<nodedb_types::Value> =
240+
match super::super::value::evaluate_sequence_default(ctx, default_expr) {
241+
Some(Ok(value)) => Some(value),
242+
Some(Err(e)) => return Err(e),
243+
None => super::super::value::evaluate_default_expr(default_expr).map_err(
244+
|e| crate::Error::PlanError {
245+
detail: format!("default for column '{col_name}': {e}"),
246+
},
247+
)?,
248+
};
249+
if let Some(val) = maybe_val {
250+
expanded.push((col_name.clone(), nodedb_value_to_sql(val)));
251+
}
243252
}
244253
}
245254
expanded_rows.push(expanded);
@@ -433,6 +442,7 @@ mod tests {
433442
credentials: Some(Arc::new(store)),
434443
wal: None,
435444
surrogate_assigner: None,
445+
sequence_registry: None,
436446
cluster_enabled: false,
437447
bitemporal_retention_registry: None,
438448
max_vector_dim: 0,

‎nodedb/src/control/planner/sql_plan_convert/dml/kv_and_vector.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,6 +204,7 @@ mod tests {
204204
credentials: None,
205205
wal: None,
206206
surrogate_assigner: None,
207+
sequence_registry: None,
207208
cluster_enabled: false,
208209
bitemporal_retention_registry: None,
209210
max_vector_dim,

‎nodedb/src/control/planner/sql_plan_convert/dml/update_delete/delete.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,7 @@ mod tests {
250250
credentials: Some(Arc::new(store)),
251251
wal: None,
252252
surrogate_assigner: None,
253+
sequence_registry: None,
253254
cluster_enabled: false,
254255
bitemporal_retention_registry: None,
255256
max_vector_dim: 0,

‎nodedb/src/control/planner/sql_plan_convert/dml/update_delete/update.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -359,6 +359,7 @@ mod tests {
359359
credentials: Some(Arc::new(store)),
360360
wal: None,
361361
surrogate_assigner: None,
362+
sequence_registry: None,
362363
cluster_enabled: false,
363364
bitemporal_retention_registry: None,
364365
max_vector_dim: 0,

‎nodedb/src/control/planner/sql_plan_convert/set_ops.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -392,6 +392,7 @@ mod tests {
392392
credentials: None,
393393
wal: None,
394394
surrogate_assigner: None,
395+
sequence_registry: None,
395396
cluster_enabled: false,
396397
bitemporal_retention_registry: None,
397398
max_vector_dim: 0,
@@ -450,6 +451,7 @@ mod tests {
450451
credentials: None,
451452
wal: None,
452453
surrogate_assigner: None,
454+
sequence_registry: None,
453455
cluster_enabled: false,
454456
bitemporal_retention_registry: None,
455457
max_vector_dim: 0,

0 commit comments

Comments
 (0)