Repository: denoland/deno
Pull request: fix(ext/node): implement `DatabaseSync.function()` and better error details

Closes #30097

Implementation is based on https://github.com/nodejs/node/blob/v24.2.0/src/node_sqlite.cc

Changes in this PR allows https://github.com/nodejs/node/blob/v24.2.0/test/parallel/test-sqlite-custom-functions.js test to pass.

Changed files:
filename: ext/node/ops/sqlite/database.rs
patch: @@ -8,6 +8,7 @@ use std::ffi::CString;
 use std::ffi::c_char;
 use std::ffi::c_void;
 use std::path::Path;
+use std::ptr::NonNull;
 use std::ptr::null;
 use std::rc::Rc;
 
@@ -31,11 +32,13 @@ use super::SqliteError;
 use super::StatementSync;
 use super::session::SessionOptions;
 use super::statement::InnerStatementPtr;
+use super::statement::check_error_code;
 use super::statement::check_error_code2;
 use super::validators;
 
 const SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION: i32 = 1005;
 const SQLITE_DBCONFIG_ENABLE_ATTACH_WRITE: i32 = 1021;
+const MAX_SAFE_JS_INTEGER: i64 = 9_007_199_254_740_991;
 
 struct DatabaseSyncOptions {
   open: bool,
@@ -234,6 +237,7 @@ pub struct DatabaseSync {
   statements: Rc<RefCell<Vec<InnerStatementPtr>>>,
   options: DatabaseSyncOptions,
   location: String,
+  ignore_next_sqlite_error: Rc<Cell<bool>>,
 }
 
 // SAFETY: we're sure this can be GCed
@@ -438,6 +442,7 @@ impl DatabaseSync {
       statements: Rc::new(RefCell::new(Vec::new())),
       location,
       options,
+      ignore_next_sqlite_error: Rc::new(Cell::new(false)),
     })
   }
 
@@ -525,7 +530,12 @@ impl DatabaseSync {
     let db = self.conn.borrow();
     let db = db.as_ref().ok_or(SqliteError::InUse)?;
 
-    db.execute_batch(sql)?;
+    if let Err(err) = db.execute_batch(sql) {
+      if self.consume_ignore_next_sqlite_error() {
+        return Ok(());
+      }
+      return Err(err.into());
+    }
 
     Ok(())
   }
@@ -561,10 +571,7 @@ impl DatabaseSync {
         std::ptr::null_mut(),
       )
     };
-
-    if r != libsqlite3_sys::SQLITE_OK {
-      return Err(SqliteError::PrepareFailed);
-    }
+    check_error_code(r, raw_handle)?;
 
     let stmt_cell = Rc::new(Cell::new(Some(raw_stmt)));
     self.statements.borrow_mut().push(stmt_cell.clone());
@@ -573,13 +580,203 @@ impl DatabaseSync {
       inner: stmt_cell,
       db: Rc::downgrade(&self.conn),
       statements: Rc::clone(&self.statements),
+      ignore_next_sqlite_error: Rc::clone(&self.ignore_next_sqlite_error),
       use_big_ints: Cell::new(false),
       allow_bare_named_params: Cell::new(true),
       allow_unknown_named_params: Cell::new(false),
       is_iter_finished: Cell::new(false),
     })
   }
 
