From: dan <Dan Kennedy>
Date: Fri, 27 Sep 2024 18:32:52 +0000 (+0000)
Subject: Fix a problem with UPDATEs that do not modify all UNINDEXED columns of a contentless_... 
X-Git-Tag: version-3.47.0~70^2~4
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=08f1ba0767d6bfe1cf098e8e81aa74681277ec0f;p=thirdparty%2Fsqlite.git

Fix a problem with UPDATEs that do not modify all UNINDEXED columns of a contentless_delete=1, contentless_unindexed=1 table.

FossilOrigin-Name: b6b1db8d343d3e55c3a5589af3ec629762e06c6b689b77defd445347198cb2e7
---

diff --git a/ext/fts5/fts5_main.c b/ext/fts5/fts5_main.c
index ff8b76694b..4ea727cac2 100644
--- a/ext/fts5/fts5_main.c
+++ b/ext/fts5/fts5_main.c
@@ -1809,16 +1809,38 @@ static void fts5StorageInsert(
   *pRc = rc;
 }
 
-static int fts5ContentlessUpdateOk(
+/*
+**
+** This function is called when the user attempts an UPDATE on a contentless
+** table. Parameter bRowidModified is true if the UPDATE statement modifies
+** the rowid value. Parameter apVal[] contains the new values for each user
+** defined column of the fts5 table. pConfig is the configuration object of the
+** table being updated (guaranteed to be contentless). The contentless_delete=1
+** and contentless_unindexed=1 options may or may not be set. 
+**
+** This function returns SQLITE_OK if the UPDATE can go ahead, or an SQLite
+** error code if it cannot. In this case an error message is also loaded into
+** pConfig. Output parameter (*pbContent) is set to true if the caller should
+** update the %_content table only - not the FTS index or any other shadow
+** table. This occurs when an UPDATE modifies only UNINDEXED columns of the
+** table.
+**
+** An UPDATE may proceed if:
+**
+**   * The only columns modified are UNINDEXED columns, or
+**
+**   * The contentless_delete=1 option was specified and all of the indexed
+**     columns (not a subset) have been modified.
+*/
+static int fts5ContentlessUpdate(
   Fts5Config *pConfig,
   sqlite3_value **apVal,
-  i64 iOld,
-  i64 iNew,
+  int bRowidModified,
   int *pbContent
 ){
   int ii;
-  int bSeenIndex = 0;
-  int bSeenIndexNC = 0;
+  int bSeenIndex = 0;             /* Have seen modified indexed column */
+  int bSeenIndexNC = 0;           /* Have seen unmodified indexed column */
   int rc = SQLITE_OK;
 
   for(ii=0; ii<pConfig->nCol; ii++){
@@ -1831,7 +1853,7 @@ static int fts5ContentlessUpdateOk(
     }
   }
 
-  if( bSeenIndex==0 && iOld==iNew ){
+  if( bSeenIndex==0 && bRowidModified==0 ){
     *pbContent = 1;
   }else{
     if( bSeenIndexNC || pConfig->bContentlessDelete==0 ){
@@ -1934,25 +1956,10 @@ static int fts5UpdateMethod(
     assert( eType0==SQLITE_INTEGER || eType0==SQLITE_NULL );
     assert( nArg!=1 || eType0==SQLITE_INTEGER );
 
-    /*
-    ** Extra rule for contentless tables:
-    **
-    ** DELETE:
-    **   It is only possible to DELETE if the contentless_delete=1 flag
-    **   is set.
-    **
-    ** UPDATE:
-    **   A "content-only" UPDATE is one that only affects UNINDEXED
-    **   columns. And that does not modify the rowid value.
-    **
-    **   If the contentless_delete flag is clear, then content-only UPDATEs
-    **   are the only ones supported. Or, if contentless_delete=1 is set,
-    **   then updates that modify all indexed columns are also supported.
-    **   If all indexed columns are updated, then rowid updates are allowed.
-    */
-
     /* DELETE */
     if( nArg==1 ){
+      /* It is only possible to DELETE from a contentless table if the
+      ** contentless_delete=1 flag is set. */
       if( fts5IsContentless(pTab, 1) && pConfig->bContentlessDelete==0 ){
         fts5SetVtabError(pTab, 
             "cannot DELETE from contentless fts5 table: %s", pConfig->zName
@@ -2001,8 +2008,10 @@ static int fts5UpdateMethod(
         i64 iNew = sqlite3_value_int64(apVal[1]);  /* New rowid */
         int bContent = 0;         /* Content only update */
 
+        /* If this is a contentless table (including contentless_unindexed=1
+        ** tables), check if the UPDATE may proceed.  */
         if( fts5IsContentless(pTab, 1) ){
-          rc = fts5ContentlessUpdateOk(pConfig,&apVal[2],iOld,iNew,&bContent);
+          rc = fts5ContentlessUpdate(pConfig, &apVal[2], iOld!=iNew, &bContent);
           if( rc!=SQLITE_OK ) goto update_out;
         }
 
@@ -2029,6 +2038,10 @@ static int fts5UpdateMethod(
             }
           }
         }else if( bContent ){
+          /* This occurs when an UPDATE on a contentless table affects *only*
+          ** UNINDEXED columns. This is a no-op for contentless_unindexed=0
+          ** tables, or a write to the %_content table only for =1 tables.  */
+          assert( fts5IsContentless(pTab, 1) );
           rc = sqlite3Fts5StorageFindDeleteRow(pStorage, iOld);
           if( rc==SQLITE_OK ){
             rc = sqlite3Fts5StorageContentInsert(pStorage, 1, apVal, pRowid);
diff --git a/ext/fts5/fts5_storage.c b/ext/fts5/fts5_storage.c
index 7f149eee8a..31f5fc5dc3 100644
--- a/ext/fts5/fts5_storage.c
+++ b/ext/fts5/fts5_storage.c
@@ -739,6 +739,12 @@ int sqlite3Fts5StorageDelete(
   if( rc==SQLITE_OK ){
     if( p->pConfig->bContentlessDelete ){
       rc = fts5StorageContentlessDelete(p, iDel);
+      if( rc==SQLITE_OK 
+       && bSaveRow 
+       && p->pConfig->eContent==FTS5_CONTENT_UNINDEXED
+      ){
+        rc = sqlite3Fts5StorageFindDeleteRow(p, iDel);
+      }
     }else{
       rc = fts5StorageDeleteFromIndex(p, iDel, apVal, bSaveRow);
     }
diff --git a/ext/fts5/test/fts5update2.test b/ext/fts5/test/fts5update2.test
new file mode 100644
index 0000000000..a402b3cb23
--- /dev/null
+++ b/ext/fts5/test/fts5update2.test
@@ -0,0 +1,122 @@
+# 2024 Sep 27
+#
+# The author disclaims copyright to this source code.  In place of
+# a legal notice, here is a blessing:
+#
+#    May you do good and not evil.
+#    May you find forgiveness for yourself and forgive others.
+#    May you share freely, never taking more than you give.
+#
+#*************************************************************************
+# This file implements regression tests for SQLite library.  The
+# focus of this script is testing the FTS5 module.
+#
+
+source [file join [file dirname [info script]] fts5_common.tcl]
+set testprefix fts5update2
+
+# If SQLITE_ENABLE_FTS5 is not defined, omit this file.
+ifcapable !fts5 {
+  finish_test
+  return
+}
+
+
+#-------------------------------------------------------------------------
+# Test that the various types of UPDATE statement are handled correctly
+# by different table types.
+#
+foreach {tn cu} {
+  1 0
+  2 1
+} {
+  reset_db
+  do_execsql_test 1.$tn.1 "
+    CREATE VIRTUAL TABLE ft1 USING fts5(a, b UNINDEXED, c UNINDEXED, d,
+        content='',
+        contentless_unindexed=$cu
+    );
+    CREATE VIRTUAL TABLE ft2 USING fts5(a, b UNINDEXED, c UNINDEXED, d,
+        content='',
+        contentless_unindexed=$cu, contentless_delete=1
+    );
+  "
+
+  do_execsql_test 1.$tn.2 {
+    INSERT INTO ft1(rowid, a, b, c, d) VALUES(1, 'a1', 'b1', 'c1', 'd1');
+    INSERT INTO ft1(rowid, a, b, c, d) VALUES(2, 'a2', 'b2', 'c2', 'd2');
+    INSERT INTO ft1(rowid, a, b, c, d) VALUES(3, 'a3', 'b3', 'c3', 'd3');
+
+    INSERT INTO ft2(rowid, a, b, c, d) VALUES(1, 'a1', 'b1', 'c1', 'd1');
+    INSERT INTO ft2(rowid, a, b, c, d) VALUES(2, 'a2', 'b2', 'c2', 'd2');
+    INSERT INTO ft2(rowid, a, b, c, d) VALUES(3, 'a3', 'b3', 'c3', 'd3');
+  }
+
+  # It should be possible to update a subset of the UNINDEXED columns of
+  # a contentless table. Regardless of whether or not contentless_unindexed=1
+  # or contentless_delete=1 is set.
+  do_execsql_test 1.$tn.3 {
+    UPDATE ft1 SET b=b||'.1';
+    UPDATE ft2 SET b=b||'.1';
+  }
+  do_execsql_test 1.$tn.4 {
+    UPDATE ft1 SET b=b||'.2', c=c||'.2';
+    UPDATE ft2 SET b=b||'.2', c=c||'.2';
+  }
+
+  set res(0) {
+    1 {} {} {} {}
+    2 {} {} {} {}
+    3 {} {} {} {}
+  }
+  set res(1) {
+    1 {} b1.1.2 c1.2 {}
+    2 {} b2.1.2 c2.2 {}
+    3 {} b3.1.2 c3.2 {}
+  }
+
+  do_execsql_test 1.$tn.5 {
+    SELECT rowid, * FROM ft2
+  } $res($cu)
+
+  do_execsql_test 1.6.1 { SELECT rowid FROM ft1('a2') } {2}
+  do_execsql_test 1.6.2 { SELECT rowid FROM ft2('a2') } {2}
+
+  # It should be possible to update all indexed columns (but no other subset)
+  # if the contentless_delete=1 option is set, as it is for "ft2".
+  do_execsql_test 1.$tn.7 {
+    UPDATE ft2 SET a='a22', d='d22' WHERE rowid=2;
+  }
+  do_execsql_test 1.$tn.8 { SELECT rowid FROM ft2('a22 AND d22') } {2}
+
+  do_execsql_test 1.$tn.9 {
+    UPDATE ft2 SET a='a33', d='d33', b='b3' WHERE rowid=3;
+  }
+
+  set res(1) {
+    1 {} b1.1.2 c1.2 {}
+    2 {} b2.1.2 c2.2 {}
+    3 {} b3 c3.2 {}
+  }
+  do_execsql_test 1.$tn.10 {
+    SELECT rowid, * FROM ft2
+  } $res($cu)
+
+  do_catchsql_test 1.$tn.11 {
+    UPDATE ft2 SET a='a11' WHERE rowid=1
+  } {1 {cannot UPDATE a subset of columns on fts5 contentless-delete table: ft2}}
+  do_catchsql_test 1.$tn.12 {
+    UPDATE ft2 SET d='d11' WHERE rowid=1
+  } {1 {cannot UPDATE a subset of columns on fts5 contentless-delete table: ft2}}
+
+  # It is not possible to update the values of indexed columns if
+  # contentless_delete=1 is not set.
+  do_catchsql_test 1.$tn.13 {
+    UPDATE ft1 SET a='a11' WHERE rowid=1
+  } {1 {cannot UPDATE contentless fts5 table: ft1}}
+  do_catchsql_test 1.$tn.14 {
+    UPDATE ft1 SET d='d11' WHERE rowid=1
+  } {1 {cannot UPDATE contentless fts5 table: ft1}}
+}
+
+finish_test
diff --git a/manifest b/manifest
index 2888f0d958..9524da68c3 100644
--- a/manifest
+++ b/manifest
@@ -1,5 +1,5 @@
-C Merge\strunk\schanges\sinto\sthis\sbranch.
-D 2024-09-27T11:35:22.006
+C Fix\sa\sproblem\swith\sUPDATEs\sthat\sdo\snot\smodify\sall\sUNINDEXED\scolumns\sof\sa\scontentless_delete=1,\scontentless_unindexed=1\stable.
+D 2024-09-27T18:32:52.251
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
 F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724
@@ -100,8 +100,8 @@ F ext/fts5/fts5_config.c a6633d88596758941c625b526075b85d3d9fd1089d8d9eab5db6e8a
 F ext/fts5/fts5_expr.c 9a56f53700d1860f0ee2f373c2b9074eaf2a7aa0637d0e27a6476de26a3fee33
 F ext/fts5/fts5_hash.c adda4272be401566a6e0ba1acbe70ee5cb97fce944bc2e04dc707152a0ec91b1
 F ext/fts5/fts5_index.c 571483823193f09439356741669aa8c81da838ae6f5e1bfa7517f7ee2fb3addd
-F ext/fts5/fts5_main.c 853d44c804b6d842f0197002a03ecf482b721834b0bf3d2f1ab3a21e4fd13df5
-F ext/fts5/fts5_storage.c 3b5d743e97502263961e706aedd1000c394ee9df7942ba6671ce4bc41fcf9993
+F ext/fts5/fts5_main.c 4e4a23b76b7ec8d93c6bcad0468d0871cf2d6d270369bfa80d2e1d3a1fc33d92
+F ext/fts5/fts5_storage.c 337b05e4c66fc822d031e264d65bde807ec2fab08665ca2cc8aaf9c5fa06792c
 F ext/fts5/fts5_tcl.c 4db9258a7882c5eac0da4433042132aaf15b87dd1e1636c7a6ca203abd2c8bfe
 F ext/fts5/fts5_test_mi.c 08c11ec968148d4cb4119d96d819f8c1f329812c568bac3684f5464be177d3ee
 F ext/fts5/fts5_test_tok.c 3cb0a9b508b30d17ef025ccddd26ae3dc8ddffbe76c057616e59a9aa85d36f3b
@@ -248,6 +248,7 @@ F ext/fts5/test/fts5unicode4.test 728c8f0caafb05567f524ad313d9f8b780fa45987b8a8d
 F ext/fts5/test/fts5unindexed.test 168838d2c385e131120bbf5b516d2432a5fabc4caa2259c932e1d49ae209a4ae
 F ext/fts5/test/fts5unindexed2.test 516236eceaac05ace322290a0d3705b4c4ffe4760d8eb9d014d9d27d56dfcc02
 F ext/fts5/test/fts5update.test b8affd796e45c94a4d19ad5c26606ea06065a0f162a9562d9f005b5a80ccf0bc
+F ext/fts5/test/fts5update2.test 1564d67b209a7d9358c450ecc53b12fcfbf176e865a3d340a31fc603cf3c0c2b
 F ext/fts5/test/fts5version.test c22d163c17e60a99f022cbc52de5a48bb7f84deaa00fe15e9bc4c3aa1996204e
 F ext/fts5/test/fts5vocab.test 2a2bdb60d0998fa3124d541b6d30b019504918dc43a6584645b63a24be72f992
 F ext/fts5/test/fts5vocab2.test bbba149c254375d00055930c1a501c9a51e80b0d20bf7b98f3e9fa3b03786373
@@ -2216,8 +2217,8 @@ F vsixtest/vsixtest.tcl 6195aba1f12a5e10efc2b8c0009532167be5e301abe5b31385638080
 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc
 F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e
 F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0
-P cd36d66c88d7282eb0a3ccde5713253f72f5843e451b2693b71adfdae28b41fb 27ef1909bb0c4d9470c6074b40500632c68341127a079a3eb3b6a19dbfb2aeac
-R af6ff70b1f29deaae408cb2f001619bf
+P 4a26a4e0015bc42b1d007def3750caf7baefe429270a295cc2f4499c98c07247
+R 95c3ae17f3cd6ac697952fa425e027b2
 U dan
-Z 395c4e3ba81ec86081df96a010608d77
+Z 35986965a22be180ac5ad4eb973f5b34
 # Remove this line to create a well-formed Fossil manifest.
diff --git a/manifest.uuid b/manifest.uuid
index 9188350703..7207c2799c 100644
--- a/manifest.uuid
+++ b/manifest.uuid
@@ -1 +1 @@
-4a26a4e0015bc42b1d007def3750caf7baefe429270a295cc2f4499c98c07247
+b6b1db8d343d3e55c3a5589af3ec629762e06c6b689b77defd445347198cb2e7