sqlite: pass conflict type to conflict resolution handler

louwers · aduh95 · commit 3462263e8b6e · 2025-01-31T01:16:46.000+01:00
PR-URL: #56352 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
diff --git a/doc/api/sqlite.md b/doc/api/sqlite.md
@@ -230,10 +230,27 @@ added: v22.12.0
 * `options` {Object} The configuration options for how the changes will be applied.
   * `filter` {Function} Skip changes that, when targeted table name is supplied to this function, return a truthy value.
     By default, all changes are attempted.
-  * `onConflict` {number} Determines how conflicts are handled. **Default**: `SQLITE_CHANGESET_ABORT`.
-    * `SQLITE_CHANGESET_OMIT`: conflicting changes are omitted.
-    * `SQLITE_CHANGESET_REPLACE`: conflicting changes replace existing values.
-    * `SQLITE_CHANGESET_ABORT`: abort on conflict and roll back database.
+  * `onConflict` {Function} A function that determines how to handle conflicts. The function receives one argument,
+    which can be one of the following values:
+
+    * `SQLITE_CHANGESET_DATA`: A `DELETE` or `UPDATE` change does not contain the expected "before" values.
+    * `SQLITE_CHANGESET_NOTFOUND`: A row matching the primary key of the `DELETE` or `UPDATE` change does not exist.
+    * `SQLITE_CHANGESET_CONFLICT`: An `INSERT` change results in a duplicate primary key.
+    * `SQLITE_CHANGESET_FOREIGN_KEY`: Applying a change would result in a foreign key violation.
+    * `SQLITE_CHANGESET_CONSTRAINT`: Applying a change results in a `UNIQUE`, `CHECK`, or `NOT NULL` constraint
+      violation.
+
+    The function should return one of the following values:
+
+    * `SQLITE_CHANGESET_OMIT`: Omit conflicting changes.
+    * `SQLITE_CHANGESET_REPLACE`: Replace existing values with conflicting changes (only valid with
+      `SQLITE_CHANGESET_DATA` or `SQLITE_CHANGESET_CONFLICT` conflicts).
+    * `SQLITE_CHANGESET_ABORT`: Abort on conflict and roll back the database.
+
+    When an error is thrown in the conflict handler or when any other value is returned from the handler,
+    applying the changeset is aborted and the database is rolled back.
+
+    **Default**: A function that returns `SQLITE_CHANGESET_ABORT`.
 * Returns: {boolean} Whether the changeset was applied succesfully without being aborted.
 
 An exception is thrown if the database is not
@@ -486,9 +503,42 @@ An object containing commonly used constants for SQLite operations.
 
 The following constants are exported by the `sqlite.constants` object.
 
-#### Conflict-resolution constants
+#### Conflict resolution constants
+
+One of the following constants is available as an argument to the `onConflict`
+conflict resolution handler passed to [`database.applyChangeset()`][]. See also
+[Constants Passed To The Conflict Handler][] in the SQLite documentation.
+
+<table>
+  <tr>
+    <th>Constant</th>
+    <th>Description</th>
+  </tr>
+  <tr>
+    <td><code>SQLITE_CHANGESET_DATA</code></td>
+    <td>The conflict handler is invoked with this constant when processing a DELETE or UPDATE change if a row with the required PRIMARY KEY fields is present in the database, but one or more other (non primary-key) fields modified by the update do not contain the expected "before" values.</td>
+  </tr>
+  <tr>
+    <td><code>SQLITE_CHANGESET_NOTFOUND</code></td>
+    <td>The conflict handler is invoked with this constant when processing a DELETE or UPDATE change if a row with the required PRIMARY KEY fields is not present in the database.</td>
+  </tr>
+  <tr>
+    <td><code>SQLITE_CHANGESET_CONFLICT</code></td>
+    <td>This constant is passed to the conflict handler while processing an INSERT change if the operation would result in duplicate primary key values.</td>
+  </tr>
+  <tr>
+    <td><code>SQLITE_CHANGESET_CONSTRAINT</code></td>
+    <td>If foreign key handling is enabled, and applying a changeset leaves the database in a state containing foreign key violations, the conflict handler is invoked with this constant exactly once before the changeset is committed. If the conflict handler returns <code>SQLITE_CHANGESET_OMIT</code>, the changes, including those that caused the foreign key constraint violation, are committed. Or, if it returns <code>SQLITE_CHANGESET_ABORT</code>, the changeset is rolled back.</td>
+  </tr>
+  <tr>
+    <td><code>SQLITE_CHANGESET_FOREIGN_KEY</code></td>
+    <td>If any other constraint violation occurs while applying a change (i.e. a UNIQUE, CHECK or NOT NULL constraint), the conflict handler is invoked with this constant.</td>
+  </tr>
+</table>
 