+  #[fast]
+  #[validate(is_open)]
+  #[undefined]
+  fn function<'a>(
+    &self,
+    scope: &mut v8::PinScope<'a, '_>,
+    #[varargs] args: Option<&v8::FunctionCallbackArguments>,
+  ) -> Result<(), SqliteError> {
+    let Some(args) = args.filter(|args| args.length() > 0) else {
+      return Err(
+        validators::Error::InvalidArgType(
+          "The \"name\" argument must be a string.",
+        )
+        .into(),
+      );
+    };
+
+    if !args.get(0).is_string() {
+      return Err(
+        validators::Error::InvalidArgType(
+          "The \"name\" argument must be a string.",
+        )
+        .into(),
+      );
+    }
+    let name = args.get(0).to_rust_string_lossy(scope);
+
+    let (options_value, function_value) = if args.length() < 3 {
+      (None, args.get(1))
+    } else {
+      (Some(args.get(1)), args.get(2))
+    };
+
+    let Ok(function) = v8::Local::<v8::Function>::try_from(function_value)
+    else {
+      return Err(
+        validators::Error::InvalidArgType(
+          "The \"function\" argument must be a function.",
+        )
+        .into(),
+      );
+    };
+
+    let mut use_big_int_arguments = false;
+    let mut varargs = false;
+    let mut deterministic = false;
+    let mut direct_only = false;
+
+    if let Some(value) = options_value
+      && !value.is_undefined()
+    {
+      if value.is_null() || !value.is_object() {
+        return Err(
+          validators::Error::InvalidArgType(
+            "The \"options\" argument must be an object.",
+          )
+          .into(),
+        );
+      }
+
+      let options = v8::Local::<v8::Object>::try_from(value).unwrap();
+
+      v8_static_strings! {
+        USE_BIG_INT_ARGUMENTS = "useBigIntArguments",
+        VARARGS = "varargs",
+        DETERMINISTIC = "deterministic",
+        DIRECT_ONLY = "directOnly",
+      }
+
+      let use_bigint_key = USE_BIG_INT_ARGUMENTS.v8_string(scope).unwrap();
+      let bigint_value = options.get(scope, use_bigint_key.into()).unwrap();
+      if !bigint_value.is_undefined() {
+        if !bigint_value.is_boolean() {
+          return Err(
+            validators::Error::InvalidArgType(
+              "The \"options.useBigIntArguments\" argument must be a boolean.",
+            )
+            .into(),
+          );
+        }
+        use_big_int_arguments = bigint_value.boolean_value(scope);
+      }
+
+      let varargs_key = VARARGS.v8_string(scope).unwrap();
+      let varargs_value = options.get(scope, varargs_key.into()).unwrap();
+      if !varargs_value.is_undefined() {
+        if !varargs_value.is_boolean() {
+          return Err(
+            validators::Error::InvalidArgType(
+              "The \"options.varargs\" argument must be a boolean.",
+            )
+            .into(),
+          );
+        }
+        varargs = varargs_value.boolean_value(scope);
+      }
+
+      let deterministic_key = DETERMINISTIC.v8_string(scope).unwrap();
+      let deterministic_value =
+        options.get(scope, deterministic_key.into()).unwrap();
+      if !deterministic_value.is_undefined() {
+        if !deterministic_value.is_boolean() {
+          return Err(
+            validators::Error::InvalidArgType(
+              "The \"options.deterministic\" argument must be a boolean.",
+            )
+            .into(),
+          );
+        }
+        deterministic = deterministic_value.boolean_value(scope);
+      }
+
+      let direct_only_key = DIRECT_ONLY.v8_string(scope).unwrap();
+      let direct_only_value =
+        options.get(scope, direct_only_key.into()).unwrap();
+      if !direct_only_value.is_undefined() {
+        if !direct_only_value.is_boolean() {
+          return Err(
+            validators::Error::InvalidArgType(
+              "The \"options.directOnly\" argument must be a boolean.",
+            )
+            .into(),
+          );
+        }
+        direct_only = direct_only_value.boolean_value(scope);
+      }
+    }
+
+    v8_static_strings! {
+      LENGTH = "length",
+    }
+
+    let argc = if varargs {
+      -1
+    } else {
+      let length_key = LENGTH.v8_string(scope).unwrap();
+      let length = function.get(scope, length_key.into()).unwrap();
+      length.int32_value(scope).unwrap_or(0)
+    };
+
+    let db = self.conn.borrow();
+    let db = db.as_ref().ok_or(SqliteError::InUse)?;
+
+    // SAFETY: lifetime of the connection is guaranteed by reference counting.
+    let raw_handle = unsafe { db.handle() };
+    let name_cstring = CString::new(name)?;
+
+    let callback = v8::Global::new(scope, function).into_raw();
+    let context =
+      v8::Global::new(scope, scope.get_current_context()).into_raw();
+
+    let data = Box::new(CustomFunctionData {
+      callback,
+      context,
+      use_big_int_arguments,
+      ignore_next_sqlite_error: Rc::clone(&self.ignore_next_sqlite_error),
+    });
+    let data_ptr = Box::into_raw(data);
+
+    let mut text_rep = libsqlite3_sys::SQLITE_UTF8;
+    if deterministic {
+      text_rep |= libsqlite3_sys::SQLITE_DETERMINISTIC;
+    }
+    if direct_only {
+      text_rep |= libsqlite3_sys::SQLITE_DIRECTONLY;
+    }
+
+    // SAFETY: `raw_handle` is a valid database handle.
+    // `data_ptr` points to a valid memory location.
+    // The v8 handles that are held in `CustomFunctionData` will be
+    // dropped when the data is destroyed via `custom_function_destroy`.
+    let result = unsafe {
+      libsqlite3_sys::sqlite3_create_function_v2(
+        raw_handle,
+        name_cstring.as_ptr(),
+        argc,
+        text_rep,
+        data_ptr as *mut c_void,
+        Some(custom_function_handler),
+        None,
+        None,
+        Some(custom_function_destroy),
+      )
+    };
+    check_error_code(result, raw_handle)?;
+
+    Ok(())
+  }
+
   // Applies a changeset to the database.
   //
   // This method is a wrapper around `sqlite3changeset_apply()`.
