datamapper / do

DataObjects
147 stars 74 forks source link

DataObjects::Command#escape_sql is broken #29

Closed jeremyevans closed 12 years ago

jeremyevans commented 12 years ago

This method takes an array of arguments and attempts to interpolate the arguments into the stored sql string (@text) using a regular expression and gsub. It attempts to do this even if no arguments are given, which can break valid queries even if no arguments are used. There are other problems:

  1. It assumes that ? is the placeholder character. However, on PostgreSQL, ? is a valid operator character, and is used for some hstore operators. You can't execute queries with hstore operators using DataObjects, since it assumes the ? is a placeholder.
  2. Parsing SQL via regular expressions is broken and should never be done. The only way to reliably parse SQL is to do it via the database's libraries. Even for the same database, the parsing rules can differ depending on configuration. For example, on PostgreSQL, '\'?\'' could be either the string '?' or the expression '\' ? '\' (two strings with \ with the ? operator between them) depending on the standard_conforming_strings setting.

This method and the code that uses it probably needs to be redesigned so that it doesn't attempt to parse SQL. As a stop gap measure until that can be done, the method should just return the stored sql string without any changes if the args argument is empty.