Patch to make plugin work with PostgreSQL 8.3

Patch for PostgreSQL support

I strongly advocate full rework of SQL towards db backend-agnostic statements. Let's follow-up here to a related discussion started at #9595:

Replying to ChrisNelson:

Replying to hasienda:

IMHO Trac has a fine db abstraction. Anything more complicated than very basic WHERE clauses has some wrapper methods to hide db-specific SQL. See t:wiki:TracDev/DatabaseApi for the current state, but beware to stick to the old style as long as backwards-compatibility down to trac-0.11 matters.

... That's less extensive than I hoped. I hoped I was missing something. My problem is one RDBMS wants single quotes and another wants double so if I do:

cursor.execute("SELECT * FROM myTable WHERE foo='bar'")

It will only work in one of them. Or maybe I'm superstitious because I've gotten burned on weird quoting issues in SQL.

Not, if you make consequent use of Trac db code. Did you look into the docs or Trac core code of the db abstraction layer yet? I.e. there is a dedicated method db.quote() in each of the supported backends with db being the ConnectionWrapper class (SQLiteConnection|MySQLConnection|PostgreSQLConnection) in mysql_backend.py, sqlite_backend.py and postgres_backend.py respectively to cope with proprietary syntax of each of them.

Replying to hasienda:

... Not, if you make consequent

Consistent?

use of Trac db code. Did you look into the docs or Trac core code of the db abstraction layer yet? I.e. there is a dedicated method db.quote() in each of the supported backends with db being the ConnectionWrapper class (SQLiteConnection|MySQLConnection|PostgreSQLConnection) in mysql_backend.py, sqlite_backend.py and postgres_backend.py respectively to cope with proprietary syntax of each of them.

db.quote() is news to me. I haven't found documentation for the DB API (not that I've looked very hard yet). That sounds like it solves the problem I mentioned above.

Is db.quote() Python or Trac? Can you point me to documentation?

Replying to ChrisNelson:

Replying to hasienda:

... Not, if you make consequent

Consistent?

consequent

2. (Logic) Following by necessary inference or rational deduction; as, a proposition consequent to other propositions. [1913 Webster]

But in all places, so consistent applies here as well.

use of Trac db code. Did you look into the docs or Trac core code of the db abstraction layer yet? I.e. there is a dedicated method db.quote() in each of the supported backends with db being the ConnectionWrapper class (SQLiteConnection|MySQLConnection|PostgreSQLConnection) in mysql_backend.py, sqlite_backend.py and postgres_backend.py respectively to cope with proprietary syntax of each of them.

db.quote() is news to me. I haven't found documentation for the DB API (not that I've looked very hard yet). That sounds like it solves the problem I mentioned above. Is db.quote() Python or Trac? Can you point me to documentation?

Hmm, how hard ...? Well, if you insist: t:browser:branches/0.11-stable/trac/db/mysql_backend.py and read through all methods of the MySQLConnection class. You find ready-made examples of these wrapper methods everywhere in Trac, i.e. at several places in trac/ticket/model.py.

Trac's smart wrappers are an extension of the Python db API v2.0, and I've already give you all the hint's. I'm not aware of a definitive walk-through/hands-on doku, just existing, more ore less (self-)documented code. Maybe you want to look at some other plugins too, how these have been converted from proprietary SQL towards making use of the abstraction Trac's layer.?

If you're unsure, throw out a patch and I'll have a look. Learning by doing is exactly the way I explored most of the Trac internals myself. You're no alone, and it'll pay well off, as soon as you get there.

Replying to hasienda:

... Maybe you want to look at some other plugins too, how these have been converted from proprietary SQL towards making use of the abstraction Trac's layer.?

[10775] is an example of what I had in mind here. You should find more easily by looking for tickets tagged tagged:MySQL (including variations like tagged:mysql ) or tagged:PostgreSQL (tagged:postgresql ), maybe even in combination with tagged:compatibility

An even simple alternative for the example above is just to pass it as argument, and Trac will do whatever needed for quotes and types depending on backend:

cursor.execute("SELECT * FROM myTable WHERE foo=%s", ('bar',))

Replying to osimons:

