Opened 7 years ago

Closed 5 years ago

# Patch to make plugin work with PostgreSQL 8.3

Reported by: Owned by: Chris.Nelson@… Chris Nelson normal TeamCalendarPlugin normal PostgreSQL compatibility Ryan J Ollos 0.11

### Description

Minimal testing but it does come up and let you toggle availability.

### Changed 7 years ago by Chris.Nelson@…

Patch for PostgreSQL support

### comment:1 follow-up:  2 Changed 5 years ago by Steffen Hoffmann

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

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.

### comment:2 in reply to:  1 ; follow-up:  3 Changed 5 years ago by Chris Nelson

... 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?

### comment:3 in reply to:  2 ; follow-ups:  4  10 Changed 5 years ago by Steffen Hoffmann

... 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.

### comment:4 in reply to:  3 Changed 5 years ago by Steffen Hoffmann

... 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

### comment:5 follow-ups:  7  9 Changed 5 years ago by Odd Simon Simonsen

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',))


### comment:6 Changed 5 years ago by Ryan J Ollos

Cc: Ryan J Ollos added; anonymous removed

### comment:7 in reply to:  5 ; follow-up:  8 Changed 5 years ago by Chris Nelson

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',))


That's very cool.

What's the extra comma for at the end?

### comment:8 in reply to:  7 Changed 5 years ago by Odd Simon Simonsen

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.

### comment:9 in reply to:  5 ; follow-up:  11 Changed 5 years ago by Chris Nelson

Owner: changed from Martin Aspeli to Chris Nelson new → assigned

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',))


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) + "')")


### comment:10 in reply to:  3 Changed 5 years ago by Chris Nelson

...

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:            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?

### comment:11 in reply to:  9 Changed 5 years ago by Odd Simon Simonsen

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.

### comment:12 Changed 5 years ago by Steffen Hoffmann

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.

### comment:13 Changed 5 years ago by Chris Nelson

(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.

### comment:14 Changed 5 years ago by Chris Nelson

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

### comment:15 Changed 5 years ago by Chris Nelson

Resolution: → fixed assigned → closed

### Modify Ticket

Action
as closed The owner will remain Chris Nelson.
The resolution will be deleted. Next status will be 'reopened'.