Repository: denoland/deno
Pull request: fix(ext/node): implement `sqlite`'s missing options

Closes #31634

Changed files:
filename: ext/node/ops/sqlite/database.rs
patch: @@ -48,6 +48,10 @@ struct DatabaseSyncOptions {
   read_only: bool,
   allow_extension: bool,
   enable_double_quoted_string_literals: bool,
+  use_big_int_arguments: bool,
+  allow_bare_named_params: bool,
+  return_arrays: bool,
+  allow_unknown_named_params: bool,
   timeout: u64,
 }
 
@@ -79,6 +83,10 @@ impl<'a> FromV8<'a> for DatabaseSyncOptions {
       ALLOW_EXTENSION_STRING = "allowExtension",
       ENABLE_DOUBLE_QUOTED_STRING_LITERALS_STRING = "enableDoubleQuotedStringLiterals",
       TIMEOUT_STRING = "timeout",
+      READ_BIG_INTS = "readBigInts",
+      RETURN_ARRAYS = "returnArrays",
+      ALLOW_BARE_NAMED_PARAMS = "allowBareNamedParameters",
+      ALLOW_UNKNOWN_NAMED_PARAMS = "allowUnknownNamedParameters",
     }
 
     let open_string = OPEN_STRING.v8_string(scope).unwrap();
@@ -175,6 +183,65 @@ impl<'a> FromV8<'a> for DatabaseSyncOptions {
       }
     }
 
+    let read_big_ints_string = READ_BIG_INTS.v8_string(scope).unwrap();
+    if let Some(read_big_ints) = obj.get(scope, read_big_ints_string.into())
+      && !read_big_ints.is_undefined()
+    {
+      options.use_big_int_arguments =
+        v8::Local::<v8::Boolean>::try_from(read_big_ints)
+          .map_err(|_| {
+            Error::InvalidArgType(
+              "The \"options.readBigInts\" argument must be a boolean.",
+            )
+          })?
+          .is_true();
+    }
+
+    let return_arrays_string = RETURN_ARRAYS.v8_string(scope).unwrap();
+    if let Some(return_arrays) = obj.get(scope, return_arrays_string.into())
+      && !return_arrays.is_undefined()
+    {
+      options.return_arrays = v8::Local::<v8::Boolean>::try_from(return_arrays)
+        .map_err(|_| {
+          Error::InvalidArgType(
+            "The \"options.returnArrays\" argument must be a boolean.",
+          )
+        })?
+        .is_true();
+    }
+
+    let allow_bare_named_params_string =
+      ALLOW_BARE_NAMED_PARAMS.v8_string(scope).unwrap();
+    if let Some(allow_bare_named_params) =
+      obj.get(scope, allow_bare_named_params_string.into())
+      && !allow_bare_named_params.is_undefined()
+    {
+      options.allow_bare_named_params =
+        v8::Local::<v8::Boolean>::try_from(allow_bare_named_params)
+          .map_err(|_| {
+            Error::InvalidArgType(
+              "The \"options.allowBareNamedParameters\" argument must be a boolean.",
+            )
+          })?
+          .is_true();
+    }
+
+    let allow_unknown_named_params_string =
+      ALLOW_UNKNOWN_NAMED_PARAMS.v8_string(scope).unwrap();
+    if let Some(allow_unknown_named_params) =
+      obj.get(scope, allow_unknown_named_params_string.into())
+      && !allow_unknown_named_params.is_undefined()
+    {
+      options.allow_unknown_named_params =
+        v8::Local::<v8::Boolean>::try_from(allow_unknown_named_params)
+          .map_err(|_| {
+            Error::InvalidArgType(
+              "The \"options.allowUnknownNamedParameters\" argument must be a boolean.",
+            )
+          })?
+          .is_true();
+    }
+
     Ok(options)
   }
 }
@@ -187,6 +254,10 @@ impl Default for DatabaseSyncOptions {
       read_only: false,
       allow_extension: false,
       enable_double_quoted_string_literals: false,
+      use_big_int_arguments: false,
+      return_arrays: false,
+      allow_bare_named_params: true,
+      allow_unknown_named_params: false,
       timeout: 0,
     }
   }
@@ -748,9 +819,12 @@ impl DatabaseSync {
       db: self.conn.clone(),
       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),
+      return_arrays: Cell::new(self.options.return_arrays),
+      use_big_ints: Cell::new(self.options.use_big_int_arguments),
+      allow_bare_named_params: Cell::new(self.options.allow_bare_named_params),
+      allow_unknown_named_params: Cell::new(
+        self.options.allow_unknown_named_params,
+      ),
       is_iter_finished: Cell::new(false),
     })
   }

