Skip to content

Commit 2d4dfe0

Browse files
Timclaude
andcommitted
Compose the named-stage master per object instead of adopting working wholesale
A named add previously adopted the whole working master root into the staged catalog, dragging unstaged schema changes into the commit: an unstaged ALTER on another table rode along, and an unstaged DROP INDEX destroyed a staged index. The staged master is now composed row by row: table and index rows of the named objects (adds, staged drops, vtab shadows, retired rename names) come from working, everything else keeps the previously staged rows, and views and triggers always keep staged state. Composed rows cross catalog numbering domains, so old-sourced table rows are renumbered to the working table number their aligned entries carry, and the serializer rejects two cross-domain pairings in constructed arrays: an unnamed entry never pairs with a table row, and a named add only pairs with an unnamed staged entry when the staged schema rows identify that entry as the same table. The index schema row comparison shared by status and diff summary moves to a single helper. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 09a38ec commit 2d4dfe0

6 files changed

Lines changed: 234 additions & 89 deletions

File tree

src/doltlite.c

Lines changed: 101 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -991,6 +991,45 @@ static int addWriteStagedCatalog(
991991
return rc;
992992
}
993993

994+
/* True when the two catalogs' index schema rows for zTable differ: any
995+
** index added, dropped, re-targeted, or redefined. Index changes carry no
996+
** named catalog entry, so every surface that reports per-table change
997+
** (status, the diff summary) attributes them through this comparison. */
998+
int doltliteIndexSchemaRowsDifferForTable(
999+
SchemaEntry *aA, int nA,
1000+
SchemaEntry *aB, int nB,
1001+
const char *zTable
1002+
){
1003+
int i, j, nMatchA = 0, nBForTable = 0;
1004+
for(i=0; i<nA; i++){
1005+
int found = 0;
1006+
if( !aA[i].zType || strcmp(aA[i].zType, "index")!=0
1007+
|| !aA[i].zTblName || strcmp(aA[i].zTblName, zTable)!=0 ){
1008+
continue;
1009+
}
1010+
for(j=0; j<nB; j++){
1011+
if( aB[j].zType && strcmp(aB[j].zType, "index")==0
1012+
&& aB[j].zTblName && strcmp(aB[j].zTblName, zTable)==0
1013+
&& aB[j].zName && aA[i].zName
1014+
&& strcmp(aB[j].zName, aA[i].zName)==0
1015+
&& ((aB[j].zSql==0)==(aA[i].zSql==0))
1016+
&& (aB[j].zSql==0 || strcmp(aB[j].zSql, aA[i].zSql)==0) ){
1017+
found = 1;
1018+
break;
1019+
}
1020+
}
1021+
if( !found ) return 1;
1022+
nMatchA++;
1023+
}
1024+
for(j=0; j<nB; j++){
1025+
if( aB[j].zType && strcmp(aB[j].zType, "index")==0
1026+
&& aB[j].zTblName && strcmp(aB[j].zTblName, zTable)==0 ){
1027+
nBForTable++;
1028+
}
1029+
}
1030+
return nMatchA!=nBForTable;
1031+
}
1032+
9941033
/* True when the final staged entry list contains a table entry named
9951034
** zTbl. Index entries follow their parent table through -a staging, and
9961035
** the parent's presence decides whether an index entry belongs at all. */
@@ -1311,14 +1350,35 @@ static int addStageNamedTables(
13111350
SchemaEntry *aStagedSchema = 0;
13121351
int nWorkSchema = 0;
13131352
int nStagedSchema = 0;
1353+
/* Objects whose master rows follow WORKING in this operation: the named
1354+
** tables (adds and staged drops) plus a staged vtab's shadows. Names are
1355+
** borrowed from the argv values and schema arrays, which outlive use. */
1356+
char **azTouched = 0;
1357+
int nTouched = 0;
13141358

13151359
#define ADDNAMED_FREE_ALL() do { \
1360+
int ft_; \
1361+
for(ft_=0; ft_<nTouched; ft_++) sqlite3_free(azTouched[ft_]); \
1362+
sqlite3_free((void*)azTouched); \
13161363
freeSchemaEntries(aWorkSchema, nWorkSchema); \
13171364
freeSchemaEntries(aStagedSchema, nStagedSchema); \
13181365
doltliteFreeCatalog(aWorking, nWorking); \
13191366
doltliteFreeCatalog(aStaged, nStaged); \
13201367
} while(0)
13211368

