From 79976a4c15b19753ea099dc5772ad0dc39543617 Mon Sep 17 00:00:00 2001 From: Tim Date: Tue, 25 Aug 2026 15:35:59 -0400 Subject: [PATCH 1/2] Fire sqlite3_update_hook on clustered PRIMARY KEY tables Non-INTEGER PRIMARY KEY tables are stored as WITHOUT ROWID, so INSERT and UPDATE go through OP_IdxInsert and never reached the OP_Insert update hook. DELETE already used OP_Delete but skipped the hook when HasRowid was false. For VisibleRowid clustered tables, attach the Table to the PK OP_IdxInsert and invoke the hook with the synthetic SQL rowid. DELETE captures sqlite3BtreeSqlRowid before the btree remove. Explicit WITHOUT ROWID stays silent, matching stock. Fixes #2427. Co-Authored-By: Grok 4.6 --- src/insert.c | 18 +++++++ src/vdbe.c | 57 ++++++++++++++++++--- test/doltlite_regression_test_c.c | 84 ++++++++++++++++++++++++++++++- 3 files changed, 152 insertions(+), 7 deletions(-) diff --git a/src/insert.c b/src/insert.c index 40a7ab4f40..70c0790a1e 100644 --- a/src/insert.c +++ b/src/insert.c @@ -2879,7 +2879,19 @@ void sqlite3CompleteInsertion( } #endif } +#ifdef DOLTLITE_PROLLY + if( VisibleRowid(pTab) ){ + pik_flags |= (update_flags & OPFLAG_ISUPDATE); + } +#endif } +#ifdef DOLTLITE_PROLLY + if( (pik_flags & OPFLAG_NCHANGE)!=0 && VisibleRowid(pTab) + && !pParse->nested ){ + sqlite3VdbeAddOp3(v, OP_IdxInsert, iIdxCur+i, aRegIdx[i], aRegIdx[i]+1); + sqlite3VdbeAppendP4(v, pTab, P4_TABLE); + }else +#endif sqlite3VdbeAddOp4Int(v, OP_IdxInsert, iIdxCur+i, aRegIdx[i], aRegIdx[i]+1, pIdx->uniqNotNull ? pIdx->nKeyCol: pIdx->nColumn); @@ -3518,6 +3530,12 @@ static int xferOptimization( } } sqlite3VdbeAddOp2(v, OP_IdxInsert, iDest, regData); +#ifdef DOLTLITE_PROLLY + if( (idxInsFlags & OPFLAG_NCHANGE)!=0 && VisibleRowid(pDest) + && !pParse->nested ){ + sqlite3VdbeAppendP4(v, pDest, P4_TABLE); + } +#endif sqlite3VdbeChangeP5(v, idxInsFlags|OPFLAG_APPEND); sqlite3VdbeAddOp2(v, OP_Next, iSrc, addr1+1); VdbeCoverage(v); sqlite3VdbeJumpHere(v, addr1); diff --git a/src/vdbe.c b/src/vdbe.c index 5e2baf6276..3dd79b9b83 100644 --- a/src/vdbe.c +++ b/src/vdbe.c @@ -6327,6 +6327,9 @@ case OP_Delete: { const char *zDb; Table *pTab; int opflags; +#ifdef DOLTLITE_PROLLY + i64 iHookRowid = 0; +#endif opflags = pOp->p2; assert( pOp->p1>=0 && pOp->p1nCursor ); @@ -6364,6 +6367,11 @@ case OP_Delete: { if( (pOp->p5 & OPFLAG_SAVEPOSITION)!=0 && pC->isTable ){ pC->movetoTarget = sqlite3BtreeIntegerKey(pC->uc.pCursor); } +#ifdef DOLTLITE_PROLLY + if( VisibleRowid(pTab) && !HasRowid(pTab) && db->xUpdateCallback ){ + iHookRowid = sqlite3BtreeSqlRowid(pC->uc.pCursor); + } +#endif }else{ zDb = 0; pTab = 0; @@ -6414,9 +6422,20 @@ case OP_Delete: { /* Invoke the update-hook if required. */ if( opflags & OPFLAG_NCHANGE ){ p->nChange++; - if( db->xUpdateCallback && ALWAYS(pTab!=0) && HasRowid(pTab) ){ + if( db->xUpdateCallback && ALWAYS(pTab!=0) +#ifdef DOLTLITE_PROLLY + && VisibleRowid(pTab) +#else + && HasRowid(pTab) +#endif + ){ +#ifdef DOLTLITE_PROLLY + db->xUpdateCallback(db->pUpdateArg, SQLITE_DELETE, zDb, pTab->zName, + HasRowid(pTab) ? pC->movetoTarget : iHookRowid); +#else db->xUpdateCallback(db->pUpdateArg, SQLITE_DELETE, zDb, pTab->zName, pC->movetoTarget); +#endif assert( pC->iDb>=0 ); } } @@ -7036,8 +7055,16 @@ case OP_IdxInsert: { /* in2 */ if( rc ) goto abort_due_to_error; x.nKey = pIn2->n; x.pKey = pIn2->z; - x.aMem = aMem + pOp->p3; - x.nMem = (u16)pOp->p4.i; +#ifdef DOLTLITE_PROLLY + if( pOp->p4type==P4_TABLE ){ + x.aMem = 0; + x.nMem = 0; + }else +#endif + { + x.aMem = aMem + pOp->p3; + x.nMem = (u16)pOp->p4.i; + } rc = sqlite3BtreeInsert(pC->uc.pCursor, &x, (pOp->p5 & (OPFLAG_APPEND|OPFLAG_SAVEPOSITION|OPFLAG_PREFORMAT)), ((pOp->p5 & OPFLAG_USESEEKRESULT) ? pC->seekResult : 0) @@ -7046,9 +7073,27 @@ case OP_IdxInsert: { /* in2 */ pC->cacheStatus = CACHE_STALE; if( rc) goto abort_due_to_error; #ifdef DOLTLITE_PROLLY - if( (pOp->p5 & OPFLAG_LASTROWID)!=0 && (pIn2->flags & MEM_Blob)!=0 ){ - db->lastRowid = doltliteSyntheticRowidFromRecord( - (const u8*)pIn2->z, pIn2->n, pC->pKeyInfo); + if( (pIn2->flags & MEM_Blob)!=0 ){ + i64 iRowid = 0; + int bRowid = 0; + if( (pOp->p5 & OPFLAG_LASTROWID)!=0 ){ + iRowid = doltliteSyntheticRowidFromRecord( + (const u8*)pIn2->z, pIn2->n, pC->pKeyInfo); + db->lastRowid = iRowid; + bRowid = 1; + } + if( pOp->p4type==P4_TABLE && db->xUpdateCallback!=0 ){ + Table *pTab = pOp->p4.pTab; + if( pTab && pTab->aCol && VisibleRowid(pTab) && pC->iDb>=0 ){ + if( !bRowid ){ + iRowid = doltliteSyntheticRowidFromRecord( + (const u8*)pIn2->z, pIn2->n, pC->pKeyInfo); + } + db->xUpdateCallback(db->pUpdateArg, + (pOp->p5 & OPFLAG_ISUPDATE) ? SQLITE_UPDATE : SQLITE_INSERT, + db->aDb[pC->iDb].zDbSName, pTab->zName, iRowid); + } + } } #endif break; diff --git a/test/doltlite_regression_test_c.c b/test/doltlite_regression_test_c.c index 18495e05b9..69ee234133 100644 --- a/test/doltlite_regression_test_c.c +++ b/test/doltlite_regression_test_c.c @@ -13234,6 +13234,87 @@ static void run_reset_database_current_branch(void){ removeDbFiles(dbpath); } +typedef struct UpdateHookLog UpdateHookLog; +struct UpdateHookLog { + char z[512]; +}; + +static void clusteredUpdateHook( + void *p, + int op, + const char *zDb, + const char *zTbl, + sqlite3_int64 rowid +){ + UpdateHookLog *pLog = (UpdateHookLog*)p; + char zOp; + char zOne[96]; + (void)zDb; + if( op==SQLITE_INSERT ) zOp = 'I'; + else if( op==SQLITE_UPDATE ) zOp = 'U'; + else if( op==SQLITE_DELETE ) zOp = 'D'; + else zOp = '?'; + snprintf(zOne, sizeof(zOne), "%s%c:%s:%lld", + pLog->z[0] ? "," : "", zOp, zTbl ? zTbl : "", (long long)rowid); + if( strlen(pLog->z)+strlen(zOne)+1 < sizeof(pLog->z) ){ + strcat(pLog->z, zOne); + } +} + +static void run_clustered_pk_update_hook(void){ + char dbpath[512]; + sqlite3 *db = 0; + UpdateHookLog log; + sqlite3_int64 rA, rB; + + printf("=== Clustered PK Update Hook Test ===\n\n"); + make_dbpath(dbpath, sizeof(dbpath), "test_clustered_pk_update_hook"); + removeDbFiles(dbpath); + check("cpk_hook_open", open_db(dbpath, &db)==SQLITE_OK); + if( !db ) return; + + memset(&log, 0, sizeof(log)); + sqlite3_update_hook(db, clusteredUpdateHook, &log); + check("cpk_hook_intpk_dml", execSql(db, + "CREATE TABLE ipk(a INTEGER PRIMARY KEY, v INT);" + "INSERT INTO ipk VALUES(1,1);" + "UPDATE ipk SET v=2 WHERE a=1;" + "DELETE FROM ipk WHERE a=1;")==SQLITE_OK); + check("cpk_hook_intpk_events", strcmp(log.z, "I:ipk:1,U:ipk:1,D:ipk:1")==0); + + memset(&log, 0, sizeof(log)); + check("cpk_hook_textpk_create", execSql(db, + "CREATE TABLE t(k TEXT PRIMARY KEY, v INT);")==SQLITE_OK); + check("cpk_hook_textpk_insert_a", + execSql(db, "INSERT INTO t VALUES('a',1);")==SQLITE_OK); + rA = queryInt64(db, "SELECT rowid FROM t WHERE k='a'"); + check("cpk_hook_textpk_insert_b", + execSql(db, "INSERT INTO t VALUES('b',2);")==SQLITE_OK); + rB = queryInt64(db, "SELECT rowid FROM t WHERE k='b'"); + check("cpk_hook_textpk_update", + execSql(db, "UPDATE t SET v=3 WHERE k='a';")==SQLITE_OK); + check("cpk_hook_textpk_delete", + execSql(db, "DELETE FROM t WHERE k='b';")==SQLITE_OK); + { + char zWant[160]; + snprintf(zWant, sizeof(zWant), "I:t:%lld,I:t:%lld,U:t:%lld,D:t:%lld", + (long long)rA, (long long)rB, (long long)rA, (long long)rB); + check("cpk_hook_textpk_rowids_nonzero", rA!=0 && rB!=0 && rA!=rB); + check("cpk_hook_textpk_events", strcmp(log.z, zWant)==0); + } + + memset(&log, 0, sizeof(log)); + check("cpk_hook_without_rowid_dml", execSql(db, + "CREATE TABLE w(k TEXT PRIMARY KEY, v INT) WITHOUT ROWID;" + "INSERT INTO w VALUES('a',1),('b',2);" + "UPDATE w SET v=3 WHERE k='a';" + "DELETE FROM w WHERE k='b';")==SQLITE_OK); + check("cpk_hook_without_rowid_silent", log.z[0]==0); + + sqlite3_close(db); + removeDbFiles(dbpath); +} + static const RegressionCase aCases[] = { { "refs_vtab_snapshot_stability", "Refs Vtab Snapshot Stability Test", run_refs_vtab_snapshot_stability }, { "storage_format_v12", "Storage Format Version 12 Test", run_storage_format_v12 }, @@ -13430,7 +13511,8 @@ static const RegressionCase aCases[] = { { "intpk_scan_delete_keeps_scan", "INT PK Scan Delete Keeps Scan Test", run_intpk_scan_delete_keeps_scan }, { "count_flush_keeps_scan", "Count Flush Keeps Scan Test", run_count_flush_keeps_scan }, { "negzero_sortkey_eq", "Negzero Sortkey Eq Test", run_negzero_sortkey_eq }, - { "reset_database_current_branch", "Reset Database Current Branch Test", run_reset_database_current_branch } + { "reset_database_current_branch", "Reset Database Current Branch Test", run_reset_database_current_branch }, + { "clustered_pk_update_hook", "Clustered PK Update Hook Test", run_clustered_pk_update_hook } }; static int run_case_by_name(const char *zName){ From 6694f8d7f03a23ca28dd6043d12473b8d504b0bc Mon Sep 17 00:00:00 2001 From: Tim Date: Wed, 26 Aug 2026 07:40:40 -0400 Subject: [PATCH 2/2] Keep unpacked keys on clustered OP_IdxInsert P4_TABLE on the PK IdxInsert stole the unpacked nMem count. Zeroing x.nMem made WITHOUT ROWID CASCADE UPDATE compare the full packed record, miss the existing PK, and insert duplicate child rows (fkey8-7.4). Restore nMem from KeyInfo.nKeyField when P3 names the unpacked key. Co-Authored-By: Grok 4.6 --- src/vdbe.c | 9 ++++++-- test/doltlite_regression_test_c.c | 38 +++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/src/vdbe.c b/src/vdbe.c index 3dd79b9b83..6eec1b491f 100644 --- a/src/vdbe.c +++ b/src/vdbe.c @@ -7057,8 +7057,13 @@ case OP_IdxInsert: { /* in2 */ x.pKey = pIn2->z; #ifdef DOLTLITE_PROLLY if( pOp->p4type==P4_TABLE ){ - x.aMem = 0; - x.nMem = 0; + if( pOp->p3>0 && pC->pKeyInfo!=0 ){ + x.aMem = aMem + pOp->p3; + x.nMem = pC->pKeyInfo->nKeyField; + }else{ + x.aMem = 0; + x.nMem = 0; + } }else #endif { diff --git a/test/doltlite_regression_test_c.c b/test/doltlite_regression_test_c.c index 69ee234133..35918c93fa 100644 --- a/test/doltlite_regression_test_c.c +++ b/test/doltlite_regression_test_c.c @@ -13293,6 +13293,8 @@ static void run_clustered_pk_update_hook(void){ rB = queryInt64(db, "SELECT rowid FROM t WHERE k='b'"); check("cpk_hook_textpk_update", execSql(db, "UPDATE t SET v=3 WHERE k='a';")==SQLITE_OK); + check("cpk_hook_textpk_update_unique", + queryInt64(db, "SELECT count(*) FROM t WHERE k='a'")==1); check("cpk_hook_textpk_delete", execSql(db, "DELETE FROM t WHERE k='b';")==SQLITE_OK); { @@ -13311,6 +13313,42 @@ static void run_clustered_pk_update_hook(void){ "DELETE FROM w WHERE k='b';")==SQLITE_OK); check("cpk_hook_without_rowid_silent", log.z[0]==0); + check("cpk_hook_cascade_schema", execSql(db, + "PRAGMA foreign_keys=ON;" + "CREATE TABLE p1(pid PRIMARY KEY);" + "CREATE TABLE c1(cid PRIMARY KEY," + " pid REFERENCES p1(pid) ON UPDATE CASCADE);" + "INSERT INTO p1 VALUES(10),(20);" + "INSERT INTO c1 VALUES(11,10),(12,10),(21,20),(22,20);" + "UPDATE p1 SET pid = pid * 10;")==SQLITE_OK); + check("cpk_hook_cascade_child_count", + queryInt64(db, "SELECT count(*) FROM c1")==4); + check("cpk_hook_cascade_child_rows", + strcmp(queryScalarText(db, + "SELECT group_concat(cid||' '||pid, ' ') " + "FROM (SELECT cid, pid FROM c1 ORDER BY cid)"), + "11 100 12 100 21 200 22 200")==0); + check("cpk_hook_cascade_parent_rows", + strcmp(queryScalarText(db, + "SELECT group_concat(pid, ' ') FROM (SELECT pid FROM p1 ORDER BY pid)"), + "100 200")==0); + + check("cpk_hook_cascade_attach", execSql(db, + "ATTACH ':memory:' AS aux;" + "CREATE TABLE aux.p1(pid PRIMARY KEY);" + "CREATE TABLE aux.c1(cid PRIMARY KEY," + " pid REFERENCES p1(pid) ON UPDATE CASCADE);" + "INSERT INTO aux.p1 VALUES(10),(20);" + "INSERT INTO aux.c1 VALUES(11,10),(12,10),(21,20),(22,20);" + "UPDATE aux.p1 SET pid = pid * 10;")==SQLITE_OK); + check("cpk_hook_cascade_attach_count", + queryInt64(db, "SELECT count(*) FROM aux.c1")==4); + check("cpk_hook_cascade_attach_rows", + strcmp(queryScalarText(db, + "SELECT group_concat(cid||' '||pid, ' ') " + "FROM (SELECT cid, pid FROM aux.c1 ORDER BY cid)"), + "11 100 12 100 21 200 22 200")==0); + sqlite3_close(db); removeDbFiles(dbpath); }