@@ -851,3 +1048,271 @@ impl DatabaseSync {
     })
   }
 }
+
+impl DatabaseSync {
+  fn consume_ignore_next_sqlite_error(&self) -> bool {
+    self.ignore_next_sqlite_error.replace(false)
+  }
+}
+
+struct CustomFunctionData {
+  callback: NonNull<v8::Function>,
+  context: NonNull<v8::Context>,
+  use_big_int_arguments: bool,
+  ignore_next_sqlite_error: Rc<Cell<bool>>,
+}
+
+unsafe extern "C" fn custom_function_handler(
+  ctx: *mut libsqlite3_sys::sqlite3_context,
+  argc: i32,
+  argv: *mut *mut libsqlite3_sys::sqlite3_value,
+) {
+  // SAFETY: `ctx` is a valid sqlite3_context pointer.
+  unsafe {
+    let data_ptr =
+      libsqlite3_sys::sqlite3_user_data(ctx) as *mut CustomFunctionData;
+    if data_ptr.is_null() {
+      sqlite_result_error(
+        ctx,
+        "Internal error: missing custom function context",
+      );
+      return;
+    }
+
+    let data = &*data_ptr;
+    let context_local: v8::Local<v8::Context> =
+      std::mem::transmute(data.context.as_ptr());
+
+    v8::callback_scope!(unsafe cb_scope, context_local);
+    v8::scope!(scope, cb_scope);
+    v8::tc_scope!(tc_scope, scope);
+
+    let function_local: v8::Local<v8::Function> =
+      std::mem::transmute(data.callback.as_ptr());
+
+    let argc_len = usize::try_from(argc).unwrap_or(0);
+    let args_slice = if argc_len == 0 {
+      &[]
+    } else {
+      std::slice::from_raw_parts(argv, argc_len)
+    };
+
+    let mut js_args = Vec::with_capacity(args_slice.len());
+    for &value_ptr in args_slice {
+      if let Some(arg) =
+        sqlite_value_to_v8(tc_scope, value_ptr, data.use_big_int_arguments)
+      {
+        js_args.push(arg);
+      } else {
+        data.ignore_next_sqlite_error.set(true);
+        sqlite_result_error(ctx, "");
+        tc_scope.rethrow();
+        return;
+      }
+    }
+
+    let recv = v8::undefined(tc_scope).into();
+    let result = function_local.call(tc_scope, recv, &js_args);
+    if tc_scope.has_caught() {
+      data.ignore_next_sqlite_error.set(true);
+      sqlite_result_error(ctx, "");
+      tc_scope.rethrow();
+      return;
+    }
+
+    if let Some(value) = result {
+      js_value_to_sqlite(tc_scope, ctx, value);
+      if tc_scope.has_caught() {
+        data.ignore_next_sqlite_error.set(true);
+        sqlite_result_error(ctx, "");
+        tc_scope.rethrow();
+      }
+    }
+  }
+}
+
+unsafe extern "C" fn custom_function_destroy(data: *mut c_void) {
+  // SAFETY: `data` is a valid pointer to CustomFunctionData.
+  // The v8 handles are properly dropped here.
+  unsafe {
+    let data = Box::from_raw(data as *mut CustomFunctionData);
+    let context_local: v8::Local<v8::Context> =
+      std::mem::transmute(data.context.as_ptr());
+
+    v8::callback_scope!(unsafe cb_scope, context_local);
+    v8::scope!(scope, cb_scope);
+
+    let _ = v8::Global::from_raw(scope, data.callback);
+    let _ = v8::Global::from_raw(scope, data.context);
+  }
+}
+
+fn sqlite_value_to_v8<'a>(
+  scope: &mut v8::PinScope<'a, '_>,
+  value: *mut libsqlite3_sys::sqlite3_value,
+  use_big_int_arguments: bool,
+) -> Option<v8::Local<'a, v8::Value>> {
+  // SAFETY: `value` is a valid sqlite3_value pointer.
+  unsafe {
+    match libsqlite3_sys::sqlite3_value_type(value) {
+      libsqlite3_sys::SQLITE_INTEGER => {
+        let val = libsqlite3_sys::sqlite3_value_int64(value);
+        if use_big_int_arguments {
+          Some(v8::BigInt::new_from_i64(scope, val).into())
+        } else if (-MAX_SAFE_JS_INTEGER..=MAX_SAFE_JS_INTEGER).contains(&val) {
+          Some(v8::Number::new(scope, val as f64).into())
+        } else {
+          let msg = format!(
+            "Value is too large to be represented as a JavaScript number: {}",
+            val
+          );
+          throw_range_error(scope, &msg);
+          None
+        }
+      }
+      libsqlite3_sys::SQLITE_FLOAT => {
+        let val = libsqlite3_sys::sqlite3_value_double(value);
+        Some(v8::Number::new(scope, val).into())
+      }
+      libsqlite3_sys::SQLITE_TEXT => {
+        let len = libsqlite3_sys::sqlite3_value_bytes(value) as usize;
+        if len == 0 {
+          let text =
+            v8::String::new_from_utf8(scope, b"", v8::NewStringType::Normal)
+              .unwrap();
+          Some(text.into())
+        } else {
+          let ptr = libsqlite3_sys::sqlite3_value_text(value);
+          let slice = std::slice::from_raw_parts(ptr, len);
+          let text =
+            v8::String::new_from_utf8(scope, slice, v8::NewStringType::Normal)
+              .unwrap();
+          Some(text.into())
+        }
+      }
+      libsqlite3_sys::SQLITE_BLOB => {
+        let len = libsqlite3_sys::sqlite3_value_bytes(value);
+        if len == 0 {
+          let ab = v8::ArrayBuffer::new(scope, 0);
+          let view = v8::Uint8Array::new(scope, ab, 0, 0).unwrap();
+          Some(view.into())
+        } else {
+          let ptr = libsqlite3_sys::sqlite3_value_blob(value) as *const u8;
+          let slice = std::slice::from_raw_parts(ptr, len as usize);
+          let backing =
+            v8::ArrayBuffer::new_backing_store_from_vec(slice.to_vec())
+              .make_shared();
+          let ab = v8::ArrayBuffer::with_backing_store(scope, &backing);
+          let view = v8::Uint8Array::new(scope, ab, 0, len as usize).unwrap();
+          Some(view.into())
+        }
+      }
+      libsqlite3_sys::SQLITE_NULL => Some(v8::null(scope).into()),
+      _ => Some(v8::undefined(scope).into()),
+    }
+  }
+}
+
+fn js_value_to_sqlite(
+  scope: &mut v8::PinScope<'_, '_>,
+  ctx: *mut libsqlite3_sys::sqlite3_context,
+  value: v8::Local<v8::Value>,
+) {
+  // SAFETY: `ctx` is a valid sqlite3_context pointer.
+  unsafe {
+    if value.is_null_or_undefined() {
+      libsqlite3_sys::sqlite3_result_null(ctx);
+      return;
+    }
+
+    if value.is_number() {
+      let number = value.number_value(scope).unwrap_or(0f64);
+      libsqlite3_sys::sqlite3_result_double(ctx, number);
+      return;
+    }
+
+    if value.is_string() {
+      let text = value.to_rust_string_lossy(scope);
+      libsqlite3_sys::sqlite3_result_text(
+        ctx,
+        text.as_ptr() as *const _,
+        text.len() as i32,
+        libsqlite3_sys::SQLITE_TRANSIENT(),
+      );
+      return;
+    }
+
+    if value.is_array_buffer_view() {
+      let view: v8::Local<v8::ArrayBufferView> = value.try_into().unwrap();
+      let mut data = view.data();
+      let mut size = view.byte_length();
+      if data.is_null() {
+        static EMPTY: [u8; 0] = [];
+        data = EMPTY.as_ptr() as *mut _;
+        size = 0;
+      }
+      libsqlite3_sys::sqlite3_result_blob(
+        ctx,
+        data,
+        size as i32,
+        libsqlite3_sys::SQLITE_TRANSIENT(),
+      );
+      return;
+    }
+
+    if value.is_big_int() {
+      let bigint: v8::Local<v8::BigInt> = value.try_into().unwrap();
+      let (int_value, lossless) = bigint.i64_value();
+      if !lossless {
+        throw_range_error(scope, "BigInt value is too large for SQLite");
+        sqlite_result_error(ctx, "");
+        return;
+      }
+      libsqlite3_sys::sqlite3_result_int64(ctx, int_value);
+      return;
+    }
+
+    if value.is_promise() {
+      sqlite_result_error(
+        ctx,
+        "Asynchronous user-defined functions are not supported",
+      );
+      return;
+    }
+
+    sqlite_result_error(
+      ctx,
+      "Returned JavaScript value cannot be converted to a SQLite value",
+    );
+  }
+}
+
+fn sqlite_result_error(
+  ctx: *mut libsqlite3_sys::sqlite3_context,
+  message: &str,
+) {
+  let msg = CString::new(message).unwrap();
+  // SAFETY: `ctx` is a valid sqlite3_context pointer.
+  unsafe {
+    libsqlite3_sys::sqlite3_result_error(
+      ctx,
+      msg.as_ptr(),
+      msg.as_bytes().len() as i32,
+    );
+  }
+}
+
+fn throw_range_error(scope: &mut v8::PinScope<'_, '_>, message: &str) {
+  let msg = v8::String::new(scope, message).unwrap();
+  let error = v8::Exception::range_error(scope, msg);
+
+  v8_static_strings!(CODE = "code", ERR_OUT_OF_RANGE = "ERR_OUT_OF_RANGE");
+  let code_key = CODE.v8_string(scope).unwrap();
+  let code_value = ERR_OUT_OF_RANGE.v8_string(scope).unwrap();
+  let error_obj: v8::Local<v8::Object> = error.try_into().unwrap();
+  error_obj
+    .set(scope, code_key.into(), code_value.into())
+    .unwrap();
+
+  scope.throw_exception(error);
+}

