MySQL very happily handles quoted numbers (with the exception of LIMIT values), but iirc the SQL standard says numeric types shouldn't be quoted. I think db2 will return an error if you quote a number, for example. So to keep the db-specific code to a minimum, we may want to update that.
On Tue, Dec 4, 2012 at 1:52 AM, Daniel Kinzler <[email protected]> wrote: > Hi all! > > I recently found that it is less than clear how numbers should be > quoted/escaped > in SQL queries. Should DatabaseBase::addQuotes() be used, or rather just > inval(), to make sure it's really a number? What's the best practice? > > Looking at DatabaseBase::makeList(), it seems that addQuotes() is used on all > values, string or not. So, what does addQuotes() actually do? Does it always > add > quotes, turning the value into a string literal, or does it rather apply > whatever quoting/escaping is appropriate for the given data type? > > addQuotes' documentation sais: > > * If it's a string, adds quotes and backslashes > * Otherwise returns as-is > > That's a plain LIE. Here's the code: > > if ( $s === null ) { > return 'NULL'; > } else { > # This will also quote numeric values. This should be harmless, > # and protects against weird problems that occur when they really > # _are_ strings such as article titles and string->number->string > # conversion is not 1:1. > return "'" . $this->strencode( $s ) . "'"; > } > > So, it actually always returns a quoted string literal, unless $s is null. > > But is it true what the comment sais? Is it really always harmless to quote > numeric values? Will all database engines always magically convert them to > numbers before executing the query? If not, this may be causing table scans. > That would be bad - but I suppose someone would have noticed by now... > > So... at the very least, addQuotes' documentation needs fixing. And perhaps it > would be nice to have an additional method that only applies the appropriate > quoting, e.g. escapeValue or some such - that's how addQuotes seems to be > currently used, but that's not what it actually does... What do you think? > > > -- daniel > > > PS: There's more fun. The DatabaseMssql class overrides addQuotes to support > Blob object. For the case $s is a Blob object, this code is used: > > return "'" . $s->fetch( $s ) . "'"; > > The value is used raw, without any escaping. Looks like if there's a ' in the > blob, fun things will happen. Or am I missing something? > > > _______________________________________________ > Wikitech-l mailing list > [email protected] > https://lists.wikimedia.org/mailman/listinfo/wikitech-l _______________________________________________ Wikitech-l mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/wikitech-l
