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.

Author: elliotchance

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

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

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())

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
 }