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?