[Trac] PostgreSQL Conversion

[Christopher Lenz]

Hi Brad,

Am 05.01.2005 um 08:14 schrieb Brad Anderson:
> Due to the overwhelming support for this post to the mailing list ;)  
> ... I
> continued on and put another patch for PostgreSQL on my wiki page.

I'm glad to the database independence story pick up steam again :-)

Two comments based on a quick read of the patch:

1/ Changes like using COALESCE instead of IFNULL could be folded into  
trunk immediately. I.e. anything that moves to more universal SQL and  
works well with SQLite (obviously). It'd be nice to have separate  
patches for such changes.

2/ Consider the following change found in  
<http://trac.dsource.org/projects/test/attachment/wiki/PostgresqlPatch/ 
pgsql_patch_02.diff>:

            cursor = db.cursor ()
   -        cursor.execute('SELECT name,value FROM ticket_custom WHERE  
ticket=%i', id)
   +        sql = "SELECT name,value FROM ticket_custom WHERE ticket=%s"  
% id
   +        cursor.execute(sql)
            rows = cursor.fetchall()

This is bad because you are now using string formatting to insert the  
parameter, instead of using the DB API. The DB API will escape the  
value for you, and quote it if necessary etc. It should be used  
wherever possible. When it absolutely isn't possible to use the DB API,  
parameters *need* to be escaped with the util.sql_escape() function.

Now you probably made those changes because you got errors, but maybe  
the following FAQ entry helps: "Using pyPgSQL, you use %s for all  
parameters, no matter which type they have"  
<http://pypgsql.sourceforge.net/pypgsql-faq.html#id2836440>. It would  
be okay to use %s everywhere IMHO, and *much* better than using string  
formatting for parameter insertion.

Cheers,
Chris
--
Christopher Lenz
/=/ cmlenz at gmx.de