From 6133f99cb678984bfb5cd1acfe8c411ed566d51d Mon Sep 17 00:00:00 2001
From: Elliot Chance <elliotchance@gmail.com>
Date: Fri, 27 Dec 2024 16:00:18 -0500
Subject: [PATCH] DELETE: Fix panic when using PRIMARY KEY

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.
---
 tests/delete.sql                          | 14 ++++++++++++++
 tests/update.sql                          | 12 ++++++++++++
 vsql/std_14_9_delete_statement_searched.v |  4 +++-
 vsql/storage.v                            |  9 +++++++++
 4 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/tests/delete.sql b/tests/delete.sql
index 6b12440..735d817 100644
--- a/tests/delete.sql
+++ b/tests/delete.sql
@@ -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
diff --git a/tests/update.sql b/tests/update.sql
index e3cf5c2..0f191bc 100644
--- a/tests/update.sql
+++ b/tests/update.sql
@@ -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
diff --git a/vsql/std_14_9_delete_statement_searched.v b/vsql/std_14_9_delete_statement_searched.v
index ce0bb1c..3213737 100644
--- a/vsql/std_14_9_delete_statement_searched.v
+++ b/vsql/std_14_9_delete_statement_searched.v
@@ -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())
diff --git a/vsql/storage.v b/vsql/storage.v
index 2135128..d3283bc 100644
--- a/vsql/storage.v
+++ b/vsql/storage.v
@@ -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
 }