An even simple alternative for the example above is just to pass it as argument, and Trac will do whatever needed for quotes and types depending on backend: {{{ #!python cursor.execute("SELECT * FROM myTable WHERE foo=%s", ('bar',)) }}}

That's very cool.

What's the extra comma for at the end?

Replying to ChrisNelson:

What's the extra comma for at the end?

Basic Python: It marks a one-item tuple, and is used to differentiate it from (something) which is just something put in brackets. If the tuple had contained two items, the comma would not be needed as the form is no longer ambiguous.

The call is cursor.execute(stmt, args), so could of course also have used a list and written ['bar'] instead (list syntax is not ambiguous). A list is mutable and a tuple is not, so for iterables that won't or shouldn't be modified the tuple is a general coding style preference.

However, for sql.execute() it just has to quack like an iterable. Use whatever makes sense for your situation and each context.

Replying to osimons:

An even simple alternative for the example above is just to pass it as argument, and Trac will do whatever needed for quotes and types depending on backend: {{{ #!python cursor.execute("SELECT * FROM myTable WHERE foo=%s", ('bar',)) }}}

My Python is a little weak. How do I adapt that pattern to:

            cursor.execute("SELECT name, due FROM milestone " +
                           "WHERE name in ('" + "','".join(milestones) + "')")

Replying to hasienda:

...

use of Trac db code. Did you look into the docs or Trac core code of the db abstraction layer yet? I.e. there is a dedicated method db.quote() in each of the supported backends with db being the ConnectionWrapper class (SQLiteConnection|MySQLConnection|PostgreSQLConnection) in mysql_backend.py, sqlite_backend.py and postgres_backend.py respectively to cope with proprietary syntax of each of them.

db.quote() is news to me. I haven't found documentation for the DB API (not that I've looked very hard yet). That sounds like it solves the problem I mentioned above. Is db.quote() Python or Trac? Can you point me to documentation?

Hmm, how hard ...? Well, if you insist: t:browser:branches/0.11-stable/trac/db/mysql_backend.py and read through all methods of the MySQLConnection class. You find ready-made examples of these wrapper methods everywhere in Trac, i.e. at several places in trac/ticket/model.py. ...

I'm missing something. I'm in Trac 0.11.6 and in trac/db, I see:

trac/db$ grep quote *.py
api.py:            user = urllib.unquote(user)
api.py:            password = unicode_passwd(urllib.unquote(password))
api.py:            value = urllib.unquote(value)

and in ticket:

trac/ticket$ grep quote *
web_ui.py:        def quote_original(author, original, link):
web_ui.py:            quote_original(ticket['reporter'], ticket['description'],
web_ui.py:                        quote_original(change['author'], change['comment'],

Where is this mysterious db.quote() of which you speak?

Replying to ChrisNelson:

My Python is a little weak. How do I adapt that pattern to: cursor.execute("SELECT name, due FROM milestone " + "WHERE name in ('" + "','".join(milestones) + "')")

You need to build the string containing one %s for each milestone. So that could be done like this:

cursor.execute("SELECT name, due FROM milestone "
+ "WHERE name in (" + ",".join(['%s']*len(milestones)) + ")", milestones)

Which when run produces this actual call - presuming length of milestones array is 3:

cursor.execute('SELECT name, due FROM milestone WHERE name in (%s,%s,%s)',
            milestones)

Now, the use of string concatenation using "+" is not always so readable. What you can do is use string interpolation for putting the %s sequence into the string, and then the combined string will be available for argument substitution by cursor.execute(). String interpolation is obviously fine for any kind of string manipulation that don't involve user submitted data:

cursor.execute("SELECT name, due FROM milestone " \
        "WHERE name in (%s)" % ",".join(['%s']*len(milestones)), milestones)

Note also the use of \ to mark that the line continues - useful to just continue the string on the next line without + concatenation. It makes it more readable.

Speaking of readability you could write even nicer as a multi-line Python string like so:

cursor.execute("""SELECT name, due
                  FROM milestone
                  WHERE name in (%s)
                  """ % ",".join(['%s']*len(milestones)), milestones)

While not consistently used it is kind of Trac current best coding style/practice appreciated by core devs, if you look i.e. at the current source for milestone model.

(In [11189]) An alternative, db-agnostic fix for updating availability. Refs #4243, #7115.

This approach assumes it is faster to manipulate Python lists in memory and minimize DB access than to do at least one DB access per record.

The patch in 4243 did one SELECT to get the state of the DB, then one UPDATE or INSERT for each and every record.

The patch in 7115 did something similar less elegantly.

This version does one SELECT, some list manipulation, one UPDATE for each *changed* record, one INSERT for *all* *new* records, and nothing for records that didn't change.

(In [11206]) Finish conversion to Trac DP API. Refs #4243, #7115.