Skip to content

Commit

Permalink
DELETE: Fix panic when using PRIMARY KEY (#203)
Browse files Browse the repository at this point in the history
When an index lookup occurs as part of a DELETE, it was not stripping
the qualified identifiers in the row necessary for storage to find the
rows.

This did not effect the equivilent UPDATE, but I have add an extra
test for that anyway.

Fixed #200
  • Loading branch information
elliotchance authored Dec 27, 2024
1 parent c1def77 commit eaa590c
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 1 deletion.
14 changes: 14 additions & 0 deletions tests/delete.sql
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,17 @@ DELETE FROM foo.bar;
-- msg: CREATE SCHEMA 1
-- msg: CREATE TABLE 1
-- msg: DELETE 0

-- # https://github.com/elliotchance/vsql/issues/200
CREATE TABLE PrimaryProduct (id INT NOT NULL, PRIMARY KEY(id));
INSERT INTO PrimaryProduct (id) VALUES (1);
INSERT INTO PrimaryProduct (id) VALUES (2);
EXPLAIN DELETE FROM PrimaryProduct WHERE id = 1;
DELETE FROM PrimaryProduct WHERE id = 1;
SELECT * FROM PrimaryProduct;
-- msg: CREATE TABLE 1
-- msg: INSERT 1
-- msg: INSERT 1
-- EXPLAIN: PRIMARY KEY ":memory:".PUBLIC.PRIMARYPRODUCT (ID INTEGER NOT NULL) BETWEEN 1 AND 1
-- msg: DELETE 1
-- ID: 2
12 changes: 12 additions & 0 deletions tests/update.sql
Original file line number Diff line number Diff line change
Expand Up @@ -108,3 +108,15 @@ SELECT * FROM foo;
-- msg: INSERT 1
-- error 22001: string data right truncation for CHARACTER VARYING(4)
-- BAZ: abc

-- # https://github.com/elliotchance/vsql/issues/200
CREATE TABLE PrimaryProduct (id INT NOT NULL, PRIMARY KEY(id));
INSERT INTO PrimaryProduct (id) VALUES (1);
EXPLAIN UPDATE PrimaryProduct SET id = 2 WHERE id = 1;
UPDATE PrimaryProduct SET id = 2 WHERE id = 1;
SELECT * FROM PrimaryProduct;
-- msg: CREATE TABLE 1
-- msg: INSERT 1
-- EXPLAIN: PRIMARY KEY ":memory:".PUBLIC.PRIMARYPRODUCT (ID INTEGER NOT NULL) BETWEEN 1 AND 1
-- msg: UPDATE 1
-- ID: 2
4 changes: 3 additions & 1 deletion vsql/std_14_9_delete_statement_searched.v
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ fn (stmt DeleteStatementSearched) execute(mut conn Connection, params map[string
mut rows := plan.execute([]Row{})!

for mut row in rows {
catalog.storage.delete_row(table_name.storage_id(), mut row)!
// for_storage() here is important because it will strip the qualified
// identifiers down to just their names used in storage.
catalog.storage.delete_row(table_name.storage_id(), mut row.for_storage())!
}

return new_result_msg('DELETE ${rows.len}', elapsed_parse, t.elapsed())
Expand Down
9 changes: 9 additions & 0 deletions vsql/storage.v
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,15 @@ fn (mut f Storage) delete_row(table_name string, mut row Row) ! {
}

page_number := f.btree.expire(row.object_key(f.tables[table_name])!, row.tid, f.transaction_id)!

// A negative page_number means the object didn't exist so there's nothing to
// save. This should not be possible because the delete_row will only be
// issued on a row that already exists, but I guess to be safe let's not let
// it panic.
if page_number < 0 {
return error('DELETE: integrity issue, preventing panic')
}

f.transaction_pages[page_number] = true
}

Expand Down

0 comments on commit eaa590c

Please sign in to comment.