Fix the preupdate hook so that it works when the "old.*" row has a column with a non-NULL default value that was added by ALTER TABLE ADD COLUMN after the current record was created.

FossilOrigin-Name: 00a398cf900179aa5a8aab09fe4a671d99e7a31583282848ef39390f2ef246eb

ext/session/sessionfault3.test

@@ -56,4 +56,28 @@ do_faultsim_test 1 -faults oom* -prep {
   faultsim_test_result {0 {}} {1 SQLITE_NOMEM}
 }
 
+#-------------------------------------------------------------------------
+reset_db
+do_execsql_test 2.0 {
+  CREATE TABLE t1(a INTEGER PRIMARY KEY, b);
+  INSERT INTO t1 VALUES(1, 'one');
+  INSERT INTO t1 VALUES(2, 'two');
+  ALTER TABLE t1 ADD COLUMN c DEFAULT 'abcdefghijklmnopqrstuvwxyz';
+}
+faultsim_save_and_close
+
+do_faultsim_test 2 -faults oom-t* -prep {
+  faultsim_restore_and_reopen
+  db eval {SELECT * FROM sqlite_schema}
+} -body {
+  sqlite3session S db main
+  S attach *
+  execsql {
+    DELETE FROM t1 WHERE a = 1;
+  }
+} -test {
+  faultsim_test_result {0 {}} {1 SQLITE_NOMEM}
+  catch { S delete }
+}
+
 finish_test

ext/session/sqlite3session.c

