mirror of
https://github.com/asg017/sqlite-vec.git
synced 2026-04-25 00:36:56 +02:00
Fix SQLITE_DONE leak in ClearMetadata that broke DELETE on long text metadata (#274)
vec0Update_Delete_ClearMetadata's long-text branch runs a DELETE via sqlite3_step, which returns SQLITE_DONE (101) on success. The code checked for failure but never normalized the success case to SQLITE_OK. The function's epilogue returned SQLITE_DONE as-is, which the caller (vec0Update_Delete) treated as an error, aborting the DELETE scan and silently leaving rows behind. - Normalize rc to SQLITE_OK after successful sqlite3_step in ClearMetadata - Move sqlite3_finalize before the rc check (cleanup on all paths) - Add test_delete_by_metadata_with_long_text regression test - Update test_deletes snapshot (row 3 now correctly deleted) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
dfd8dc5290
commit
4f7a8e410d
3 changed files with 43 additions and 26 deletions
|
|
@ -8944,11 +8944,17 @@ int vec0Update_Delete_ClearMetadata(vec0_vtab *p, int metadata_idx, i64 rowid, i
|
||||||
}
|
}
|
||||||
sqlite3_bind_int64(stmt, 1, rowid);
|
sqlite3_bind_int64(stmt, 1, rowid);
|
||||||
rc = sqlite3_step(stmt);
|
rc = sqlite3_step(stmt);
|
||||||
|
sqlite3_finalize(stmt);
|
||||||
if(rc != SQLITE_DONE) {
|
if(rc != SQLITE_DONE) {
|
||||||
rc = SQLITE_ERROR;
|
rc = SQLITE_ERROR;
|
||||||
goto done;
|
goto done;
|
||||||
}
|
}
|
||||||
sqlite3_finalize(stmt);
|
// Fix for https://github.com/asg017/sqlite-vec/issues/274
|
||||||
|
// sqlite3_step returns SQLITE_DONE (101) on DML success, but the
|
||||||
|
// `done:` epilogue treats anything other than SQLITE_OK as an error.
|
||||||
|
// Without this, SQLITE_DONE propagates up to vec0Update_Delete,
|
||||||
|
// which aborts the DELETE scan and silently drops remaining rows.
|
||||||
|
rc = SQLITE_OK;
|
||||||
}
|
}
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -27,8 +27,8 @@
|
||||||
OrderedDict({
|
OrderedDict({
|
||||||
'chunk_id': 1,
|
'chunk_id': 1,
|
||||||
'size': 8,
|
'size': 8,
|
||||||
'validity': b'\x06',
|
'validity': b'\x02',
|
||||||
'rowids': b'\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x00\x00\x00\x00\x00\x00\x03\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00',
|
'rowids': b'\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00',
|
||||||
}),
|
}),
|
||||||
]),
|
]),
|
||||||
}),
|
}),
|
||||||
|
|
@ -37,7 +37,7 @@
|
||||||
'rows': list([
|
'rows': list([
|
||||||
OrderedDict({
|
OrderedDict({
|
||||||
'rowid': 1,
|
'rowid': 1,
|
||||||
'data': b'\x06',
|
'data': b'\x02',
|
||||||
}),
|
}),
|
||||||
]),
|
]),
|
||||||
}),
|
}),
|
||||||
|
|
@ -46,7 +46,7 @@
|
||||||
'rows': list([
|
'rows': list([
|
||||||
OrderedDict({
|
OrderedDict({
|
||||||
'rowid': 1,
|
'rowid': 1,
|
||||||
'data': b'\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x00\x00\x00\x00\x00\x00\x03\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00',
|
'data': b'\x00\x00\x00\x00\x00\x00\x00\x00\x02\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00',
|
||||||
}),
|
}),
|
||||||
]),
|
]),
|
||||||
}),
|
}),
|
||||||
|
|
@ -55,7 +55,7 @@
|
||||||
'rows': list([
|
'rows': list([
|
||||||
OrderedDict({
|
OrderedDict({
|
||||||
'rowid': 1,
|
'rowid': 1,
|
||||||
'data': b'\x00\x00\x00\x00\x00\x00\x00\x00\x9a\x99\x99\x99\x99\x99\x01@ffffff\n@\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00',
|
'data': b'\x00\x00\x00\x00\x00\x00\x00\x00\x9a\x99\x99\x99\x99\x99\x01@\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00',
|
||||||
}),
|
}),
|
||||||
]),
|
]),
|
||||||
}),
|
}),
|
||||||
|
|
@ -64,17 +64,13 @@
|
||||||
'rows': list([
|
'rows': list([
|
||||||
OrderedDict({
|
OrderedDict({
|
||||||
'rowid': 1,
|
'rowid': 1,
|
||||||
'data': b'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x05\x00\x00\x00test2\x00\x00\x00\x00\x00\x00\x00\r\x00\x00\x00123456789012\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00',
|
'data': b'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x05\x00\x00\x00test2\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00',
|
||||||
}),
|
}),
|
||||||
]),
|
]),
|
||||||
}),
|
}),
|
||||||
'v_metadatatext03': OrderedDict({
|
'v_metadatatext03': OrderedDict({
|
||||||
'sql': 'select * from v_metadatatext03',
|
'sql': 'select * from v_metadatatext03',
|
||||||
'rows': list([
|
'rows': list([
|
||||||
OrderedDict({
|
|
||||||
'rowid': 3,
|
|
||||||
'data': '1234567890123',
|
|
||||||
}),
|
|
||||||
]),
|
]),
|
||||||
}),
|
}),
|
||||||
'v_rowids': OrderedDict({
|
'v_rowids': OrderedDict({
|
||||||
|
|
@ -86,12 +82,6 @@
|
||||||
'chunk_id': 1,
|
'chunk_id': 1,
|
||||||
'chunk_offset': 1,
|
'chunk_offset': 1,
|
||||||
}),
|
}),
|
||||||
OrderedDict({
|
|
||||||
'rowid': 3,
|
|
||||||
'id': None,
|
|
||||||
'chunk_id': 1,
|
|
||||||
'chunk_offset': 2,
|
|
||||||
}),
|
|
||||||
]),
|
]),
|
||||||
}),
|
}),
|
||||||
'v_vector_chunks00': OrderedDict({
|
'v_vector_chunks00': OrderedDict({
|
||||||
|
|
@ -99,7 +89,7 @@
|
||||||
'rows': list([
|
'rows': list([
|
||||||
OrderedDict({
|
OrderedDict({
|
||||||
'rowid': 1,
|
'rowid': 1,
|
||||||
'vectors': b'\x00\x00\x00\x00""""3333\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00',
|
'vectors': b'\x00\x00\x00\x00""""\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00',
|
||||||
}),
|
}),
|
||||||
]),
|
]),
|
||||||
}),
|
}),
|
||||||
|
|
@ -370,14 +360,6 @@
|
||||||
'f': 2.2,
|
'f': 2.2,
|
||||||
't': 'test2',
|
't': 'test2',
|
||||||
}),
|
}),
|
||||||
OrderedDict({
|
|
||||||
'rowid': 3,
|
|
||||||
'vector': b'3333',
|
|
||||||
'b': 1,
|
|
||||||
'n': 3,
|
|
||||||
'f': 3.3,
|
|
||||||
't': '1234567890123',
|
|
||||||
}),
|
|
||||||
]),
|
]),
|
||||||
})
|
})
|
||||||
# ---
|
# ---
|
||||||
|
|
|
||||||
|
|
@ -265,6 +265,35 @@ def test_deletes(db, snapshot):
|
||||||
assert vec0_shadow_table_contents(db, "v") == snapshot()
|
assert vec0_shadow_table_contents(db, "v") == snapshot()
|
||||||
|
|
||||||
|
|
||||||
|
def test_delete_by_metadata_with_long_text(db):
|
||||||
|
"""Regression for https://github.com/asg017/sqlite-vec/issues/274.
|
||||||
|
|
||||||
|
ClearMetadata left rc=SQLITE_DONE after the long-text DELETE, which
|
||||||
|
propagated as an error and silently aborted the DELETE scan.
|
||||||
|
"""
|
||||||
|
db.execute(
|
||||||
|
"create virtual table v using vec0("
|
||||||
|
" tag text, embedding float[4], chunk_size=8"
|
||||||
|
")"
|
||||||
|
)
|
||||||
|
for i in range(6):
|
||||||
|
db.execute(
|
||||||
|
"insert into v(tag, embedding) values (?, zeroblob(16))",
|
||||||
|
[f"long_text_value_{i}"],
|
||||||
|
)
|
||||||
|
for i in range(4):
|
||||||
|
db.execute(
|
||||||
|
"insert into v(tag, embedding) values (?, zeroblob(16))",
|
||||||
|
[f"long_text_value_0"],
|
||||||
|
)
|
||||||
|
assert db.execute("select count(*) from v").fetchone()[0] == 10
|
||||||
|
|
||||||
|
# DELETE by metadata WHERE — the pattern from the issue
|
||||||
|
db.execute("delete from v where tag = 'long_text_value_0'")
|
||||||
|
assert db.execute("select count(*) from v where tag = 'long_text_value_0'").fetchone()[0] == 0
|
||||||
|
assert db.execute("select count(*) from v").fetchone()[0] == 5
|
||||||
|
|
||||||
|
|
||||||
def test_knn(db, snapshot):
|
def test_knn(db, snapshot):
|
||||||
db.execute(
|
db.execute(
|
||||||
"create virtual table v using vec0(vector float[1], name text, chunk_size=8)"
|
"create virtual table v using vec0(vector float[1], name text, chunk_size=8)"
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue