DBI and SQL Escape

Scott Hickey has discovered a bug in DBI:

;; assume you have a table1 with an id and a date field. 
(exec h "insert into table1 values (?id , ?date)" `((id . 1) (date . ,(srfi19:current-date))))
;; => regexp-replace*: expects type  as 2nd argument, given:
;;    #(struct:tm:date 9150000 8 19 0 30 9 2009 -18000); other arguments were: #px"\\'" "''"

This issue is now fixed and the newer version of the DBI and are now available through planet:

• bzlib/dbi:1:2
• bzlib/dbd-jazmysql:1:1
• bzlib/dbd-jsqlite:1:2
• bzlib/dbd-spgsql:1:1

This post documents the issue and the resolution of the bug.

This issue is caused by the default SQL escape code that does not know how to handle srfi date objects. The default SQL escape code is quite primitive for the reasons below.

To work around the problem - you can use prepared statements:

(prepare h 'insert-table1 "insert into table1 values (?id , ?date)")
(exec h 'insert-table1 `((id . 1) (date . ,(srfi19:current-date))))

The default preparation code exists as a prepared statement proxy for those database drivers that have no prepared statement capabilities. This is one of the selling points of DBI over other API - the query always allow parameterizations. But that means DBI cannot delegate the task of SQL escaping back to the user.

Because my usage of database has always surround prepared statements, I did not write an extensive SQL escaping library (and hence the bug). Plus, there were technical reasons that prepared statements are superior:

• SQL escapes does not work uniformly across all databases, especially for types such as blobs and date objects, which each database have their own syntax (and some basically discourage using SQL escapes for blobs)
  
• SQL escapes are prone to SQL injections if done poorly. One of my previous gigs was to weed out SQL injection bugs in client code base and while the concept is simple many implementations still got it wrong
  
• In general prepared statements will have better performances (but this unfortunately is not always true if the cached query plan results in a miss by the server) for multiple uses

Prepared statements (and stored procedures) are superior to SQL escapes in just about all aspects, including performance and security. There are only three downsides that I am aware of for prepared statements:

• it might cause the database to hold onto the referred objects so it cannot be dropped - this mainly impacts development environment, since that actually helps your production environment from having tragic accident of dropping tables, views, etc.
  
• it might not work well for code that creates dynamic SQL statements that refers to tables with unique prefixes (wordpress and a bunch of PHP code falls into this design style), since there might be thousands of such unique prefixes in a given database. In general, such design really should be discouraged, since databases are designed more for few large tables instead of many small tables
• it's not all that useful and can potentially be slower for one-call statements, but most of the time this is a non-issue

Anyhow - the reason I am highlighting the merit of prepared statements over SQL escapes is that I believe going toward prepared statements is the way to go, especially for databases that already have them. So I decide to make the database drivers for dbd-spgsql, dbd-jsqlite, and dbd-jazmysql to implicitly create prepared statements if you do not want to explicitly name the query via the prepare call.

So - you can just write:

(exec h "insert into table1 values (?id , ?date)" `((id . 1) (date . ,(srfi19:current-date))))

And it will behave as if you do the following:

;; note - prepare only takes symbol as a key - so you cannot do this manually yourself
(prepare h "insert into table1 values (?id , ?date)" "insert into table1 values (?id , ?date)")
(exec h "insert into table1 values (?id , ?date)" `((id . 1) (date . ,(srfi19:current-date))))

So dbd-spgsql, dbd-jazmysql, dbd-jsqlite will no longer use SQL escape for parameterization going forward. They have been made available via planet.

Thank you Scott for discovering and reporting this issue.