(opendal) branch main updated: fix(services/d1): repair the upsert statement that breaks every write (#8068)

[er...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

erickguan pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/opendal.git


The following commit(s) were added to refs/heads/main by this push:
     new daeed4485 fix(services/d1): repair the upsert statement that breaks every write (#8068)
daeed4485 is described below

commit daeed44853296c3f8a20bd0abc171830348d13e0
Author: ZIHAN DAI <99...@users.noreply.github.com>
AuthorDate: Sat Aug 15 14:32:43 2026 +1000

    fix(services/d1): repair the upsert statement that breaks every write (#8068)
    
    * fix(services/d1): repair the upsert statement that breaks every write
    
    The statement built in D1Core::set carries two defects, and either one
    alone is enough to stop a write from landing.
    
    It is a raw string, so the three trailing backslashes are literal
    backslash characters rather than line continuations. SQLite stops at the
    first one:
    
      sqlite> INSERT INTO "data" ("key", "value") \
      Parse error near line 1: unrecognized token: "\"
    
    And the key placeholder is quoted. 'val' is a string literal in SQL, so
    '?' is the one-character string "?" rather than a bind marker. That
    leaves a single placeholder for the two parameters set always sends, and
    every row lands under the literal key "?":
    
      sqlite> INSERT INTO "data" ("key","value") VALUES ('?','hello') ...
      sqlite> SELECT '['||key||'] -> '||value FROM data;
      [?] -> hello
    
    Both arrived together in 311f324cc (#7857), which rewrote the literal as
    a raw string in order to quote the table and column identifiers:
    
      -  "INSERT INTO {table} ({key_field}, {value_field}) \
      -      VALUES (?, ?) \
      +  r#"INSERT INTO "{table}" ("{key_field}", "{value_field}") \
      +      VALUES ('?', ?) \
    
    The identifier quoting that change was after is kept; only the escaping
    and the placeholder are corrected. The statement moves into a
    build_set_query helper so it can be asserted on directly.
    
    The rest of the crate is unaffected: delete and the two SELECTs are
    single-line raw strings with bare `?`, and a sweep for the shape --
    a raw string with a line-ending backslash -- finds no other occurrence
    in the repository.
    
    One observation left out of this change, for a maintainer to judge
    separately: set matches on the HTTP status alone, while get and
    get_length both run D1Response::parse. D1 answers a rejected statement
    with 200 and success:false, so the parse error above was reported to
    callers as a successful write. That is why a service whose every write
    fails has no bug report against it.
    
    Two unit tests. Restoring either defect on its own fails
    set_query_binds_both_parameters; the second test pins the identifier
    quoting and passes against both the old and the new statement, so it
    holds the #7857 intent in place.
    
    * Address review: inline the statement and drop the test module
    
    Review points from @erickguan, both applied:
    
    - build_set_query removed; the format! goes back inline at the call
      site, with the same local bindings the original had
    - test module removed
    
    The diff against main is now three lines: the three literal backslashes
    go, and the key placeholder loses its quotes.
---
 core/services/d1/src/core.rs | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/core/services/d1/src/core.rs b/core/services/d1/src/core.rs
index 9e99f5e59..5b19d8f9f 100644
--- a/core/services/d1/src/core.rs
+++ b/core/services/d1/src/core.rs
@@ -133,9 +133,9 @@ impl D1Core {
         let key_field = &self.key_field;
         let value_field = &self.value_field;
         let query = format!(
-            r#"INSERT INTO "{table}" ("{key_field}", "{value_field}") \
-                VALUES ('?', ?) \
-                ON CONFLICT ("{key_field}") \
+            r#"INSERT INTO "{table}" ("{key_field}", "{value_field}")
+                VALUES (?, ?)
+                ON CONFLICT ("{key_field}")
                     DO UPDATE SET "{value_field}" = EXCLUDED."{value_field}""#,
         );