filename: ext/node/ops/sqlite/statement.rs
patch: @@ -70,6 +70,7 @@ pub struct StatementSync {
   pub statements: Rc<RefCell<Vec<InnerStatementPtr>>>,
   pub ignore_next_sqlite_error: Rc<Cell<bool>>,
 
+  pub return_arrays: Cell<bool>,
   pub use_big_ints: Cell<bool>,
   pub allow_bare_named_params: Cell<bool>,
   pub allow_unknown_named_params: Cell<bool>,
@@ -315,7 +316,7 @@ impl StatementSync {
   fn read_row<'a>(
     &self,
     scope: &mut v8::PinScope<'a, '_>,
-  ) -> Result<Option<v8::Local<'a, v8::Object>>, SqliteError> {
+  ) -> Result<Option<v8::Local<'a, v8::Value>>, SqliteError> {
     if self.step()? {
       return Ok(None);
     }
@@ -339,11 +340,15 @@ impl StatementSync {
       values.push(value);
     }
 
-    let null = v8::null(scope).into();
-    let result =
-      v8::Object::with_prototype_and_properties(scope, null, &names, &values);
-
-    Ok(Some(result))
+    if self.return_arrays.get() {
+      let result = v8::Array::new_with_elements(scope, &values);
+      Ok(Some(result.into()))
+    } else {
+      let null = v8::null(scope).into();
+      let result =
+        v8::Object::with_prototype_and_properties(scope, null, &names, &values);
+      Ok(Some(result.into()))
+    }
   }
 
   fn bind_value(
@@ -612,9 +617,7 @@ impl StatementSync {
     let _reset = ResetGuard(self);
 
     let entry = self.read_row(scope)?;
-    let result = entry
-      .map(|r| r.into())
-      .unwrap_or_else(|| v8::undefined(scope).into());
+    let result = entry.unwrap_or_else(|| v8::undefined(scope).into());
 
     Ok(result)
   }
@@ -662,7 +665,7 @@ impl StatementSync {
 
     let _reset = ResetGuard(self);
     while let Some(result) = self.read_row(scope)? {
-      arr.push(result.into());
+      arr.push(result);
     }
 
     let arr = v8::Array::new_with_elements(scope, &arr);
@@ -736,7 +739,7 @@ impl StatementSync {
         return;
       };
 
-      let values = &[v8::Boolean::new(scope, false).into(), row.into()];
+      let values = &[v8::Boolean::new(scope, false).into(), row];
       let null = v8::null(scope).into();
       let result =
         v8::Object::with_prototype_and_properties(scope, null, names, values);
@@ -853,6 +856,17 @@ impl StatementSync {
     Ok(())
   }
 
+  #[fast]
+  #[undefined]
+  fn set_return_arrays(
+    &self,
+    #[validate(validators::return_arrays_bool)] enabled: bool,
+  ) -> Result<(), SqliteError> {
+    self.assert_statement_finalized()?;
+    self.return_arrays.set(enabled);
+    Ok(())
+  }
+
   #[getter]
   #[rename("sourceSQL")]
   #[string]

filename: ext/node/ops/sqlite/validators.rs
patch: @@ -133,3 +133,16 @@ pub(super) fn read_big_ints_bool(
     "The \"readBigInts\" argument must be a boolean.",
   ))
 }
+
+pub(super) fn return_arrays_bool(
+  _: &mut v8::PinScope<'_, '_>,
+  value: v8::Local<v8::Value>,
+) -> Result<(), Error> {
+  if value.is_boolean() {
+    return Ok(());
+  }
+
+  Err(Error::InvalidArgType(
+    "The \"returnArrays\" argument must be a boolean.",
+  ))
+}

filename: tests/node_compat/config.jsonc
patch: @@ -1094,12 +1094,7 @@
     "parallel/test-sqlite-custom-functions.js": {},
     "parallel/test-sqlite-data-types.js": {},
     "parallel/test-sqlite-database-sync-dispose.js": {},
-    "parallel/test-sqlite-database-sync.js": {
-      "darwin": false,
-      "linux": false,
-      "windows": false,
-      "reason": "https://github.com/denoland/deno/issues/31634"
-    },
+    "parallel/test-sqlite-database-sync.js": {},
     "parallel/test-sqlite-named-parameters.js": {},
     "parallel/test-sqlite-session.js": {
       "flaky": true