-The following constants are meant for use with [`database.applyChangeset()`](#databaseapplychangesetchangeset-options).
+One of the following constants must be returned from the `onConflict` conflict
+resolution handler passed to [`database.applyChangeset()`][]. See also
+[Constants Returned From The Conflict Handler][] in the SQLite documentation.
 
 <table>
   <tr>
@@ -501,7 +551,7 @@ The following constants are meant for use with [`database.applyChangeset()`](#da
   </tr>
   <tr>
     <td><code>SQLITE_CHANGESET_REPLACE</code></td>
-    <td>Conflicting changes replace existing values.</td>
+    <td>Conflicting changes replace existing values. Note that this value can only be returned when the type of conflict is either <code>SQLITE_CHANGESET_DATA</code> or <code>SQLITE_CHANGESET_CONFLICT</code>.</td>
   </tr>
   <tr>
     <td><code>SQLITE_CHANGESET_ABORT</code></td>
@@ -510,11 +560,14 @@ The following constants are meant for use with [`database.applyChangeset()`](#da
 </table>
 
 [Changesets and Patchsets]: https://www.sqlite.org/sessionintro.html#changesets_and_patchsets
+[Constants Passed To The Conflict Handler]: https://www.sqlite.org/session/c_changeset_conflict.html
+[Constants Returned From The Conflict Handler]: https://www.sqlite.org/session/c_changeset_abort.html
 [SQL injection]: https://en.wikipedia.org/wiki/SQL_injection
 [`ATTACH DATABASE`]: https://www.sqlite.org/lang_attach.html
 [`PRAGMA foreign_keys`]: https://www.sqlite.org/pragma.html#pragma_foreign_keys
 [`SQLITE_DETERMINISTIC`]: https://www.sqlite.org/c3ref/c_deterministic.html
 [`SQLITE_DIRECTONLY`]: https://www.sqlite.org/c3ref/c_deterministic.html
+[`database.applyChangeset()`]: #databaseapplychangesetchangeset-options
 [`sqlite3_changes64()`]: https://www.sqlite.org/c3ref/changes.html
 [`sqlite3_close_v2()`]: https://www.sqlite.org/c3ref/close.html
 [`sqlite3_create_function_v2()`]: https://www.sqlite.org/c3ref/create_function.html
diff --git a/src/env_properties.h b/src/env_properties.h
@@ -144,10 +144,13 @@
   V(entry_type_string, "entryType")                                            \
   V(env_pairs_string, "envPairs")                                              \
   V(env_var_settings_string, "envVarSettings")                                 \
+  V(err_sqlite_error_string, "ERR_SQLITE_ERROR")                               \
+  V(errcode_string, "errcode")                                                 \
   V(errno_string, "errno")                                                     \
   V(error_string, "error")                                                     \
-  V(events, "events")                                                          \
+  V(errstr_string, "errstr")                                                   \
   V(events_waiting, "eventsWaiting")                                           \
+  V(events, "events")                                                          \
   V(exchange_string, "exchange")                                               \
   V(expire_string, "expire")                                                   \
   V(exponent_string, "exponent")                                               \
diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc
@@ -42,6 +42,7 @@ using v8::Number;
 using v8::Object;
 using v8::SideEffectType;
 using v8::String;
+using v8::TryCatch;
 using v8::Uint8Array;
 using v8::Value;
 
@@ -66,13 +67,14 @@ inline MaybeLocal<Object> CreateSQLiteError(Isolate* isolate,
                                             const char* message) {
   Local<String> js_msg;
   Local<Object> e;
+  Environment* env = Environment::GetCurrent(isolate);
   if (!String::NewFromUtf8(isolate, message).ToLocal(&js_msg) ||
       !Exception::Error(js_msg)
            ->ToObject(isolate->GetCurrentContext())
            .ToLocal(&e) ||
       e->Set(isolate->GetCurrentContext(),
-             OneByteString(isolate, "code"),
-             OneByteString(isolate, "ERR_SQLITE_ERROR"))
+             env->code_string(),
+             env->err_sqlite_error_string())
           .IsNothing()) {
     return MaybeLocal<Object>();
   }
@@ -85,15 +87,14 @@ inline MaybeLocal<Object> CreateSQLiteError(Isolate* isolate, sqlite3* db) {
   const char* errmsg = sqlite3_errmsg(db);
   Local<String> js_errmsg;
   Local<Object> e;
+  Environment* env = Environment::GetCurrent(isolate);
   if (!String::NewFromUtf8(isolate, errstr).ToLocal(&js_errmsg) ||
       !CreateSQLiteError(isolate, errmsg).ToLocal(&e) ||
       e->Set(isolate->GetCurrentContext(),
-             OneByteString(isolate, "errcode"),
+             env->errcode_string(),
              Integer::New(isolate, errcode))
           .IsNothing() ||
-      e->Set(isolate->GetCurrentContext(),
-             OneByteString(isolate, "errstr"),
-             js_errmsg)
+      e->Set(isolate->GetCurrentContext(), env->errstr_string(), js_errmsg)
           .IsNothing()) {
     return MaybeLocal<Object>();
   }
@@ -114,6 +115,19 @@ inline void THROW_ERR_SQLITE_ERROR(Isolate* isolate, const char* message) {
   }
 }
 
+inline void THROW_ERR_SQLITE_ERROR(Isolate* isolate, int errcode) {
+  const char* errstr = sqlite3_errstr(errcode);
+
+  Environment* env = Environment::GetCurrent(isolate);
+  auto error = CreateSQLiteError(isolate, errstr).ToLocalChecked();
+  error
+      ->Set(isolate->GetCurrentContext(),
+            env->errcode_string(),
+            Integer::New(isolate, errcode))
+      .ToChecked();
+  isolate->ThrowException(error);
+}
+
 class UserDefinedFunction {
  public:
   explicit UserDefinedFunction(Environment* env,
@@ -731,11 +745,11 @@ void DatabaseSync::CreateSession(const FunctionCallbackInfo<Value>& args) {
 
 // the reason for using static functions here is that SQLite needs a
 // function pointer
-static std::function<int()> conflictCallback;
+static std::function<int(int)> conflictCallback;
 
 static int xConflict(void* pCtx, int eConflict, sqlite3_changeset_iter* pIter) {
   if (!conflictCallback) return SQLITE_CHANGESET_ABORT;
-  return conflictCallback();
+  return conflictCallback(eConflict);
 }
 
 static std::function<bool(std::string)> filterCallback;
@@ -773,15 +787,27 @@ void DatabaseSync::ApplyChangeset(const FunctionCallbackInfo<Value>& args) {
         options->Get(env->context(), env->onconflict_string()).ToLocalChecked();
 
     if (!conflictValue->IsUndefined()) {
-      if (!conflictValue->IsNumber()) {
+      if (!conflictValue->IsFunction()) {
         THROW_ERR_INVALID_ARG_TYPE(
             env->isolate(),
-            "The \"options.onConflict\" argument must be a number.");
+            "The \"options.onConflict\" argument must be a function.");
         return;
       }
-
-      int conflictInt = conflictValue->Int32Value(env->context()).FromJust();
-      conflictCallback = [conflictInt]() -> int { return conflictInt; };
+      Local<Function> conflictFunc = conflictValue.As<Function>();
+      conflictCallback = [env, conflictFunc](int conflictType) -> int {
+        Local<Value> argv[] = {Integer::New(env->isolate(), conflictType)};
+        TryCatch try_catch(env->isolate());
+        Local<Value> result =
+            conflictFunc->Call(env->context(), Null(env->isolate()), 1, argv)
+                .FromMaybe(Local<Value>());
+        if (try_catch.HasCaught()) {
+          try_catch.ReThrow();
+          return SQLITE_CHANGESET_ABORT;
+        }
+        constexpr auto invalid_value = -1;
+        if (!result->IsInt32()) return invalid_value;
+        return result->Int32Value(env->context()).FromJust();
+      };
     }
 
     if (options->HasOwnProperty(env->context(), env->filter_string())
@@ -819,12 +845,16 @@ void DatabaseSync::ApplyChangeset(const FunctionCallbackInfo<Value>& args) {
       xFilter,
       xConflict,
       nullptr);
+  if (r == SQLITE_OK) {
+    args.GetReturnValue().Set(true);
+    return;
+  }
   if (r == SQLITE_ABORT) {
+    // this is not an error, return false
     args.GetReturnValue().Set(false);
     return;
   }
-  CHECK_ERROR_OR_THROW(env->isolate(), db->connection_, r, SQLITE_OK, void());
-  args.GetReturnValue().Set(true);
+  THROW_ERR_SQLITE_ERROR(env->isolate(), r);
 }
 
 void DatabaseSync::EnableLoadExtension(
@@ -1662,6 +1692,12 @@ void DefineConstants(Local<Object> target) {
   NODE_DEFINE_CONSTANT(target, SQLITE_CHANGESET_OMIT);
   NODE_DEFINE_CONSTANT(target, SQLITE_CHANGESET_REPLACE);
   NODE_DEFINE_CONSTANT(target, SQLITE_CHANGESET_ABORT);
+
+  NODE_DEFINE_CONSTANT(target, SQLITE_CHANGESET_DATA);
+  NODE_DEFINE_CONSTANT(target, SQLITE_CHANGESET_NOTFOUND);
+  NODE_DEFINE_CONSTANT(target, SQLITE_CHANGESET_CONFLICT);
+  NODE_DEFINE_CONSTANT(target, SQLITE_CHANGESET_CONSTRAINT);
+  NODE_DEFINE_CONSTANT(target, SQLITE_CHANGESET_FOREIGN_KEY);
 }
 
 static void Initialize(Local<Object> target,
diff --git a/test/parallel/test-sqlite-session.js b/test/parallel/test-sqlite-session.js
