query.Execute() has no return value

ID:2802889
 
Jul 8 2022, 11:05 pm
BYOND Version:514
Operating System:Windows 10 Pro 64-bit
Web Browser:Chrome 103.0.5060.114
Applies to:DM Language
Status: Open
Issue hasn't been assigned a status value.
Descriptive Problem Summary:

Despite the reference examples for using query.Execute() showing it used as if it has some kind of return value. It does not. It should probably match other implementations of SQLite where Execute() will return true on success and false on failure.

Numbered Steps to Reproduce Problem:

Code Snippet (if applicable) to Reproduce Problem:
var/database/query/my_query = new("SELECT * FROM my_table")
world << my_query.Execute(my_db) // null


Expected Results:
Execute() to have a true/false return value.

Actual Results:

No return value at all. This means "if(query.Execute(db))" in the reference never actually triggers.



Jul 10 2022, 11:38 pm
I'm looking at the code now and it very definitely returns a 1 or 0 numeric value for Execute(). The only time null should be returned is if the argument isn't a database. So it seems the problem is the my_db value not being a /database.

I do think however it'd be wise to crash the proc with an error message in that case. Returning null doesn't make nearly as much sense.
Jul 11 2022, 12:07 am
My code assumes it does so, but I've come across multiple instances where

if(!query.Execute(db))


Doesn't work due to something like a syntax error in the SQL. The only indicator that it failed is ErrorMsg() not being empty.
Jul 11 2022, 10:36 am
Execute() always returns 0 or 1 unless the argument is not a database. I can guarantee that from the code. The only way for that not to happen would be in the event of an exception, but then either that would generate a runtime error or it'd choke completely from the exception not being caught.

I'll definitely change the behavior in 515 to throw a runtime error in the event that a non-db argument happens though.
Jul 11 2022, 2:51 pm
After more testing, the issue is it returns 1 on invalid syntax, it shouldn't.
mob
    verb
        MakeDB()
            var/database/db = new("database.db")
            var/database/query/query = new("CREATE TABLE test (test INTEGER)")
            world << query.Execute(db) // 1

        TestQuery()
            var/database/db = new("database.db")
            var/database/query/query = new("SELECT * FROM tesdt")
            world << query.Execute(db) // 1


The query in TestQuery() throws a syntax error, in other languages this would make Execute() either crash to catch an exception, or it returns false to indicate the query failed. In DM it returns 1 and gives no indication the query failed.