@@ -48,8 +48,7 @@ struct sqlite3_session {
   int bEnable;                    /* True if currently recording */
   int bIndirect;                  /* True if all changes are indirect */
   int bAutoAttach;                /* True to auto-attach tables */
-  int bImplicitPK;                /* True to handle tables with implicit PK */
-  int rc;                         /* Non-zero if an error has occurred */
+  int bImplicitPK;                /* True to handle tables with implicit PK */ int rc;                         /* Non-zero if an error has occurred */
   void *pFilterCtx;               /* First argument to pass to xTableFilter */
   int (*xTableFilter)(void *pCtx, const char *zTab);
   i64 nMalloc;                    /* Number of bytes of data allocated */
@@ -1757,16 +1756,19 @@ static void sessionPreupdateOneChange(
       for(i=0; i<(pTab->nCol-pTab->bRowid); i++){
         sqlite3_value *p = 0;
         if( op!=SQLITE_INSERT ){
-          TESTONLY(int trc = ) pSession->hook.xOld(pSession->hook.pCtx, i, &p);
-          assert( trc==SQLITE_OK );
+          /* This may fail if the column has a non-NULL default and was added 
+          ** using ALTER TABLE ADD COLUMN after this record was created. */
+          rc = pSession->hook.xOld(pSession->hook.pCtx, i, &p);
         }else if( pTab->abPK[i] ){
           TESTONLY(int trc = ) pSession->hook.xNew(pSession->hook.pCtx, i, &p);
           assert( trc==SQLITE_OK );
         }
 
-        /* This may fail if SQLite value p contains a utf-16 string that must
-        ** be converted to utf-8 and an OOM error occurs while doing so. */
-        rc = sessionSerializeValue(0, p, &nByte);
+        if( rc==SQLITE_OK ){
+          /* This may fail if SQLite value p contains a utf-16 string that must
+          ** be converted to utf-8 and an OOM error occurs while doing so. */
+          rc = sessionSerializeValue(0, p, &nByte);
+        }
         if( rc!=SQLITE_OK ) goto error_out;
       }
       if( pTab->bRowid ){

manifest

@@ -1,5 +1,5 @@
-C Fix\sharmless\sstatic\sanalyzer\swarning\sin\ssqlite3-rsync.
-D 2024-09-17T22:57:08.639
+C Fix\sthe\spreupdate\shook\sso\sthat\sit\sworks\swhen\sthe\s"old.*"\srow\shas\sa\scolumn\swith\sa\snon-NULL\sdefault\svalue\sthat\swas\sadded\sby\sALTER\sTABLE\sADD\sCOLUMN\safter\sthe\scurrent\srecord\swas\screated.
+D 2024-09-18T15:02:27.631
 F .fossil-settings/empty-dirs dbb81e8fc0401ac46a1491ab34a7f2c7c0452f2f06b54ebb845d024ca8283ef1
 F .fossil-settings/ignore-glob 35175cdfcf539b2318cb04a9901442804be81cd677d8b889fcc9149c21f239ea
 F LICENSE.md df5091916dbb40e6e9686186587125e1b2ff51f022cc334e886c19a0e9982724
@@ -583,7 +583,7 @@ F ext/session/sessionconflict.test 8b8cbd98548e2e636ddc17d0986276f60e833fb865617
 F ext/session/sessiondiff.test e89f7aedcdd89e5ebac3a455224eb553a171e9586fc3e1e6a7b3388d2648ba8d
 F ext/session/sessionfault.test c2b43d01213b389a3f518e90775fca2120812ba51e50444c4066962263e45c11
 F ext/session/sessionfault2.test b0d6a7c1d7398a7e800d84657404909c7d385965ea8576dc79ed344c46fbf41c
-F ext/session/sessionfault3.test 7c7547202775de268f3fe6f074c4d0d165151829710b4e64f90d4a01645ba9e7
+F ext/session/sessionfault3.test ce0b5d182133935c224d72507dbf1c5be1a1febf7e85d0b0fbd6d2f724b32b96
 F ext/session/sessioninvert.test 04075517a9497a80d39c495ba6b44f3982c7371129b89e2c52219819bc105a25
 F ext/session/sessionmem.test f2a735db84a3e9e19f571033b725b0b2daf847f3f28b1da55a0c1a4e74f1de09
 F ext/session/sessionnoact.test 506526a5fe29421ecc50d371774ef1bb04cbd9d906a8a468f0556cdbde184c22
@@ -594,7 +594,7 @@ F ext/session/sessionrowid.test 85187c2f1b38861a5844868126f69f9ec62223a03449a98a
 F ext/session/sessionsize.test 8fcf4685993c3dbaa46a24183940ab9f5aa9ed0d23e5fb63bfffbdb56134b795
 F ext/session/sessionstat1.test 5e718d5888c0c49bbb33a7a4f816366db85f59f6a4f97544a806421b85dc2dec
 F ext/session/sessionwor.test 6fd9a2256442cebde5b2284936ae9e0d54bde692d0f5fd009ecef8511f4cf3fc
-F ext/session/sqlite3session.c c7473aafbd88f796391a8c25aa90975a8f3729ab7f4f8cf74ab9d3b014e10abe
+F ext/session/sqlite3session.c e74d3e58d4c20e72c14c23f909e1a6c7ea71ae31129fb1685babf2cab558d613
 F ext/session/sqlite3session.h 683ccbf16e2c2521661fc4c1cf918ce57002039efbcabcd8097fa4bca569104b
 F ext/session/test_session.c aa29abdcc9011ac02f4fa38e8ede226106eaeee7c3ea7d8b2b999a124e0c368c
 F ext/userauth/sqlite3userauth.h 7f3ea8c4686db8e40b0a0e7a8e0b00fac13aa7a3
@@ -839,9 +839,9 @@ F src/util.c 5d1a0134cf4240648d1c6bb5cc8efaca0ea2b5d5c840985aec7e947271f04375
 F src/vacuum.c b763b6457bd058d2072ef9364832351fd8d11e8abf70cbb349657360f7d55c40
 F src/vdbe.c be5f58bc29f60252e041a618eae59e8d57d460ba136c5403cf0abf955560c457
 F src/vdbe.h c2549a215898a390de6669cfa32adba56f0d7e17ba5a7f7b14506d6fd5f0c36a
-F src/vdbeInt.h 949669dfd8a41550d27dcb905b494f2ccde9a2e6c1b0b04daa1227e2e74c2b2c
-F src/vdbeapi.c 7c4e2f7635ea1ab7db5e1e24b33a1c20e4a123e926456614f064b58b13b85992
-F src/vdbeaux.c 25d685cafe119ff890c94345e884ea558a6b5d823bfa52ba708eb8ff3c70aa71
+F src/vdbeInt.h af7d7e8291edd0b19f2cd698e60e4d4031078f9a2f2328ac8f0b7efb134f8a1d
+F src/vdbeapi.c 483a391801ecacfeaa1d202c1d4dc2d7e520ba67566ca540868602117d6bdec5
+F src/vdbeaux.c 676dbee99b4febdd94bc9658667a2e3bc413c4c0e356242d90f98a1155d513e5
 F src/vdbeblob.c 255be187436da38b01f276c02e6a08103489bbe2a7c6c21537b7aecbe0e1f797
 F src/vdbemem.c 831a244831eaa45335f9ae276b50a7a82ee10d8c46c2c72492d4eb8c98d94d89
 F src/vdbesort.c d0a3c7056c081703c8b6d91ad60f17da5e062a5c64bf568ed0fa1b5f4cae311f
@@ -1286,7 +1286,7 @@ F test/genesis.tcl 1e2e2e8e5cc4058549a154ff1892fe5c9de19f98
 F test/having.test a89236dd8d55aa50c4805f82ac9daf64d477a44d712d8209c118978d0ca21ec9
 F test/hexlit.test 4a6a5f46e3c65c4bf1fa06f5dd5a9507a5627751
 F test/hidden.test 23c1393a79e846d68fd902d72c85d5e5dcf98711
-F test/hook.test 18cae9140fa7f9a6f346e892a3fe3e31b2ca0be1494cd01b918adb74281016a6
+F test/hook.test 3481a68009fe143e3363fca922f6fc7a1e1f3776c51e42777f1a01b26ad2a9c8
 F test/hook2.test b9ff3b8c6519fb67f33192f1afe86e7782ee4ac8
 F test/icu.test 8da7d52cd9722c82f33b0466ed915460cb03c23a38f18a9a2d3ff97da9a4a8c0
 F test/ieee754.test b0945d12be7d255f3dfa18e2511b17ca37e0edd2b803231c52d05b86c04ab26e
@@ -2213,8 +2213,11 @@ F vsixtest/vsixtest.tcl 6195aba1f12a5e10efc2b8c0009532167be5e301abe5b31385638080
 F vsixtest/vsixtest.vcxproj.data 2ed517e100c66dc455b492e1a33350c1b20fbcdc
 F vsixtest/vsixtest.vcxproj.filters 37e51ffedcdb064aad6ff33b6148725226cd608e
 F vsixtest/vsixtest_TemporaryKey.pfx e5b1b036facdb453873e7084e1cae9102ccc67a0
-P 97528788145b83a1486dbaf09326ebedbc07bd0b47a57cdff773885b0b984604
-R de863a50ae68edb342391097eaaa3380
-U drh
-Z d1cb55d24cd7d46dead0c36bd671adaa
+P a63e412b6b2939422ecfa99d91fccb7a9c61e1533bb0db20ff12f3815ef41a2c
+R 2022f5c82821db1ada9b61c2c71e65f9
+T *branch * preupdate-hook-fix
+T *sym-preupdate-hook-fix *
+T -sym-trunk *
+U dan
+Z c649b0105c7af3011732c3c809572a4f
 # Remove this line to create a well-formed Fossil manifest.

manifest.uuid

@@ -1 +1 @@
-a63e412b6b2939422ecfa99d91fccb7a9c61e1533bb0db20ff12f3815ef41a2c
+00a398cf900179aa5a8aab09fe4a671d99e7a31583282848ef39390f2ef246eb

src/vdbeInt.h

@@ -543,6 +543,7 @@ struct PreUpdate {
   Mem *aNew;                      /* Array of new.* values */
   Table *pTab;                    /* Schema object being updated */
   Index *pPk;                     /* PK index if pTab is WITHOUT ROWID */
+  sqlite3_value **apDflt;         /* Array of default values, if required */
 };
 
 /*

src/vdbeapi.c

@@ -2222,7 +2222,28 @@ int sqlite3_preupdate_old(sqlite3 *db, int iIdx, sqlite3_value **ppValue){
   if( iIdx==p->pTab->iPKey ){
     sqlite3VdbeMemSetInt64(pMem, p->iKey1);
   }else if( iIdx>=p->pUnpacked->nField ){
-    *ppValue = (sqlite3_value *)columnNullValue();
+    /* This occurs when the table has been extended using ALTER TABLE
+    ** ADD COLUMN. The value to return is the default value of the column. */
+    Column *pCol = &p->pTab->aCol[iIdx];
+    if( pCol->iDflt>0 ){
+      if( p->apDflt==0 ){
+        int nByte = sizeof(sqlite3_value*)*p->pTab->nCol;
+        p->apDflt = (sqlite3_value**)sqlite3DbMallocZero(db, nByte);
+        if( p->apDflt==0 ) goto preupdate_old_out;
+      }
+      if( p->apDflt[iIdx]==0 ){
+        Expr *pDflt = p->pTab->u.tab.pDfltList->a[pCol->iDflt-1].pExpr;
+        sqlite3_value *pVal = 0;
+        rc = sqlite3ValueFromExpr(db, pDflt, ENC(db), pCol->affinity, &pVal);
+        if( rc==SQLITE_OK && pVal==0 ){
+          rc = SQLITE_CORRUPT_BKPT;
+        }
+        p->apDflt[iIdx] = pVal;
+      }
+      *ppValue = p->apDflt[iIdx];
+    }else{
+      *ppValue = (sqlite3_value *)columnNullValue();
+    }
   }else if( p->pTab->aCol[iIdx].affinity==SQLITE_AFF_REAL ){
     if( pMem->flags & (MEM_Int|MEM_IntReal) ){
       testcase( pMem->flags & MEM_Int );

src/vdbeaux.c

@@ -5543,5 +5543,12 @@ void sqlite3VdbePreUpdateHook(
     }
     sqlite3DbNNFreeNN(db, preupdate.aNew);
   }
+  if( preupdate.apDflt ){
+    int i;
+    for(i=0; i<pTab->nCol; i++){
+      sqlite3ValueFree(preupdate.apDflt[i]);
+    }
+    sqlite3DbFree(db, preupdate.apDflt);
+  }
 }
 #endif /* SQLITE_ENABLE_PREUPDATE_HOOK */

test/hook.test

@@ -706,11 +706,13 @@ ifcapable altertable {
   }
 }
 
-if 0 {
+if 1 {
   # At time of writing, these two are broken. They demonstrate that the
   # sqlite3_preupdate_old() method does not handle the case where ALTER TABLE
   # has been used to add a column with a default value other than NULL.
   #
+  # 2024-09-18: These are now fixed.
+  #
   do_preupdate_test 7.5.2.1 {
     DELETE FROM t8 WHERE a = 'one'
   } {
@@ -1022,4 +1024,37 @@ do_catchsql_test 12.6 {
   INSERT INTO t4 VALUES('def', 3);
 } {1 {UNIQUE constraint failed: t4.a}}
 
+#-------------------------------------------------------------------------
+# Test adding non-NULL default values using ALTER TABLE.
+#
+reset_db
+db preupdate hook preupdate_hook
+do_execsql_test 13.0 {
+  CREATE TABLE t1(a INTEGER PRIMARY KEY);
+  INSERT INTO t1 VALUES(100), (200), (300), (400);
+}
+
+do_execsql_test 13.1 {
+  ALTER TABLE t1 ADD COLUMN b DEFAULT 1234;
+  ALTER TABLE t1 ADD COLUMN c DEFAULT 'abcdef';
+  ALTER TABLE t1 ADD COLUMN d DEFAULT NULL;
+}
+
+do_preupdate_test 13.2 {
+  DELETE FROM t1 WHERE a=300
+} {DELETE main t1 300 300 0 300 1234 abcdef {}}
+
+do_preupdate_test 13.3 {
+  UPDATE t1 SET d='hello world' WHERE a=200
+} {
+  UPDATE main t1 200 200 0 200 1234 abcdef {} 
+                           200 1234 abcdef {hello world}
+}
+
+do_preupdate_test 13.4 {
+  INSERT INTO t1 DEFAULT VALUES;
+} {
+  INSERT main t1 401 401 0 401 1234 abcdef {}
+}
+
 finish_test