Skip to content

Commit ce402d5

Browse files
committed
Patch Doltlite mergeScan to skip tree entries covered by a mutmap DELETE
When prollyBtCursorIndexMoveto lands a cursor directly at a mutmap INSERT entry via setCursorToMutMapEntryPhys, mmIdx can jump past sort-earlier DELETE entries. The subsequent scan's mergeScan emits those tree entries as live because the "cmp*dir < 0" branch doesn't consult the mutmap for the current tree key. SQLite then follows the emitted rowid into TableMoveto and trips "database disk image is malformed" in sqlite3VdbeFinishMoveto. Fix reproduced at the CLI level: inside BEGIN IMMEDIATE on a composite-PK table, the sequence INSERT + DELETE + UPDATE-with-prefix-where produces the malformed-disk-image error without the patch and returns the expected single updated row with it. Drop the diagnostic step; verify each patch's effect in-line instead.
1 parent 1ca8176 commit ce402d5

2 files changed

Lines changed: 85 additions & 37 deletions

File tree

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
Check mutmap for tree-key DELETE when emitting a tree entry in mergeScan
2+
3+
prollyBtCursorIndexMoveto can position the merge cursor directly at a
4+
mutmap INSERT entry via setCursorToMutMapEntryPhys, which jumps mmIdx
5+
past any sort-earlier mutmap entries. The subsequent mergeScan
6+
iteration, on seeing `cmp*dir < 0` (tree entry ahead of mutmap[mmIdx]),
7+
emits the tree entry without noticing that the mutmap contains a
8+
DELETE for that tree key at an earlier order index. The tree key was
9+
logically deleted but stays visible to the scan — SQLite then follows
10+
its rowid and calls TableMoveto to a rowid that mutmap says is
11+
deleted, which hits the "database disk image is malformed" check in
12+
sqlite3VdbeFinishMoveto.
13+
14+
Fix: when about to emit a tree entry, look up the current tree key in
15+
the mutmap; if a DELETE record exists, skip the tree entry and advance
16+
the tree cursor. No behavior change in the common case (no matching
17+
mutmap entry → emit tree as before), constant-factor overhead
18+
otherwise.
19+
20+
Reproduces deterministically on:
21+
22+
CREATE TABLE c (ka TEXT, kb TEXT, kc TEXT, v TEXT,
23+
PRIMARY KEY (ka, kb, kc));
24+
INSERT INTO c VALUES ('db','t','a','');
25+
BEGIN IMMEDIATE;
26+
INSERT INTO c VALUES ('db','t','b','');
27+
DELETE FROM c WHERE ka='db' AND kb='t' AND kc='a';
28+
UPDATE c SET v = 'x' WHERE ka='db' AND kb='t'; -- malformed w/o fix
29+
30+
--- a/src/prolly_btree.c
31+
+++ b/src/prolly_btree.c
32+
@@ -3631,6 +3631,29 @@
33+
}
34+
cmp = mergeCompare(pCur, e);
35+
if( cmp*dir < 0 ){
36+
+ /* Tree entry is ahead of mutmap[mmIdx] in scan direction.
37+
+ ** Check whether the mutmap has a DELETE entry for the tree
38+
+ ** key at an order index the iteration has already walked
39+
+ ** past (e.g. after setCursorToMutMapEntryPhys jumped mmIdx
40+
+ ** directly to a later INSERT). Without this check the scan
41+
+ ** would emit a logically-deleted tree row and SQLite would
42+
+ ** later TableMoveto a rowid that mutmap says is gone. */
43+
+ ProllyMutMapEntry *delE = 0;
44+
+ int delRc;
45+
+ if( pCur->curIntKey ){
46+
+ delRc = prollyMutMapFindRc(pCur->pMutMap, 0, 0,
47+
+ prollyCursorIntKey(&pCur->pCur), &delE);
48+
+ }else{
49+
+ const u8 *pK; int nK;
50+
+ prollyCursorKey(&pCur->pCur, &pK, &nK);
51+
+ delRc = prollyMutMapFindRc(pCur->pMutMap, pK, nK, 0, &delE);
52+
+ }
53+
+ if( delRc!=SQLITE_OK ) return delRc;
54+
+ if( delE && delE->op==PROLLY_EDIT_DELETE ){
55+
+ int advRc = advanceTreeCursor(pCur, dir);
56+
+ if( advRc!=SQLITE_OK ) return advRc;
57+
+ continue;
58+
+ }
59+
pCur->mergeSrc = MERGE_SRC_TREE;
60+
if( pRes ) *pRes = 0;
61+
return SQLITE_OK;

.github/workflows/phpunit-tests-doltlite.yml

Lines changed: 24 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,12 @@ jobs:
6565
patch -d doltlite-src -p1 --no-backup-if-mismatch \
6666
< .github/doltlite-patches/eqSeen-preservation.patch
6767
68+
# --- Patch 3: check mutmap for tree-key DELETE in mergeScan ---
69+
patch -d doltlite-src -p1 --no-backup-if-mismatch \
70+
< .github/doltlite-patches/mergeScan-check-tree-delete.patch
71+
6872
echo "--- Patched lines ---"
69-
grep -n 'patched off in CI\|savedEqSeen' "$BUILD" "$MOVETO"
73+
grep -n 'patched off in CI\|savedEqSeen\|walked past' "$BUILD" "$MOVETO"
7074
7175
- name: Build Doltlite shared library
7276
run: |
@@ -155,6 +159,25 @@ jobs:
155159
echo "::error::eqSeen-preservation patch did not take effect: expected only 'id', got '${REMAINING}'"
156160
exit 1
157161
fi
162+
# 3) mergeScan patch: UPDATE inside a transaction after an
163+
# INSERT + DELETE no longer trips "database disk image is
164+
# malformed".
165+
VAL=$(php -r '
166+
$db = new PDO("sqlite::memory:");
167+
$db->exec("CREATE TABLE c (ka TEXT, kb TEXT, kc TEXT, v TEXT, PRIMARY KEY (ka, kb, kc))");
168+
$db->exec("INSERT INTO c VALUES (\"db\", \"t\", \"a\", \"\")");
169+
$db->exec("BEGIN IMMEDIATE");
170+
$db->exec("INSERT INTO c VALUES (\"db\", \"t\", \"b\", \"\")");
171+
$db->exec("DELETE FROM c WHERE ka = \"db\" AND kb = \"t\" AND kc = \"a\"");
172+
$db->exec("UPDATE c SET v = \"x\" WHERE ka = \"db\" AND kb = \"t\"");
173+
$db->exec("COMMIT");
174+
echo $db->query("SELECT v FROM c")->fetch()[0];
175+
' 2>&1)
176+
echo "post-UPDATE value = ${VAL}"
177+
if [ "${VAL}" != "x" ]; then
178+
echo "::error::mergeScan patch did not take effect: expected 'x', got '${VAL}'"
179+
exit 1
180+
fi
158181
159182
- name: Install Composer dependencies (root)
160183
uses: ramsey/composer-install@v3
@@ -169,42 +192,6 @@ jobs:
169192
ignore-cache: "yes"
170193
composer-options: "--optimize-autoloader"
171194

172-
- name: Diagnose testAlterTableAddAndDropColumns
173-
run: |
174-
php -r '
175-
require __DIR__ . "/packages/mysql-on-sqlite/tests/wp-sqlite-schema.php";
176-
require __DIR__ . "/packages/mysql-on-sqlite/src/load.php";
177-
$GLOBALS["table_prefix"] = "wptests_";
178-
$GLOBALS["wpdb"] = new class() { public function set_prefix(string $p): void {} };
179-
if (!function_exists("do_action")) { function do_action() {} }
180-
if (!function_exists("apply_filters")) { function apply_filters($t, $v, ...$a) { return $v; } }
181-
define("FQDB", ":memory:");
182-
define("FQDBDIR", __DIR__);
183-
184-
$pdo_class = PHP_VERSION_ID >= 80400 ? PDO\SQLite::class : PDO::class;
185-
$pdo = new $pdo_class("sqlite::memory:");
186-
$conn = new WP_SQLite_Connection(["pdo" => $pdo]);
187-
$driver = new WP_SQLite_Driver($conn, "wp");
188-
// The driver creates its own internal connection; attach the
189-
// trace logger to THAT connection so we see every query.
190-
$driver->get_connection()->set_query_logger(function (string $sql, array $params) {
191-
$short = substr(str_replace("\n", " ", preg_replace("/\s+/", " ", trim($sql))), 0, 250);
192-
fwrite(STDOUT, sprintf("[SQL] %s\n", $short));
193-
});
194-
195-
$driver->query("CREATE TABLE t (a INT)");
196-
echo "== running ALTER TABLE t ADD b INT, DROP a ==\n";
197-
try {
198-
$driver->query("ALTER TABLE t ADD b INT, DROP a");
199-
echo "ALTER succeeded\n";
200-
} catch (Throwable $e) {
201-
echo "Exception: " . $e->getMessage() . "\n";
202-
if ($e->getPrevious()) echo "Cause: " . $e->getPrevious()->getMessage() . "\n";
203-
echo "At: " . $e->getFile() . ":" . $e->getLine() . "\n";
204-
}
205-
'
206-
continue-on-error: true
207-
208195
- name: Run PHPUnit tests against Doltlite
209196
run: php ./vendor/bin/phpunit -c ./phpunit.xml.dist
210197
working-directory: packages/mysql-on-sqlite

0 commit comments

Comments
 (0)