Allow single argument passed to DbContextManager.execute

[Ryan J Ollos]

A common programming error with the Trac API is to pass a single argument without wrapping it in a tuple or list:

self.env.db_query("SELECT * FROM ticket WHERE id=%s", id)

rather than:

self.env.db_query("SELECT * FROM ticket WHERE id=%s", (id,))

Could we allow non-tuple/list arguments and have DbContextManager.execute wrap the argument in a tuple?

[Ryan J Ollos]

I'd like to address the programming error described in ​SO:53904305/121694.

[Ryan J Ollos]

Proposed change in [875cef97ac/rjollos.git].

[Jun Omae]

I think _to_list() could be simplified like this:

     def _to_list(self, params):
-        if not isinstance(params, (list, tuple)):
-            return [params] if params is not None else []
-        return params
+        if params is None:
+            return []
+        if isinstance(params, (list, tuple)):
+            return params
+        return [params]

Minor thing, psycopg2 and mysqldb library accept %(name)s-style markers with a dict parameter. The changes of API would break the behaviors.

psycopg2

>>> env = Environment('/home/jun66j5/var/trac/1.0-postgres')
>>> env.db_query('SELECT %(name)s', {'name': 42})
[(42,)]
>>> env.db_query('SELECT %(name)s', {'name': "aa'bb"})
[("aa'bb",)]
>>> env.db_query('SELECT %(name)s', {'name': 'aa"bb'})
[('aa"bb',)]

MySQLdb

>>> env = Environment('/home/jun66j5/var/trac/1.0-mysql')
>>> env.db_query('SELECT %(name)s', {'name': 42})
[(42L,)]
>>> env.db_query('SELECT %(name)s', {'name': "aa'bb"})
[(u"aa'bb",)]
>>> env.db_query('SELECT %(name)s', {'name': 'aa"bb'})
[(u'aa"bb',)]

[Ryan J Ollos]

I suppose we could pass through dicts as well:

if isinstance(params, (list, tuple, dict)):
    return params

[Ryan J Ollos]

Further thought, another way this could be misinterpreted is the developer expecting to be able to pass multiple arguments like:

env.db_query(query, arg1, arg2, ...)

If we were implementing the API now, it might make more sense to have the function signature:

    def execute(self, query, *params):

I'm considering it might be better to just leave the behavior as it is.

Anyway, my test case is not very good and fails on MySQL because system is a reserved word (#13128).

[Ryan J Ollos]

Replying to Ryan J Ollos:

If we were implementing the API now, it might make more sense to have the function signature:

    def execute(self, query, *params):

For example, the former seems more readable and less error prone:

                    db("""
                        UPDATE wiki SET text=%s WHERE name=%s AND version=%s
                        """, self.text, self.name, self.version)

                    db("""
                        UPDATE wiki SET text=%s WHERE name=%s AND version=%s
                        """, (self.text, self.name, self.version))

But it would likely be a bad idea to differ from the ​Python sqlite API, which requires that parameters be a sequence or dict.