reform icon indicating copy to clipboard operation
reform copied to clipboard

Check for absence of required arguments

Open AlekSi opened this issue 7 years ago • 2 comments

One example is Querier.FindAllFrom(view View, column string, args ...interface{}), which can be called like this:

s.q.FindAllFrom(ProjectTable, "id")

It generates this query (simplified):

SELECT * FROM projects WHERE id IN ()

This query is valid for some databases (e.g. SQLite3) and invalid for others (e.g. PostgreSQL, MySQL). Quote from https://www.sqlite.org/lang_expr.html:

Note that SQLite allows the parenthesized list of scalar values on the right-hand side of an IN or NOT IN operator to be an empty list but most other SQL database database engines and the SQL92 standard require the list to contain at least one element.

I think if we start to require non-empty list here, we will break some apps using SQLite3 and depending on this behavior, so we can't fix it until v2. In the same time, we don't want to encourage this behavior even in v1, so we should emit a warning via logger.

@AlekseyMartynov @rumyantseva WDYT?

AlekSi avatar Jan 11 '17 06:01 AlekSi

Alternatively, we can check that list is empty, and do not perform a query at all. InsertMulti is one existing example.

AlekSi avatar Jan 11 '17 07:01 AlekSi

How about replacing empty IN with WHERE 0 in this particular case? With a warning as you suggest. This wouldn't break existing apps.

AlekseyMartynov avatar Jan 11 '17 08:01 AlekseyMartynov