1369+
#define ADDNAMED_TOUCH(zN) do { \
1370+
char **azNew = sqlite3_realloc((void*)azTouched, \
1371+
(nTouched+1)*(int)sizeof(char*)); \
1372+
char *zOwn_ = azNew ? sqlite3_mprintf("%s", (zN)) : 0; \
1373+
if( azNew ) azTouched = azNew; \
1374+
if( !azNew || !zOwn_ ){ \
1375+
ADDNAMED_FREE_ALL(); \
1376+
sqlite3_result_error_nomem(context); \
1377+
return SQLITE_NOMEM; \
1378+
} \
1379+
azTouched[nTouched++] = zOwn_; \
1380+
} while(0)
1381+
13221382
rc = addLoadWorkingAndStagedCatalogs(db, pWorkingHash,
13231383
&aWorking, &nWorking,
13241384
&aStaged, &nStaged);
@@ -1369,6 +1429,7 @@ static int addStageNamedTables(
13691429
{
13701430
Table *pLive = sqlite3FindTable(db, zTable, "main");
13711431
if( pLive && IsVirtual(pLive) ){
1432+
int w;
13721433
rc = addStageShadowTablesOf(db, context, &aStaged, &nStaged,
13731434
aWorking, nWorking,
13741435
aWorkSchema, nWorkSchema,
@@ -1377,6 +1438,16 @@ static int addStageNamedTables(
13771438
ADDNAMED_FREE_ALL();
13781439
return rc;
13791440
}
1441+
ADDNAMED_TOUCH(zTable);
1442+
for(w=0; w<nWorkSchema; w++){
1443+
if( aWorkSchema[w].zType
1444+
&& strcmp(aWorkSchema[w].zType, "table")==0
1445+
&& aWorkSchema[w].zName
1446+
&& strcmp(aWorkSchema[w].zName, zTable)!=0
1447+
&& sqlite3IsShadowTableOf(db, pLive, aWorkSchema[w].zName) ){
1448+
ADDNAMED_TOUCH(aWorkSchema[w].zName);
1449+
}
1450+
}
13801451
updateMaster = 1;
13811452
continue;
13821453
}
@@ -1422,6 +1493,7 @@ static int addStageNamedTables(
14221493
addRemoveShadowEntriesOfDroppedVtab(aStaged, &nStaged,
14231494
aStagedSchema, nStagedSchema,
14241495
aWorking, nWorking, zTable);
1496+
ADDNAMED_TOUCH(zTable);
14251497
updateMaster = 1;
14261498
}
14271499
if( !found ){
@@ -1446,21 +1518,44 @@ static int addStageNamedTables(
14461518
int nameMatch = aStaged[k].zName && aWorking[j].zName
14471519
&& strcmp(aStaged[k].zName, aWorking[j].zName)==0;
14481520
int rootMatch = aStaged[k].iTable==iTable;
1449-
int unnamedRootMatch = rootMatch
1450-
&& (!aStaged[k].zName || !aWorking[j].zName);
1521+
/* An unnamed staged entry on a bare number match can be a
1522+
** cross-domain collision with an index entry; pair only when
1523+
** the staged catalog's own schema rows say the entry at this
1524+
** number is this very table. */
1525+
int unnamedRootMatch = 0;
14511526
int renameRootMatch = rootMatch
14521527
&& aStaged[k].zName && aWorking[j].zName
14531528
&& strcmp(aStaged[k].zName, aWorking[j].zName)!=0
14541529
&& !addFindEntryByName(aWorking, nWorking, aStaged[k].zName)
14551530
&& !addFindEntryByName(aStaged, nStaged, aWorking[j].zName);
1531+
if( rootMatch && !aStaged[k].zName && aWorking[j].zName ){
1532+
int r;
1533+
for(r=0; r<nStagedSchema; r++){
1534+
if( aStagedSchema[r].iRootpage==aStaged[k].iTable
1535+
&& aStagedSchema[r].zType
1536+
&& strcmp(aStagedSchema[r].zType, "table")==0
1537+
&& aStagedSchema[r].zName
1538+
&& strcmp(aStagedSchema[r].zName, aWorking[j].zName)==0 ){
1539+
unnamedRootMatch = 1;
1540+
break;
1541+
}
1542+
}
1543+
}
14561544
if( nameMatch || unnamedRootMatch || renameRootMatch ){
14571545
int schemaChanged =
14581546
prollyHashCompare(&aStaged[k].schemaHash, &aWorking[j].schemaHash)!=0;
14591547
int nameChanged =
14601548
(!aStaged[k].zName) != (!aWorking[j].zName)
14611549
|| (aStaged[k].zName && aWorking[j].zName
14621550
&& strcmp(aStaged[k].zName, aWorking[j].zName)!=0);
1463-
char *zDup = aWorking[j].zName
1551+
char *zDup;
1552+
/* A rename retires the old name: its rows (and its indexes'
1553+
** rows, keyed by the old tbl_name) must follow WORKING too,
1554+
** where they no longer exist. */
1555+
if( nameChanged && aStaged[k].zName ){
1556+
ADDNAMED_TOUCH(aStaged[k].zName);
1557+
}
1558+
zDup = aWorking[j].zName
14641559
? sqlite3_mprintf("%s", aWorking[j].zName) : 0;
14651560
if( aWorking[j].zName && !zDup ){
14661561
ADDNAMED_FREE_ALL();
@@ -1504,6 +1599,7 @@ static int addStageNamedTables(
15041599
ADDNAMED_FREE_ALL();
15051600
return rc;
15061601
}
1602+
ADDNAMED_TOUCH(zTable);
15071603
break;
15081604
}
15091605
}
@@ -1523,6 +1619,7 @@ static int addStageNamedTables(
15231619
&pWorkingMaster->root, pWorkingMaster->flags,
15241620
pStagedMaster ? &pStagedMaster->root : 0,
15251621
pStagedMaster ? pStagedMaster->flags : 0,
1622+
(const char**)azTouched, nTouched,
15261623
&composedRoot);
15271624
if( rc!=SQLITE_OK ){
15281625
ADDNAMED_FREE_ALL();
@@ -1549,6 +1646,7 @@ static int addStageNamedTables(
15491646
addAlignStagedEntriesToWorking(aWorking, nWorking, aStaged, nStaged);
15501647
rc = addWriteStagedCatalog(db, cs, aStaged, nStaged);
15511648
ADDNAMED_FREE_ALL();
1649+
#undef ADDNAMED_TOUCH
15521650
#undef ADDNAMED_FREE_ALL
15531651
if( rc!=SQLITE_OK ){
15541652
sqlite3_result_error_code(context, rc);

src/doltlite_diff.c

Lines changed: 2 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -282,45 +282,6 @@ static int batchAppend(DoltliteDiffCursor *pCur,
282282
return SQLITE_OK;
283283
}
284284

285-
/* Index changes carry no named entry of their own: attribute them to the
286-
** parent table by comparing each catalog's index schema rows (name + sql),
287-
** so an index-only commit reports the table with schema_change=1 the way
288-
** Dolt does. */
289-
static int indexRowsDifferForTable(
290-
SchemaEntry *aA, int nA,
291-
SchemaEntry *aB, int nB,
292-
const char *zTable
293-
){
294-
int i, j, nMatchA = 0, nB4T = 0;
295-
for(i=0; i<nA; i++){
296-
int found = 0;
297-
if( !aA[i].zType || strcmp(aA[i].zType, "index")!=0
298-
|| !aA[i].zTblName || strcmp(aA[i].zTblName, zTable)!=0 ){
299-
continue;
300-
}
301-
for(j=0; j<nB; j++){
302-
if( aB[j].zType && strcmp(aB[j].zType, "index")==0
303-
&& aB[j].zTblName && strcmp(aB[j].zTblName, zTable)==0
304-
&& aB[j].zName && aA[i].zName
305-
&& strcmp(aB[j].zName, aA[i].zName)==0
306-
&& ((aB[j].zSql==0)==(aA[i].zSql==0))
307-
&& (aB[j].zSql==0 || strcmp(aB[j].zSql, aA[i].zSql)==0) ){
308-
found = 1;
309-
break;
310-
}
311-
}
312-
if( !found ) return 1;
313-
nMatchA++;
314-
}
315-
for(j=0; j<nB; j++){
316-
if( aB[j].zType && strcmp(aB[j].zType, "index")==0
317-
&& aB[j].zTblName && strcmp(aB[j].zTblName, zTable)==0 ){
318-
nB4T++;
319-
}
320-
}
321-
return nMatchA!=nB4T;
322-
}
323-
324285
static int loadIndexSchemaRows(
325286
sqlite3 *db,
326287
const ProllyHash *pCatHash,
@@ -388,7 +349,7 @@ static int diffFilteredTableRoots(
388349
rc = loadIndexSchemaRows(db, pParentCat, &aParentRows, &nParentRows);
389350
}
390351
if( rc==SQLITE_OK
391-
&& indexRowsDifferForTable(aChildRows, nChildRows,
352+
&& doltliteIndexSchemaRowsDifferForTable(aChildRows, nChildRows,
392353
aParentRows, nParentRows,
393354
pCur->zFilterTable) ){
394355
schemaChange = 1;
@@ -515,7 +476,7 @@ static int diffCatalogPair(
515476
dataChange = (prollyHashCompare(&e->root, &p->root) != 0) ? 1 : 0;
516477
schemaChange = (prollyHashCompare(&e->schemaHash, &p->schemaHash) != 0) ? 1 : 0;
517478
if( !schemaChange
518-
&& indexRowsDifferForTable(aChildRows, nChildRows,
479+
&& doltliteIndexSchemaRowsDifferForTable(aChildRows, nChildRows,
519480
aParentRows, nParentRows, e->zName) ){
520481
schemaChange = 1;
521482
}

src/doltlite_internal.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -922,9 +922,12 @@ int doltliteMergeCatalogs(sqlite3 *db,
922922
int bPreferOurMaster,
923923
char ***pazReindex, int *pnReindex);
924924
void doltliteFreeNameList(char **az, int n);
925+
int doltliteIndexSchemaRowsDifferForTable(SchemaEntry *aA, int nA,
926+
SchemaEntry *aB, int nB, const char *zTable);
925927
int doltliteBuildNamedStageMasterRoot(sqlite3 *db,
926928
const ProllyHash *pWorkingMaster, u8 workingFlags,
927929
const ProllyHash *pOldMaster, u8 oldFlags,
930+
const char **azTouched, int nTouched,
928931
ProllyHash *pNewRoot);
929932
int doltliteReindexNamedIndexes(sqlite3 *db, char **az, int n);
930933

src/doltlite_status.c

Lines changed: 22 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -361,15 +361,6 @@ static int statusRowExists(
361361
return 0;
362362
}
363363

364-
static int statusSchemaSqlChanged(const SchemaEntry *pA, const SchemaEntry *pB){
365-
const char *zA;
366-
const char *zB;
367-
if( !pA || !pB ) return 1;
368-
zA = pA->zSql ? pA->zSql : "";
369-
zB = pB->zSql ? pB->zSql : "";
370-
return strcmp(zA, zB)!=0;
371-
}
372-
373364
static int statusMaybeAddParentSchemaChange(
374365
DoltliteStatusCursor *pCur,
375366
const char *zParent,
@@ -396,7 +387,7 @@ static int statusCompareIndexSchemaObjects(
396387
SchemaEntry *aTo = 0;
397388
int nFrom = 0;
398389
int nTo = 0;
399-
int i;
390+
int i, j;
400391
int rc;
401392

402393
if( !cs || !pCache ) return SQLITE_OK;
@@ -405,30 +396,28 @@ static int statusCompareIndexSchemaObjects(
405396
rc = loadSchemaFromCatalog(db, cs, pCache, pToCat, &aTo, &nTo);
406397
if( rc!=SQLITE_OK ) goto index_schema_done;
407398

408-
for(i=0; i<nTo; i++){
409-
SchemaEntry *pOld;
410-
if( !aTo[i].zType || strcmp(aTo[i].zType, "index")!=0 ) continue;
411-
if( !aTo[i].zName || !aTo[i].zTblName ) continue;
412-
pOld = findSchemaEntry(aFrom, nFrom, aTo[i].zName);
413-
if( !pOld
414-
|| !pOld->zType
415-
|| strcmp(pOld->zType, "index")!=0
416-
|| !pOld->zTblName
417-
|| strcmp(pOld->zTblName, aTo[i].zTblName)!=0
418-
|| statusSchemaSqlChanged(pOld, &aTo[i]) ){
419-
rc = statusMaybeAddParentSchemaChange(pCur, aTo[i].zTblName,
420-
staged, zFilter);
421-
if( rc!=SQLITE_OK ) goto index_schema_done;
399+
/* One comparison per distinct parent table across both row sets; the
400+
** shared comparator decides whether that table's index set changed. */
401+
for(i=0; i<nFrom+nTo; i++){
402+
SchemaEntry *pRow = i<nFrom ? &aFrom[i] : &aTo[i-nFrom];
403+
int seen = 0;
404+
if( !pRow->zType || strcmp(pRow->zType, "index")!=0
405+
|| !pRow->zTblName ){
406+
continue;
422407
}
423-
}
424-
425-
for(i=0; i<nFrom; i++){
426-
SchemaEntry *pNew;
427-
if( !aFrom[i].zType || strcmp(aFrom[i].zType, "index")!=0 ) continue;
428-
if( !aFrom[i].zName || !aFrom[i].zTblName ) continue;
429-
pNew = findSchemaEntry(aTo, nTo, aFrom[i].zName);
430-
if( !pNew || !pNew->zType || strcmp(pNew->zType, "index")!=0 ){
431-
rc = statusMaybeAddParentSchemaChange(pCur, aFrom[i].zTblName,
408+
for(j=0; j<i; j++){
409+
SchemaEntry *pPrev = j<nFrom ? &aFrom[j] : &aTo[j-nFrom];
410+
if( pPrev->zType && strcmp(pPrev->zType, "index")==0
411+
&& pPrev->zTblName
412+
&& strcmp(pPrev->zTblName, pRow->zTblName)==0 ){
413+
seen = 1;
414+
break;
415+
}
416+
}
417+
if( seen ) continue;
418+
if( doltliteIndexSchemaRowsDifferForTable(aFrom, nFrom, aTo, nTo,
419+
pRow->zTblName) ){
420+
rc = statusMaybeAddParentSchemaChange(pCur, pRow->zTblName,
432421
staged, zFilter);
433422
if( rc!=SQLITE_OK ) goto index_schema_done;
434423
}

0 commit comments

Comments
 (0)