allow empty slice for 'in' queries

[markus-wa]

Can be useful for queries like below.

db.Exec("DELETE FROM server_config WHERE server_id = ? AND config_id NOT IN (?)", srvID, configIDs)

[JoeFerrucci]

Why not just check before you execute your query.

if len(configIDs) != 0 {
 db.Exec("DELETE FROM server_config WHERE server_id = ? AND config_id NOT IN (?)", srvID, configIDs)
}

[markus-wa]

if len(configIDs) == 0 we want to delete ALL server_config entries with server_id == srvID - so the query needs to be run regardless.

note the NOT IN rather than IN

[JoeFerrucci]

if len(configIDs) == 0 {
 db.Exec("DELETE FROM server_config WHERE server_id = ?", srvID)
} else {
 db.Exec("DELETE FROM server_config WHERE server_id = ? AND config_id NOT IN (?)", srvID, configIDs)
}

[markus-wa]

I mean, of course - but why make things complicated? especially when there are multiple such parameters that would lead to unnecessarily complicated repetition and nested if statements.

I don't see any benefit to keeping this check.

[JoeFerrucci]

Have you tried running your query manually with an empty config_ids set to see what it does?

If you tried that it would give you an error. Empty IN and NOT IN are not supported

But even if SQL did support that syntax, the size of the table and the access frequency could affect performance and cost. From a performance and cost perspective, its best to do the simple calculation in the app instead of having the DB do it; but even still, SQL doesn't like empty sets in that clause which is why that check is there.

[markus-wa]

Empty IN actually works fine with SQLite @JoeFerrucci - and that's why there is a second condition, which is indexed.. there will only ever be < 5 entries matching a server_id in this scenario.

But even otherwise, for this app there will never be any amount of data where a full table scan would slow anything down whatsoever.

I've been using my patch without problems for my usecase for 2 weeks now.

SQLFiddle: https://sqlfiddle.com/sqlite/online-compiler?&id=5ad24c05-77a0-43d9-92d4-2d2ad520f9a0

[markus-wa]

What's the benefit of returning the error early vs letting the DB engine decide if it wants to allow empty IN statements? Except for saving a few milliseconds in what is probably a very rare condition.

[JoeFerrucci]

Most SQL databases consider this syntax invalid and will raise an error; your PR cannot be merged in. If your local branch with your changes works for you that's perfect.

[markus-wa]

I'm sorry but I still don't understand the benefit of the check.

Either the error gets raised here or ot gets raised when the DB is queried - what's the harm in raising it later?

Also why does it matter how many DBs support this? What would we do if it was 50/50? Toss a coin?

[markus-wa]

I suppose one reason I can see is that it would be a breaking change and it's risky if it only helps for SQLite, which is fair enough.

[JoeFerrucci]

Based on the SQL spec, at least one is required.