Database queries are broken when multiple queries are passed in a single query object.

ID:2035969
 
Feb 13 2016, 12:11 pm (Edited on Feb 13 2016, 12:23 pm)
BYOND Version:510.1323
Operating System:Windows 7 Ultimate 64-bit
Web Browser:Chrome 48.0.2564.103
Applies to:Dream Daemon
Status: Open
Issue hasn't been assigned a status value.
Descriptive Problem Summary:

Running a two-part query results in the first query running twice.

Code Snippet (if applicable) to Reproduce Problem:
var/database/query/q = new("INSERT INTO players (name,ckey) VALUES (?,?);SELECT last_insert_rowid()",lowertext(name),ckey)
if(q.Execute(playerdb)&&q.NextRow())
   ...yada yada


Workaround:
Separate into two blocking queries... Which is really entirely counterintuitive.

var/database/query/q = new("INSERT INTO players (name,ckey) VALUES (?,?)",lowertext(name),ckey)
if(q.Execute(playerdb))
    q.Clear()
    q.Add("SELECT last_insert_rowid()")
    if(q.Execute(playerdb)&&q.NextRow())
        var/list/l = q.GetRowData()
        return l["last_insert_rowid()"]
world.log << q.ErrorMsg()


Expected Results:
Insert once, then return a single row to get the last_instert_rowid() value from SQLlite.

Actual Results:

Performs the first query twice, inserting two identical rows and returns no rows.


Also, databasequery is SEVERELY lacking in standardized behavior for last_row information. You have to perform a second query to get the row of the value you just changed/inserted, which is very much something that no other database implementation I've ever worked with requires.

Most Execute() functions return the row that was affected by the query on an Insert.

There's no way this is an actual bug, because someone else would have reported it by now, right? Right?
Feb 13 2016, 2:26 pm
I'll have to look into how SQLite deals with two-part queries. Compound statements weren't really a consideration when this was built.

I'm not sure it would necessarily be best to return a value other than pass/fail for Execute() (I understand the reasoning, but it would frell up any existing implementations), but /database could be extended to get that info fairly painlessly.

database
    proc/RowsAffected()
        var/database/query/q = new("SELECT changes()")
        return (q.Execute(src) && q.NextRow()) ? q.GetColumn(1) : 0
    proc/LastInsertID()
        var/database/query/q = new("SELECT last_insert_rowid()")
        if(q.Execute(src) && q.NextRow()) return q.GetColumn(1)

I'm not averse to adding some convenience routines like that to stddef.dm.
Feb 13 2016, 2:53 pm
Generally, most engines use a special flag or proc or function for running multiqueries over single queries.

Mainly to lower the exploit vector of sql injection when programmers make the mistake of leaving a vulnerability in.

Its a bit less damaging to change a update to apply to all rows, or to insert bad data, or select private data, then it is to drop the entire database.

You could make this require setting a property on the query object to enable multiquery support

I'm not sure it would necessarily be best to return a value other than pass/fail for Execute()

Just make it set properties on the query object.

Or do it how other engines do it, and make multiple execute methods that have different return values. one for returning just the first row, one for the first column. Or maybe make that a property of the query object, something that says how to return from execute.

The possibilities are endless
May 2 2016, 8:00 pm
Oh thanks for posting this. I was tearing my hair out looking for this.