Skip to content

Commit b83c10c

Browse files
timsehntimsehn
authored
Merge pull request #982 from dolthub/macneale4-claude/dolt-revert
doltlite revert to better match git
2 parents 9cd5f1b + 3c15bad commit b83c10c

4 files changed

Lines changed: 315 additions & 22 deletions

File tree

‎src/doltlite.c‎

Lines changed: 192 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3429,6 +3429,162 @@ static void doltliteCherryPickFunc(
34293429
}
34303430
}
34313431

3432+
static int doltliteTableEntryDiffers(
3433+
const struct TableEntry *a, const struct TableEntry *b
3434+
){
3435+
if( !a && !b ) return 0;
3436+
if( !a || !b ) return 1;
3437+
if( prollyHashCompare(&a->root, &b->root)!=0 ) return 1;
3438+
if( prollyHashCompare(&a->schemaHash, &b->schemaHash)!=0 ) return 1;
3439+
return 0;
3440+
}
3441+
3442+
static int doltliteAppendTableName(
3443+
char ***pazNames, int *pn, int *pnAlloc, const char *zName
3444+
){
3445+
int i;
3446+
if( !zName ) return SQLITE_OK;
3447+
for(i=0; i<*pn; i++){
3448+
if( strcmp((*pazNames)[i], zName)==0 ) return SQLITE_OK;
3449+
}
3450+
if( *pn == *pnAlloc ){
3451+
int n = *pnAlloc ? *pnAlloc*2 : 8;
3452+
char **a = sqlite3_realloc(*pazNames, n*sizeof(char*));
3453+
if( !a ) return SQLITE_NOMEM;
3454+
*pazNames = a;
3455+
*pnAlloc = n;
3456+
}
3457+
(*pazNames)[*pn] = sqlite3_mprintf("%s", zName);
3458+
if( !(*pazNames)[*pn] ) return SQLITE_NOMEM;
3459+
(*pn)++;
3460+
return SQLITE_OK;
3461+
}
3462+
3463+
static void doltliteFreeNameList(char **az, int n){
3464+
int i;
3465+
if( !az ) return;
3466+
for(i=0; i<n; i++) sqlite3_free(az[i]);
3467+
sqlite3_free(az);
3468+
}
3469+
3470+
static int doltliteCollectChangedNames(
3471+
struct TableEntry *aFrom, int nFrom,
3472+
struct TableEntry *aTo, int nTo,
3473+
char ***pazNames, int *pn, int *pnAlloc
3474+
){
3475+
int i, rc;
3476+
for(i=0; i<nFrom; i++){
3477+
struct TableEntry *p = doltliteFindTableByName(aTo, nTo, aFrom[i].zName);
3478+
if( doltliteTableEntryDiffers(&aFrom[i], p) ){
3479+
rc = doltliteAppendTableName(pazNames, pn, pnAlloc, aFrom[i].zName);
3480+
if( rc!=SQLITE_OK ) return rc;
3481+
}
3482+
}
3483+
for(i=0; i<nTo; i++){
3484+
struct TableEntry *p = doltliteFindTableByName(aFrom, nFrom, aTo[i].zName);
3485+
if( !p ){
3486+
rc = doltliteAppendTableName(pazNames, pn, pnAlloc, aTo[i].zName);
3487+
if( rc!=SQLITE_OK ) return rc;
3488+
}
3489+
}
3490+
return SQLITE_OK;
3491+
}
3492+
3493+
static int doltliteNameInList(char **az, int n, const char *zName){
3494+
int i;
3495+
if( !zName ) return 0;
3496+
for(i=0; i<n; i++){
3497+
if( strcmp(az[i], zName)==0 ) return 1;
3498+
}
3499+
return 0;
3500+
}
3501+
3502+
// Compares the catalog of |pCommit| against |pParent|, appending names of
3503+
// tables that differ to *pazTouched. These are the tables the revert of pCommit
3504+
// would modify.
3505+
static int doltliteCollectRevertTouchedTables(
3506+
sqlite3 *db,
3507+
const ProllyHash *pCommitCat,
3508+
const ProllyHash *pParentCat,
3509+
char ***pazTouched, int *pnTouched, int *pnAlloc
3510+
){
3511+
struct TableEntry *aCommit = 0, *aParent = 0;
3512+
int nCommit = 0, nParent = 0;
3513+
int rc;
3514+
rc = doltliteLoadCatalog(db, pCommitCat, &aCommit, &nCommit, 0);
3515+
if( rc!=SQLITE_OK ) return rc;
3516+
rc = doltliteLoadCatalog(db, pParentCat, &aParent, &nParent, 0);
3517+
if( rc==SQLITE_OK ){
3518+
rc = doltliteCollectChangedNames(aCommit, nCommit, aParent, nParent,
3519+
pazTouched, pnTouched, pnAlloc);
3520+
}
3521+
doltliteFreeCatalog(aCommit, nCommit);
3522+
doltliteFreeCatalog(aParent, nParent);
3523+
return rc;
3524+
}
3525+
3526+
// Sets *pConflict=1 if the working set has an uncommitted change to a table
3527+
// the revert would modify. Changes to unrelated tables are allowed and become
3528+
// part of the revert commit, matching Dolt.
3529+
static int doltliteRevertHasDirtyTouchedTables(
3530+
sqlite3 *db,
3531+
char **azTouched, int nTouched,
3532+
int *pConflict
3533+
){
3534+
ChunkStore *cs = doltliteGetChunkStore(db);
3535+
ProllyHash headCatHash, workingCatHash;
3536+
u8 *wBuf = 0; int nWBuf = 0;
3537+
struct TableEntry *aHead = 0, *aWorking = 0;
3538+
int nHead = 0, nWorking = 0;
3539+
int i, rc;
3540+
3541+
*pConflict = 0;
3542+
3543+
if( !cs ) return SQLITE_OK;
3544+
if( !doltliteHasUncommittedChanges(db) ) return SQLITE_OK;
3545+
3546+
rc = doltliteGetHeadCatalogHash(db, &headCatHash);
3547+
if( rc!=SQLITE_OK ) return rc;
3548+
3549+
rc = doltliteFlushAndSerializeCatalog(db, &wBuf, &nWBuf);
3550+
if( rc!=SQLITE_OK ) return rc;
3551+
rc = chunkStorePut(cs, wBuf, nWBuf, &workingCatHash);
3552+
sqlite3_free(wBuf);
3553+
if( rc!=SQLITE_OK ) return rc;
3554+
3555+
rc = doltliteLoadCatalog(db, &workingCatHash, &aWorking, &nWorking, 0);
3556+
if( rc!=SQLITE_OK ) return rc;
3557+
3558+
if( !prollyHashIsEmpty(&headCatHash) ){
3559+
rc = doltliteLoadCatalog(db, &headCatHash, &aHead, &nHead, 0);
3560+
if( rc!=SQLITE_OK ) goto cleanup;
3561+
}
3562+
3563+
for(i=0; i<nWorking; i++){
3564+
struct TableEntry *pH = doltliteFindTableByName(aHead, nHead,
3565+
aWorking[i].zName);
3566+
if( !doltliteTableEntryDiffers(pH, &aWorking[i]) ) continue;
3567+
3568+
if( doltliteNameInList(azTouched, nTouched, aWorking[i].zName) ){
3569+
*pConflict = 1;
3570+
goto cleanup;
3571+
}
3572+
}
3573+
3574+
for(i=0; i<nHead; i++){
3575+
if( doltliteFindTableByName(aWorking, nWorking, aHead[i].zName) ) continue;
3576+
if( doltliteNameInList(azTouched, nTouched, aHead[i].zName) ){
3577+
*pConflict = 1;
3578+
goto cleanup;
3579+
}
3580+
}
3581+
3582+
cleanup:
3583+
doltliteFreeCatalog(aHead, nHead);
3584+
doltliteFreeCatalog(aWorking, nWorking);
3585+
return rc;
3586+
}
3587+
34323588
static void doltliteRevertFunc(
34333589
sqlite3_context *context,
34343590
int argc,
@@ -3438,10 +3594,15 @@ static void doltliteRevertFunc(
34383594
ChunkStore *cs = doltliteGetChunkStore(db);
34393595
const char *zRef;
34403596
ProllyHash revertHash, ourHead;
3597+
ProllyHash liveOurCatalog;
34413598
DoltliteCommit revertCommit, parentCommit, ourCommit;
34423599
int nConflicts = 0;
34433600
int rc;
34443601
char hexBuf[PROLLY_HASH_SIZE*2+1];
3602+
char **azTouched = 0;
3603+
int nTouched = 0;
3604+
int nTouchedAlloc = 0;
3605+
int conflictWithDirty = 0;
34453606

34463607
memset(&revertCommit, 0, sizeof(revertCommit));
34473608
memset(&parentCommit, 0, sizeof(parentCommit));
@@ -3471,12 +3632,6 @@ static void doltliteRevertFunc(
34713632
return;
34723633
}
34733634

3474-
if( doltliteHasUncommittedChanges(db) ){
3475-
sqlite3_result_error(context,
3476-
"cannot revert with uncommitted changes", -1);
3477-
return;
3478-
}
3479-
34803635
rc = doltliteResolveRef(db,zRef, &revertHash);
34813636
if( rc!=SQLITE_OK ){
34823637
sqlite3_result_error(context, "invalid commit hash", -1);
@@ -3510,19 +3665,41 @@ static void doltliteRevertFunc(
35103665
return;
35113666
}
35123667

3668+
rc = doltliteCollectRevertTouchedTables(db, &revertCommit.catalogHash,
3669+
&parentCommit.catalogHash, &azTouched, &nTouched, &nTouchedAlloc);
3670+
if( rc!=SQLITE_OK ) goto revert_error;
3671+
3672+
rc = doltliteRevertHasDirtyTouchedTables(db, azTouched, nTouched,
3673+
&conflictWithDirty);
3674+
if( rc!=SQLITE_OK ) goto revert_error;
3675+
if( conflictWithDirty ){
3676+
doltliteCommitClear(&revertCommit);
3677+
doltliteCommitClear(&parentCommit);
3678+
doltliteCommitClear(&ourCommit);
3679+
doltliteFreeNameList(azTouched, nTouched);
3680+
sqlite3_result_error(context,
3681+
"cannot revert with uncommitted changes", -1);
3682+
return;
3683+
}
3684+
rc = doltliteFlushCatalogToHash(db, &liveOurCatalog);
3685+
if( rc!=SQLITE_OK ) goto revert_error;
3686+
35133687
{
35143688
char msg[512];
35153689
sqlite3_snprintf(sizeof(msg), msg, "Revert \"%s\"",
35163690
revertCommit.zMessage ? revertCommit.zMessage : zRef);
35173691

35183692
rc = applyMergedCatalogAndCommit(db, context,
3519-
&revertCommit.catalogHash, &ourCommit.catalogHash,
3693+
&revertCommit.catalogHash, &liveOurCatalog,
35203694
&parentCommit.catalogHash, &ourHead, msg, &nConflicts, hexBuf);
35213695
}
35223696

35233697
doltliteCommitClear(&revertCommit);
35243698
doltliteCommitClear(&parentCommit);
35253699
doltliteCommitClear(&ourCommit);
3700+
doltliteFreeNameList(azTouched, nTouched);
3701+
azTouched = 0;
3702+
nTouched = 0;
35263703

35273704
if( rc==SQLITE_BUSY ){
35283705
sqlite3_result_error(context,
@@ -3540,6 +3717,14 @@ static void doltliteRevertFunc(
35403717
}else if( hexBuf[0] ){
35413718
sqlite3_result_text(context, hexBuf, -1, SQLITE_TRANSIENT);
35423719
}
3720+
return;
3721+
3722+
revert_error:
3723+
doltliteCommitClear(&revertCommit);
3724+
doltliteCommitClear(&parentCommit);
3725+
doltliteCommitClear(&ourCommit);
3726+
doltliteFreeNameList(azTouched, nTouched);
3727+
sqlite3_result_error(context, "revert failed", -1);
35433728
}
35443729

35453730
static int doltliteRebaseCollectReplaySet(

‎test/doltlite_revert_dirty.sh‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
#!/bin/bash
2+
DOLTLITE=./doltlite
3+
PASS=0; FAIL=0; ERRORS=""
4+
5+
run_test() {
6+
local n="$1" s="$2" e="$3" d="$4"
7+
local r=$(echo "$s" | perl -e 'alarm(10);exec @ARGV' $DOLTLITE "$d" 2>&1)
8+
if [ "$r" = "$e" ]; then
9+
PASS=$((PASS+1))
10+
else
11+
FAIL=$((FAIL+1))
12+
ERRORS="$ERRORS\nFAIL: $n\n expected: $e\n got: $r"
13+
fi
14+
}
15+
16+
run_test_match() {
17+
local n="$1" s="$2" p="$3" d="$4"
18+
local r=$(echo "$s" | perl -e 'alarm(10);exec @ARGV' $DOLTLITE "$d" 2>&1)
19+
if echo "$r" | grep -qE "$p"; then
20+
PASS=$((PASS+1))
21+
else
22+
FAIL=$((FAIL+1))
23+
ERRORS="$ERRORS\nFAIL: $n\n pattern: $p\n got: $r"
24+
fi
25+
}
26+
27+
db_rm() { rm -f "$1" "${1}-wal"; }
28+
29+
echo "=== dolt_revert dirty-set behavior ==="
30+
echo ""
31+
32+
# Case 1: dirty unstaged change to an UNRELATED table -> revert SUCCEEDS,
33+
# dirty change is included in the revert commit, leaving a clean worktree.
34+
35+
DB=/tmp/test_rv_dirty_unrelated_$$.db; db_rm "$DB"
36+
echo "CREATE TABLE t(id INTEGER PRIMARY KEY, x TEXT);
37+
CREATE TABLE meta(id INTEGER PRIMARY KEY, note TEXT);
38+
SELECT dolt_commit('-Am','schema');
39+
INSERT INTO t VALUES(1,'a');
40+
SELECT dolt_commit('-Am','add row');
41+
INSERT INTO meta VALUES(1,'side');" | $DOLTLITE "$DB" > /dev/null 2>&1
42+
43+
run_test_match "rv_dirty_unrelated_hash" \
44+
"SELECT dolt_revert((SELECT commit_hash FROM dolt_log LIMIT 1));" \
45+
"^[0-9a-f]{40}$" "$DB"
46+
run_test "rv_dirty_unrelated_t_reverted" "SELECT count(*) FROM t;" "0" "$DB"
47+
run_test "rv_dirty_unrelated_meta_kept" "SELECT note FROM meta WHERE id=1;" "side" "$DB"
48+
run_test "rv_dirty_unrelated_status_clean" \
49+
"SELECT count(*) FROM dolt_status WHERE table_name='meta';" \
50+
"0" "$DB"
51+
run_test "rv_dirty_unrelated_log_has_revert" \
52+
"SELECT count(*) FROM dolt_log WHERE message LIKE 'Revert%';" \
53+
"1" "$DB"
54+
55+
db_rm "$DB"
56+
57+
# Case 2: dirty unstaged change to the SAME table the revert would touch
58+
# -> revert REFUSES, no new commit, dirty change still there.
59+
60+
DB=/tmp/test_rv_dirty_same_table_$$.db; db_rm "$DB"
61+
echo "CREATE TABLE t(id INTEGER PRIMARY KEY, x TEXT);
62+
SELECT dolt_commit('-Am','schema');
63+
INSERT INTO t VALUES(1,'a');
64+
SELECT dolt_commit('-Am','add row');
65+
INSERT INTO t VALUES(99,'side');" | $DOLTLITE "$DB" > /dev/null 2>&1
66+
67+
run_test_match "rv_dirty_same_refuses" \
68+
"SELECT dolt_revert((SELECT commit_hash FROM dolt_log LIMIT 1));" \
69+
"cannot revert with uncommitted changes" "$DB"
70+
run_test "rv_dirty_same_no_new_commit" \
71+
"SELECT count(*) FROM dolt_log WHERE message LIKE 'Revert%';" \
72+
"0" "$DB"
73+
run_test "rv_dirty_same_row_kept" \
74+
"SELECT x FROM t WHERE id=99;" "side" "$DB"
75+
76+
db_rm "$DB"
77+
78+
# Case 3: STAGED change to an unrelated table -> revert SUCCEEDS and includes
79+
# the staged change in the revert commit, matching Dolt.
80+
81+
DB=/tmp/test_rv_staged_unrelated_$$.db; db_rm "$DB"
82+
echo "CREATE TABLE t(id INTEGER PRIMARY KEY, x TEXT);
83+
CREATE TABLE meta(id INTEGER PRIMARY KEY, note TEXT);
84+
SELECT dolt_commit('-Am','schema');
85+
INSERT INTO t VALUES(1,'a');
86+
SELECT dolt_commit('-Am','add row');
87+
INSERT INTO meta VALUES(1,'side');
88+
SELECT dolt_add('meta');" | $DOLTLITE "$DB" > /dev/null 2>&1
89+
90+
run_test_match "rv_staged_unrelated_hash" \
91+
"SELECT dolt_revert((SELECT commit_hash FROM dolt_log LIMIT 1));" \
92+
"^[0-9a-f]{40}$" "$DB"
93+
run_test "rv_staged_unrelated_log_has_revert" \
94+
"SELECT count(*) FROM dolt_log WHERE message LIKE 'Revert%';" \
95+
"1" "$DB"
96+
run_test "rv_staged_unrelated_status_clean" \
97+
"SELECT count(*) FROM dolt_status WHERE table_name='meta';" \
98+
"0" "$DB"
99+
100+
db_rm "$DB"
101+
102+
# Case 4: clean working set -> revert SUCCEEDS (regression guard).
103+
104+
DB=/tmp/test_rv_clean_$$.db; db_rm "$DB"
105+
echo "CREATE TABLE t(id INTEGER PRIMARY KEY, x TEXT);
106+
SELECT dolt_commit('-Am','schema');
107+
INSERT INTO t VALUES(1,'a');
108+
SELECT dolt_commit('-Am','add row');" | $DOLTLITE "$DB" > /dev/null 2>&1
109+
110+
run_test_match "rv_clean_hash" \
111+
"SELECT dolt_revert((SELECT commit_hash FROM dolt_log LIMIT 1));" \
112+
"^[0-9a-f]{40}$" "$DB"
113+
run_test "rv_clean_t_reverted" "SELECT count(*) FROM t;" "0" "$DB"
114+
115+
db_rm "$DB"
116+
117+
echo ""
118+
echo "Results: $PASS passed, $FAIL failed out of $((PASS+FAIL)) tests"
119+
if [ $FAIL -gt 0 ]; then echo -e "$ERRORS"; exit 1; fi

‎test/run_doltlite_tests.sh‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ TESTS=(
4040
doltlite_conflicts.sh
4141
doltlite_conflict_rows.sh
4242
doltlite_cherry_pick.sh
43+
doltlite_revert_dirty.sh
4344

4445
# Virtual tables
4546
doltlite_diff_table.sh

0 commit comments

Comments
 (0)