Modify

Opened 4 years ago

Closed 2 years ago

#7115 closed enhancement (fixed)

Patch to make plugin work with PostgreSQL 8.3

Reported by: Chris.Nelson@… Owned by: ChrisNelson
Priority: normal Component: TeamCalendarPlugin
Severity: normal Keywords: PostgreSQL compatibility
Cc: rjollos Trac Release: 0.11

Description

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

Attachments (1)

teamcalendar.patch (3.5 KB) - added by Chris.Nelson@… 4 years ago.
Patch for PostgreSQL support

Download all attachments as: .zip

Change History (16)

Changed 4 years ago by Chris.Nelson@…

Patch for PostgreSQL support

comment:1 follow-up: Changed 3 years ago by hasienda

  • Keywords PostgreSQL compatibility added

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.

comment:2 in reply to: ↑ 1 ; follow-up: Changed 3 years ago by ChrisNelson

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?

comment:3 in reply to: ↑ 2 ; follow-ups: Changed 3 years ago by hasienda

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.

comment:4 in reply to: ↑ 3 Changed 3 years ago by hasienda

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

comment:5 follow-ups: Changed 3 years ago by 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:

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

comment:6 Changed 3 years ago by rjollos

  • Cc rjollos added

comment:7 in reply to: ↑ 5 ; follow-up: Changed 3 years ago by ChrisNelson

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:

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 3 years ago by osimons

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.

comment:9 in reply to: ↑ 5 ; follow-up: Changed 3 years ago by ChrisNelson

  • Owner changed from optilude to ChrisNelson
  • Status changed from new to assigned

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:

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 3 years ago by ChrisNelson

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?

comment:11 in reply to: ↑ 9 Changed 3 years ago by osimons

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.

comment:12 Changed 3 years ago by hasienda

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 2 years ago by ChrisNelson

(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 2 years ago by ChrisNelson

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

comment:15 Changed 2 years ago by ChrisNelson

  • Resolution set to fixed
  • Status changed from assigned to closed

Add Comment

Modify Ticket

Action
as closed .
as The resolution will be set. Next status will be 'closed'.
to The owner will be changed from ChrisNelson. Next status will be 'closed'.
The resolution will be deleted. Next status will be 'reopened'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.