filename: ext/node/ops/sqlite/mod.rs
patch: @@ -78,10 +78,6 @@ pub enum SqliteError {
   #[property("code" = self.code())]
   AlreadyOpen,
   #[class(generic)]
-  #[error("failed to prepare statement")]
-  #[property("code" = self.code())]
-  PrepareFailed,
-  #[class(generic)]
   #[error("failed to create session")]
   #[property("code" = self.code())]
   SessionCreateFailed,

filename: ext/node/ops/sqlite/statement.rs
patch: @@ -69,6 +69,7 @@ pub struct StatementSync {
   pub inner: InnerStatementPtr,
   pub db: Weak<RefCell<Option<rusqlite::Connection>>>,
   pub statements: Rc<RefCell<Vec<InnerStatementPtr>>>,
+  pub ignore_next_sqlite_error: Rc<Cell<bool>>,
 
   pub use_big_ints: Cell<bool>,
   pub allow_bare_named_params: Cell<bool>,
@@ -427,6 +428,10 @@ impl StatementSync {
 
   fn check_error_code(&self, r: i32) -> Result<(), SqliteError> {
     if r != ffi::SQLITE_OK {
+      if self.ignore_next_sqlite_error.get() {
+        self.ignore_next_sqlite_error.set(false);
+        return Ok(());
+      }
       let db_rc = self.db.upgrade().ok_or(SqliteError::InUse)?;
       let db = db_rc.borrow();
       let db = db.as_ref().ok_or(SqliteError::InUse)?;
@@ -583,6 +588,7 @@ impl StatementSync {
   //
   // The prepared statement does not return any results, this method returns undefined.
   // Optionally, parameters can be bound to the prepared statement.
+  #[reentrant]
   fn get<'a>(
     &self,
     scope: &mut v8::PinScope<'a, '_>,

filename: tests/node_compat/config.toml
patch: @@ -973,6 +973,7 @@
 "parallel/test-socket-address.js" = {}
 "parallel/test-socket-write-after-fin-error.js" = {}
 "parallel/test-source-map-enable.js" = {}
+"parallel/test-sqlite-custom-functions.js" = {}
 "parallel/test-sqlite-data-types.js" = {}
 "parallel/test-sqlite-database-sync-dispose.js" = {}
 "parallel/test-sqlite-named-parameters.js" = {}

filename: tests/unit_node/sqlite_test.ts
patch: @@ -1,6 +1,7 @@
 // Copyright 2018-2025 the Deno authors. MIT license.
 import sqlite, { backup, DatabaseSync } from "node:sqlite";
 import { assert, assertEquals, assertThrows } from "@std/assert";
+import * as nodeAssert from "node:assert";
 
 const tempDir = Deno.makeTempDirSync();
 
@@ -480,3 +481,14 @@ Deno.test("[node/sqlite] StatementSync iterate should not reuse previous state",
   db.exec("DELETE FROM numbers");
   assertEquals(Array.from(stmt.iterate()), []);
 });
+
+Deno.test("[node/sqlite] detailed SQLite errors", () => {
+  using db = new DatabaseSync(":memory:");
+
+  nodeAssert.throws(() => db.prepare("SELECT * FROM noop"), {
+    message: "no such table: noop",
+    code: "ERR_SQLITE_ERROR",
+    errcode: 1,
+    errstr: "SQL logic error",
+  });
+});