Fix SQLite database queries

Content.Tests/DMProject/Tests/Database/blob.dm

@@ -0,0 +1,27 @@
+/proc/RunTest()
+	var/database/db = new("blob.db")
+	var/database/query/query = new("CREATE TABLE test (value)")
+	query.Execute(db)
+
+	var/icon/icon = new
+	query.Add("INSERT INTO test VALUES (?)", icon)
+	query.Execute(db)
+	ASSERT(query.RowsAffected() == 1)
+
+	query.Add("SELECT typeof(value) FROM test")
+	query.Execute(db)
+	ASSERT(query.NextRow())
+	var/list/type_row = query.GetRowData()
+	ASSERT(type_row["typeof(value)"] == "blob")
+	ASSERT(!query.Error())
+
+	// BYOND stores icons as BLOBs but returns null for BLOB columns through its DM API.
+	query.Add("SELECT value FROM test")
+	query.Execute(db)
+	ASSERT(query.NextRow())
+	ASSERT(isnull(query.GetColumn(0)))
+	ASSERT(!query.Error())
+
+	del(query)
+	del(db)
+	fdel("blob.db")

Content.Tests/DMProject/Tests/Database/clear.dm

@@ -7,8 +7,8 @@
 	// Add with no parameters does nothing
 	query.Add()
 
-	// Execute without a command does nothing
-	query.Execute()
+	// Execute without a command does nothing and returns false.
+	ASSERT(!query.Execute(db))
 
 	// and shouldn't report an error
 	ASSERT(!query.Error())

Content.Tests/DMProject/Tests/Database/read.dm

@@ -1,4 +1,5 @@
 /proc/RunTest()	
+	fdel("database.db")
 	var/database/db = new("database.db")
 
 	var/database/query/query = new("CREATE TABLE test (id int, name string, points float)")
@@ -20,22 +21,23 @@
 	ASSERT(assoc["name"] == "foo")
 	ASSERT(assoc["points"] == 1.5)
 
-	ASSERT(query.GetColumn(0) == 1)
-	ASSERT(query.GetColumn(1) == "foo")
-	ASSERT(query.GetColumn(2) == 1.5)
+	ASSERT(isnull(query.GetColumn(0)))
+	ASSERT(query.GetColumn(1) == 1)
+	ASSERT(query.GetColumn(2) == "foo")
+	ASSERT(query.GetColumn(3) == 1.5)
 
 	var/list/columns = query.Columns()	
 	ASSERT(columns[1] == "id")
 	ASSERT(columns[2] == "name")
 	ASSERT(columns[3] == "points")
 
-	ASSERT(query.Columns(0) == "id")
-	ASSERT(query.Columns(1) == "name")
-	ASSERT(query.Columns(2) == "points")
+	ASSERT(query.Columns(0) == "")
+	ASSERT(query.Columns(1) == "id")
+	ASSERT(query.Columns(2) == "name")
+	ASSERT(query.Columns(3) == "points")
 
 	ASSERT(!query.Columns(10))
-
-	ASSERT(query.Error() && query.ErrorMsg())
+	ASSERT(!query.Error())
 
 	query.Close()
 	db.Close()
@@ -46,10 +48,10 @@
 	query.Execute(db)
 	query.NextRow()
 
-	ASSERT(query.GetColumn(0) == 1)
+	ASSERT(query.GetColumn(1) == 1)
 
 	ASSERT(!query.GetColumn(10))
-	ASSERT(query.Error() && query.ErrorMsg())
+	ASSERT(!query.Error())
 
 	del(query)
 	del(db)

Content.Tests/DMProject/Tests/Database/reexecute.dm

@@ -0,0 +1,25 @@
+/proc/RunTest()
+	fdel("reexecute.db")
+	var/database/db = new("reexecute.db")
+	var/database/query/query = new("CREATE TABLE test (value)")
+	ASSERT(query.Execute(db))
+
+	query.Add("INSERT INTO test VALUES (1), (2)")
+	ASSERT(query.Execute(db))
+
+	query.Add("SELECT value FROM test ORDER BY value")
+	ASSERT(query.Execute(db))
+	ASSERT(query.NextRow())
+	var/list/first_row = query.GetRowData()
+	ASSERT(first_row["value"] == 1)
+
+	// Re-executing must discard the still-open reader and restart the result set.
+	ASSERT(query.Execute(db))
+	ASSERT(query.NextRow())
+	var/list/reexecuted_first_row = query.GetRowData()
+	ASSERT(reexecuted_first_row["value"] == 1)
+	ASSERT(!query.Error())
+
+	del(query)
+	del(db)
+	fdel("reexecute.db")

