From: dan Date: Wed, 18 Sep 2024 15:02:27 +0000 (+0000) Subject: Fix the preupdate hook so that it works when the "old.*" row has a column with a... X-Git-Tag: version-3.47.0~107^2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=refs%2Fheads%2Fpreupdate-hook-fix;p=thirdparty%2Fsqlite.git 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 --- diff --git a/ext/session/sessionfault3.test b/ext/session/sessionfault3.test index af5a4cdb43..5af9c9ed70 100644 --- a/ext/session/sessionfault3.test +++ b/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 diff --git a/ext/session/sqlite3session.c b/ext/session/sqlite3session.c index 7a8132bfa6..72f4249234 100644 --- a/ext/session/sqlite3session.c +++ b/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 ){ diff --git a/manifest b/manifest index d371868d8a..b8687b09f6 100644 --- a/manifest +++ b/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. diff --git a/manifest.uuid b/manifest.uuid index d4cf2dec51..14998f1fd5 100644 --- a/manifest.uuid +++ b/manifest.uuid @@ -1 +1 @@ -a63e412b6b2939422ecfa99d91fccb7a9c61e1533bb0db20ff12f3815ef41a2c +00a398cf900179aa5a8aab09fe4a671d99e7a31583282848ef39390f2ef246eb diff --git a/src/vdbeInt.h b/src/vdbeInt.h index 2a23c3f285..b26860f3a6 100644 --- a/src/vdbeInt.h +++ b/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 */ }; /* diff --git a/src/vdbeapi.c b/src/vdbeapi.c index bf185dd3ad..c129b94656 100644 --- a/src/vdbeapi.c +++ b/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 ); diff --git a/src/vdbeaux.c b/src/vdbeaux.c index f1e0cccdc1..a66bdecffb 100644 --- a/src/vdbeaux.c +++ b/src/vdbeaux.c @@ -5543,5 +5543,12 @@ void sqlite3VdbePreUpdateHook( } sqlite3DbNNFreeNN(db, preupdate.aNew); } + if( preupdate.apDflt ){ + int i; + for(i=0; inCol; i++){ + sqlite3ValueFree(preupdate.apDflt[i]); + } + sqlite3DbFree(db, preupdate.apDflt); + } } #endif /* SQLITE_ENABLE_PREUPDATE_HOOK */ diff --git a/test/hook.test b/test/hook.test index 3d735875df..8638d3a6ba 100644 --- a/test/hook.test +++ b/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