Fix SQL parser to support negative numbers in INSERT statements (#4660)

This fix is intended to resolve the community reported issue #4659

# Description of Changes
Add handling for `UnaryOperator::Minus` and `UnaryOperator::Plus` in a
new `parse_insert_value()` used during `INSERT` operations. This works
by convert negative unary expressions to signed numeric strings in
INSERT VALUES clauses.

# API and ABI breaking changes
None.

# Expected complexity level and risk
1 (Low). Localized parser fix.

# Testing
- [X] Ran local tests to verify negative numbers work in Rust,
TypeScript, and C# modules
- [X] Confirmed positive numbers and invalid expressions still work
correctly
* Testing involved running `spacetime sql module "INSERT INTO table
(col) VALUES (-100.0);"` from a CLI.

---------

Co-authored-by: joshua-spacetime <josh@clockworklabs.io>

Author: rekhoff

crates/sql-parser/src/parser/mod.rs

@@ -217,12 +217,6 @@ const _: () = assert!(size_of::<SqlParseResult<SqlExpr>>() == 40);
 
 /// Parse a scalar expression
 fn parse_expr(expr: Expr, depth: usize) -> SqlParseResult<SqlExpr> {
-    fn signed_num(sign: impl Into<String>, expr: Expr) -> Result<SqlExpr, Box<SqlUnsupported>> {
-        match expr {
-            Expr::Value(Value::Number(n, _)) => Ok(SqlExpr::Lit(SqlLiteral::Num((sign.into() + &n).into_boxed_str()))),
-            expr => Err(SqlUnsupported::Expr(expr).into()),
-        }
-    }
     recursion::guard(depth, recursion::MAX_RECURSION_EXPR, "sql-parser::parse_expr")?;
     match expr {
         Expr::Nested(expr) => parse_expr(*expr, depth + 1),
@@ -231,15 +225,19 @@ fn parse_expr(expr: Expr, depth: usize) -> SqlParseResult<SqlExpr> {
         Expr::UnaryOp {
             op: UnaryOperator::Plus,
             expr,
-        } if matches!(&*expr, Expr::Value(Value::Number(..))) => {
-            signed_num("+", *expr).map_err(SqlParseError::SqlUnsupported)
-        }
+        } => Ok(SqlExpr::Lit(parse_signed_literal_expr(
+            UnaryOperator::Plus,
+            *expr,
+            SqlUnsupported::Expr,
+        )?)),
         Expr::UnaryOp {
             op: UnaryOperator::Minus,
             expr,
-        } if matches!(&*expr, Expr::Value(Value::Number(..))) => {
-            signed_num("-", *expr).map_err(SqlParseError::SqlUnsupported)
-        }
+        } => Ok(SqlExpr::Lit(parse_signed_literal_expr(
+            UnaryOperator::Minus,
+            *expr,
+            SqlUnsupported::Expr,
+        )?)),
         Expr::Identifier(ident) => Ok(SqlExpr::Var(ident.into())),
         Expr::CompoundIdentifier(mut idents) if idents.len() == 2 => {
             let table = idents.swap_remove(0).into();
@@ -273,6 +271,44 @@ fn parse_expr(expr: Expr, depth: usize) -> SqlParseResult<SqlExpr> {
     }
 }
 
+fn parse_signed_literal_expr(
+    op: UnaryOperator,
+    expr: Expr,
+    unsupported: fn(Expr) -> SqlUnsupported,
+) -> SqlParseResult<SqlLiteral> {
+    match expr {
+        Expr::Value(Value::Number(n, _)) => {
+            let sign = match op {
+                UnaryOperator::Plus => "+",
+                UnaryOperator::Minus => "-",
+                _ => unreachable!("caller only passes unary plus/minus"),
+            };
+            Ok(SqlLiteral::Num(format!("{sign}{n}").into_boxed_str()))
+        }
+        expr => Err(unsupported(Expr::UnaryOp {
+            op,
+            expr: Box::new(expr),
+        })
+        .into()),
+    }
+}
+
+/// Parse a literal expression.
+pub(crate) fn parse_literal_expr(expr: Expr, unsupported: fn(Expr) -> SqlUnsupported) -> SqlParseResult<SqlLiteral> {
+    match expr {
+        Expr::Value(value) => parse_literal(value),
+        Expr::UnaryOp {
+            op: UnaryOperator::Plus,
+            expr,
+        } => parse_signed_literal_expr(UnaryOperator::Plus, *expr, unsupported),
+        Expr::UnaryOp {
+            op: UnaryOperator::Minus,
+            expr,
+        } => parse_signed_literal_expr(UnaryOperator::Minus, *expr, unsupported),
+        expr => Err(unsupported(expr).into()),
+    }
+}
+
 /// Parse an optional scalar expression
 pub(crate) fn parse_expr_opt(opt: Option<Expr>) -> SqlParseResult<Option<SqlExpr>> {
     opt.map(|expr| parse_expr(expr, 0)).transpose()

crates/sql-parser/src/parser/sql.rs

@@ -142,7 +142,7 @@ use crate::ast::{
 };
 
 use super::{
-    errors::SqlUnsupported, parse_expr_opt, parse_ident, parse_literal, parse_parts, parse_projection, RelParser,
+    errors::SqlUnsupported, parse_expr_opt, parse_ident, parse_literal_expr, parse_parts, parse_projection, RelParser,
     SqlParseResult,
 };
 
@@ -242,11 +242,7 @@ fn parse_values(values: Query) -> SqlParseResult<SqlValues> {
                 for row in rows {
                     let mut literals = Vec::new();
                     for expr in row {
-                        if let Expr::Value(value) = expr {
-                            literals.push(parse_literal(value)?);
-                        } else {
-                            return Err(SqlUnsupported::InsertValue(expr).into());
-                        }
+                        literals.push(parse_literal_expr(expr, SqlUnsupported::InsertValue)?);
                     }
                     row_literals.push(literals);
                 }
@@ -274,10 +270,10 @@ fn parse_assignments(assignments: Vec<Assignment>) -> SqlParseResult<Vec<SqlSet>
 
 /// Parse a column/variable assignment in an UPDATE or SET statement
 fn parse_assignment(Assignment { id, value }: Assignment) -> SqlParseResult<SqlSet> {
-    match value {
-        Expr::Value(value) => Ok(SqlSet(parse_parts(id)?, parse_literal(value)?)),
-        _ => Err(SqlUnsupported::Assignment(value).into()),
-    }
+    Ok(SqlSet(
+        parse_parts(id)?,
+        parse_literal_expr(value, SqlUnsupported::Assignment)?,
+    ))
 }
 
 /// Parse a DELETE statement
@@ -311,12 +307,7 @@ fn parse_set_var(variable: ObjectName, mut value: Vec<Expr>) -> SqlParseResult<S
     if value.len() == 1 {
         Ok(SqlSet(
             parse_ident(variable)?,
-            match value.swap_remove(0) {
-                Expr::Value(value) => parse_literal(value)?,
-                expr => {
-                    return Err(SqlUnsupported::Assignment(expr).into());
-                }
-            },
+            parse_literal_expr(value.swap_remove(0), SqlUnsupported::Assignment)?,
         ))
     } else {
         Err(SqlUnsupported::feature(Statement::SetVariable {
@@ -454,6 +445,20 @@ mod tests {
         }
     }
 
+    #[test]
+    fn signed_numeric_literals_are_supported_across_sql_api() {
+        for sql in [
+            "select a from t where b = -1",
+            "delete from t where a = +1",
+            "insert into t values (-1, +2.5)",
+            "update t set a = -1, b = +2 where c = -3",
+            "set x = -1",
+            "set y to +2.5",
+        ] {
+            assert!(parse_sql(sql).is_ok());
+        }
+    }
+
     #[test]
     fn invalid() {
         for sql in [