OpenDreamRuntime/Objects/Types/DreamObjectDatabaseQuery.cs

@@ -7,26 +7,49 @@ namespace OpenDreamRuntime.Objects.Types;
 public sealed class DreamObjectDatabaseQuery(DreamObjectDefinition objectDefinition) : DreamObject(objectDefinition) {
     private SqliteCommand? _command;
     private SqliteDataReader? _reader;
+    private DreamObjectDatabase? _temporaryDatabase;
 
     private string? _errorMessage;
     private int? _errorCode;
 
     public override void Initialize(DreamProcArguments args) {
         base.Initialize(args);
 
-        if (!args.GetArgument(0).TryGetValueAsString(out var command)) {
+        var commandArgument = args.GetArgument(0);
+        if (commandArgument.IsNull) {
             return;
         }
 
+        if (!commandArgument.TryGetValueAsString(out var command)) {
+            throw new DMCrashRuntime("Invalid database query text");
+        }
+
         SetupCommand(command, args.Values[1..]);
     }
 
     protected override void HandleDeletion() {
         ClearCommand();
         CloseReader();
+        CloseTemporaryDatabase();
         base.HandleDeletion();
     }
 
+    /// <summary>
+    /// Associates a database with this query that was opened from a filename
+    /// (BYOND's <c>query.Execute(filename)</c> overload). The query owns it for
+    /// the duration of reading rows, and closes it when done.
+    /// </summary>
+    public void SetTemporaryDatabase(DreamObjectDatabase database) {
+        CloseTemporaryDatabase();
+        _temporaryDatabase = database;
+    }
+
+    private void CloseTemporaryDatabase() {
+        _temporaryDatabase?.Close();
+        _temporaryDatabase?.DecRef();
+        _temporaryDatabase = null;
+    }
+
     /// <summary>
     /// Sets up the SQLiteCommand, setting up parameters when provided.
     /// Supports strings and floats from DMcode.
@@ -39,33 +62,26 @@ public void SetupCommand(string command, ReadOnlySpan<DreamValue> values) {
         for (var i = 0; i < values.Length; i++) {
             var arg = values[i];
 
-            var type = arg.Type;
-            switch (type) {
-                case DreamValue.DreamValueType.String:
-                    if (arg.TryGetValueAsString(out var stringValue)) {
-                        _command.Parameters.AddWithValue($"@{i}", stringValue);
-                    }
-
-                    break;
-                case DreamValue.DreamValueType.Float:
-                    if (arg.TryGetValueAsFloat(out var floatValue)) {
-                        _command.Parameters.AddWithValue($"@{i}", floatValue);
-                    }
-
-                    break;
-
-                case DreamValue.DreamValueType.DreamResource:
-                case DreamValue.DreamValueType.DreamObject:
-                case DreamValue.DreamValueType.DreamType:
-                case DreamValue.DreamValueType.DreamProc:
-                case DreamValue.DreamValueType.Appearance:
-                default:
-                    // TODO: support saving BLOBS for icons, if we really want to
-                    break;
+            if (arg.IsNull) {
+                _command.Parameters.AddWithValue($"@{i}", DBNull.Value);
+            } else if (arg.TryGetValueAsDreamResource(out var resource)) {
+                AddBlobParameter(i, resource.ResourceData);
+            } else if (arg.TryGetValueAsDreamObject<DreamObjectIcon>(out var icon)) {
+                AddBlobParameter(i, icon.Icon.GenerateDMI().ResourceData);
+            } else if (arg.TryGetValueAsString(out var stringValue)) {
+                _command.Parameters.AddWithValue($"@{i}", stringValue);
+            } else if (arg.TryGetValueAsFloat(out var floatValue)) {
+                _command.Parameters.AddWithValue($"@{i}", floatValue);
+            } else {
+                throw new DMCrashRuntime("Invalid database query parameter");
             }
         }
     }
 
+    private void AddBlobParameter(int index, byte[]? data) {
+        _command!.Parameters.Add($"@{index}", SqliteType.Blob).Value = data is null ? DBNull.Value : data;
+    }
+
     /// <summary>
     /// Gets the names of all the columns in the current query
     /// </summary>
@@ -84,26 +100,21 @@ public List<DreamValue> GetAllColumns() {
     }
 
     /// <summary>
-    /// Gets the name of a single column in the current query
+    /// Gets the name of a selected column for <c>/database/query.Columns(index)</c>.
     /// </summary>
-    /// <param name="id">The column ordinal value.</param>
-    /// <returns>A <see cref="DreamValue"/> of the name of the column.</returns>
-    public DreamValue GetColumn(int id) {
-        if (_reader is null) {
-            return DreamValue.Null;
-        }
-
-        try {
-            var name = _reader.GetName(id);
-            return new DreamValue(name);
-        } catch (IndexOutOfRangeException exception) {
-            _errorCode = 1;
-            _errorMessage = exception.Message;
+    /// <param name="column">The one-based column index.</param>
+    /// <returns>The column name, or an empty string for an invalid index.</returns>
+    public DreamValue GetColumnName(int column) {
+        if (_reader is null || column <= 0 || column > _reader.FieldCount) {
+            // BYOND returns an empty string for an invalid Columns() index.
+            return new DreamValue(string.Empty);
         }
 
-        return DreamValue.Null;
+        return new DreamValue(_reader.GetName(column - 1));
     }
 
+    public bool HasCommand => _command != null;
+
     public void ClearCommand() {
         _command?.Dispose();
         _command = null;
@@ -126,44 +137,49 @@ public void CloseReader() {
     /// Executes the currently held query against the SQLite database
     /// </summary>
     /// <param name="database">The <see cref="DreamObjectDatabase"/> that this query is being run against.</param>
-    public void ExecuteCommand(DreamObjectDatabase database) {
-        if (!database.TryGetConnection(out var connection)) {
-            throw new DMCrashRuntime("Bad database");
+    public bool ExecuteCommand(DreamObjectDatabase database) {
+        if (_command == null) {
+            return false;
         }
 
-        if (_command == null) {
-            return;
+        CloseReader();
+
+        if (!database.TryGetConnection(out var connection)) {
+            throw new DMCrashRuntime("Bad database");
         }
 
         _command.Connection = connection;
 
         try {
             _reader = _command.ExecuteReader();
+            return true;
         } catch (SqliteException exception) {
             _errorCode = exception.SqliteErrorCode;
             _errorMessage = exception.Message;
             database.SetError(exception.SqliteErrorCode, exception.Message);
+            return false;
         }
     }
 
-    public void NextRow() {
-        _reader?.Read();
+    public bool NextRow() {
+        return _reader?.Read() ?? false;
     }
 
     /// <summary>
-    /// Attempts to fetch the value of a specific column.
+    /// Attempts to fetch the current row's value for <c>/database/query.GetColumn(index)</c>.
     /// </summary>
-    /// <param name="column">The ordinal column number</param>
-    /// <param name="value">The out variable to be populated with the <see cref="DreamValue"/>of the result.</param>
-    /// <returns></returns>
-    public bool TryGetColumn(int column, out DreamValue value) {
-        if (_reader is null) {
+    /// <param name="column">The one-based column index.</param>
+    /// <param name="value">The current row's column value.</param>
+    /// <returns>True when the value was read; otherwise false.</returns>
+    public bool TryGetColumnValue(int column, out DreamValue value) {
+        if (_reader is null || column <= 0 || column > _reader.FieldCount) {
+            // BYOND returns null for an invalid GetColumn() index.
             value = DreamValue.Null;
             return false;
         }
 
         try {
-            value = GetDreamValueFromDbObject(_reader.GetValue(column));
+            value = GetDreamValueFromDbObject(_reader.GetValue(column - 1));
             return true;
         } catch (Exception exception) {
             _errorCode = 1;
@@ -213,6 +229,9 @@ private static DreamValue GetDreamValueFromDbObject(object value) {
             long longValue => new DreamValue(longValue),
             int intValue => new DreamValue(intValue),
             string stringValue => new DreamValue(stringValue),
+            // BYOND's database API returns null, rather than exposing SQLite BLOB data.
+            byte[] => DreamValue.Null,
+            DBNull => DreamValue.Null,
             _ => throw new ArgumentOutOfRangeException(nameof(value)),
         };
     }