Skip to content

Commit 5367e09

Browse files
committed
fix(sql): reject procedural division/modulo by zero (#227)
The procedural executor's constant-expression evaluator — a separate, sqlparser-based tree-walk evaluator used by trigger bodies and EXCEPTION blocks, with no dependency on the DataFusion-backed row-expression evaluator — still folded `/` and `%` by a zero divisor to NULL instead of raising an error. A trigger computing `NEW.ratio := NEW.numerator / NEW.denominator` would silently write NULL when the denominator was zero instead of failing the insert. try_eval_constant, eval_expr, and eval_binary_op now return Result<Option<Value>, ConstEvalError> instead of Option<Value>, and raise a division-by-zero signal only when a `/` or `%` reaches a zero divisor; every other historically-NULL-folding case (unknown identifiers, unsupported operators, overflow, non-finite float results) keeps folding to None as before. The signal converts to a new crate::Error::DivisionByZero variant, which pgwire renders as SQLSTATE 22012 (division_by_zero), so trigger bodies can also match it with EXCEPTION WHEN DIVISION_BY_ZERO. Add the supporting DivisionByZero plumbing at the nodedb-types layer (ErrorCode, ErrorDetails, msgpack wire tag, SQLSTATE constant, NodeDbError constructor) and e2e coverage: a BEFORE INSERT trigger dividing by zero rejects the insert without persisting the row, a non-zero divisor still computes and persists normally, and an EXCEPTION WHEN DIVISION_BY_ZERO handler catches the signal and lets the insert proceed with its fallback assignment.
1 parent a2fdf9e commit 5367e09

13 files changed

Lines changed: 324 additions & 53 deletions

File tree

nodedb-types/src/error/code.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ impl ErrorCode {
4343
pub const PLAN_ERROR: Self = Self(1200);
4444
pub const FAN_OUT_EXCEEDED: Self = Self(1201);
4545
pub const SQL_NOT_ENABLED: Self = Self(1202);
46+
/// Expression evaluation divided or took a modulus by zero.
47+
pub const DIVISION_BY_ZERO: Self = Self(1204);
4648

4749
// Engine ops (1300–1399)
4850
pub const ARRAY: Self = Self(1300);

nodedb-types/src/error/ctors/read_query_auth.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,19 @@ impl NodeDbError {
120120
}
121121
}
122122

123+
/// Expression evaluation divided or took a modulus by zero. Distinct
124+
/// from `plan_error` so clients can match on the specific code
125+
/// (SQLSTATE `22012`, `division_by_zero`) rather than parsing the
126+
/// message (nodedb issue #216).
127+
pub fn division_by_zero() -> Self {
128+
Self {
129+
code: ErrorCode::DIVISION_BY_ZERO,
130+
message: "division by zero".to_string(),
131+
details: ErrorDetails::DivisionByZero,
132+
cause: None,
133+
}
134+
}
135+
123136
pub fn authorization_denied(resource: impl Into<String>) -> Self {
124137
let resource = resource.into();
125138
Self {

nodedb-types/src/error/details.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,9 @@ pub enum ErrorDetails {
8080
FanOutExceeded { shards_touched: u16, limit: u16 },
8181
#[serde(rename = "sql_not_enabled")]
8282
SqlNotEnabled,
83+
/// Expression evaluation divided or took a modulus by zero.
84+
#[serde(rename = "division_by_zero")]
85+
DivisionByZero,
8386

8487
// Auth
8588
#[serde(rename = "authorization_denied")]

nodedb-types/src/error/msgpack/constants.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@
6969
// | 61 | MoveTenantSnapshotFailed |
7070
// | 62 | MoveTenantCutoverFailed |
7171
// | 63 | MoveTenantAlreadyAtTarget |
72+
// | 68 | DivisionByZero |
7273

7374
pub(super) const TAG_CONSTRAINT_VIOLATION: u16 = 1;
7475
pub(super) const TAG_WRITE_CONFLICT: u16 = 2;
@@ -136,3 +137,4 @@ pub(super) const TAG_MOVE_TENANT_ALREADY_AT_TARGET: u16 = 63;
136137
pub(super) const TAG_MIRROR_READ_ONLY: u16 = 64;
137138
pub(super) const TAG_STALE_READ_NOT_LEADER: u16 = 65;
138139
pub(super) const TAG_MIRROR_NOT_PROMOTED: u16 = 66;
140+
pub(super) const TAG_DIVISION_BY_ZERO: u16 = 68;

nodedb-types/src/error/msgpack/decode/from_messagepack.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,10 @@ impl<'a> FromMessagePack<'a> for ErrorDetails {
123123
skip_fields(reader, field_count)?;
124124
Ok(ErrorDetails::SqlNotEnabled)
125125
}
126+
TAG_DIVISION_BY_ZERO => {
127+
skip_fields(reader, field_count)?;
128+
Ok(ErrorDetails::DivisionByZero)
129+
}
126130
TAG_AUTHORIZATION_DENIED => {
127131
let (resource,) = read1_str(reader, field_count)?;
128132
Ok(ErrorDetails::AuthorizationDenied { resource })
@@ -346,6 +350,7 @@ mod tests {
346350
for v in [
347351
ErrorDetails::DeadlineExceeded,
348352
ErrorDetails::SqlNotEnabled,
353+
ErrorDetails::DivisionByZero,
349354
ErrorDetails::AuthExpired,
350355
ErrorDetails::SyncConnectionFailed,
351356
ErrorDetails::Config,

nodedb-types/src/error/msgpack/encode.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,7 @@ impl ToMessagePack for ErrorDetails {
153153
limit,
154154
} => write2(writer, TAG_FAN_OUT_EXCEEDED, shards_touched, limit),
155155
ErrorDetails::SqlNotEnabled => write_unit(writer, TAG_SQL_NOT_ENABLED),
156+
ErrorDetails::DivisionByZero => write_unit(writer, TAG_DIVISION_BY_ZERO),
156157
ErrorDetails::AuthorizationDenied { resource } => {
157158
write1(writer, TAG_AUTHORIZATION_DENIED, resource)
158159
}

nodedb-types/src/error/sqlstate.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,10 @@ pub const CANNOT_DROP_DEFAULT_DATABASE: &str = "0A000";
4040
/// `22003` — `numeric_value_out_of_range`
4141
pub const NUMERIC_VALUE_OUT_OF_RANGE: &str = "22003";
4242

43+
/// `22012` — `division_by_zero` (`/` or `%` with a zero divisor; nodedb
44+
/// issue #216 — raised at runtime instead of evaluating to `NULL`)
45+
pub const DIVISION_BY_ZERO: &str = "22012";
46+
4347
// ── Class 23 — Integrity Constraint Violation ────────────────────────────────
4448

4549
/// `23000` — `integrity_constraint_violation` (generic)
@@ -237,6 +241,7 @@ mod tests {
237241
NO_DATA,
238242
FEATURE_NOT_SUPPORTED,
239243
NUMERIC_VALUE_OUT_OF_RANGE,
244+
DIVISION_BY_ZERO,
240245
INTEGRITY_CONSTRAINT_VIOLATION,
241246
NOT_NULL_VIOLATION,
242247
FOREIGN_KEY_VIOLATION,

nodedb-types/src/error/types.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ impl NodeDbError {
9898
| ErrorDetails::AuthExpired
9999
| ErrorDetails::Config
100100
| ErrorDetails::SqlNotEnabled
101+
| ErrorDetails::DivisionByZero
101102
)
102103
}
103104
}

0 commit comments

Comments
 (0)