Core queries coding style cleanup for PDO development

patch file

@lilou: Seems a bit strange... Your error message is all about permission denied, but not programming error nor conflict... Would you mind to check those patched files permission, if they are able to open/read by your web server owner? E.g. are they have permission 644? I test the patch once again, and it is function for both MySQL and PostgreSQL ;-)

@mooffie: according to my personal research project progress, the correctly use of DB API can greatly improve cross database compatibility, even with PDO and Oracle supporting. For more detail, please refer to my recent blog post.

db_query() should use correctly, e.g. escape empty string '' with %s (http://drupal.org/node/194162).

I mark this into my personal research project issue.

Patch reroll via latest CVS HEAD. Test without ill effect. It should be able to handle within D6, in order to prepare for next generation development.

@birdmanx35: thanks for review and pointing out. Some changes are already fixed in other issue. Here is the updated version.

Thanks for suggestion. I search out most core queries, and replace (most) "!=" as "<>". I just search with following simple script, so may still miss some out:

find . | xargs fgrep -nH "db_query" | grep '!='
find . | xargs fgrep -nH "db_" | grep '!='
find . | xargs fgrep -nH "_query" | grep '!='

Since Drupal 7.x is now open for public development, this patch is reroll via latest CVS HEAD. No special change since #18.

P.S. In case of PDO, we will still need to clean up the '%s' syntax: change them into %s (with '') or else PDO placeholder replacement will not accept it. BTW, I will reactive this issue after the successful commit of this first stage clean up, as that will affect almost ALL core queries.

chx: totally agree with you. Since I clone these handling from ADOdb (where ADOdb is a OOP implementation), they are handled as driver object method, etc. If we will revamp our drivers as OOP in 7.x, it is a good idea for giving a simple ADOdb-clone. But before that, I would like to handle these simple abstraction as constant temporarily, and so we will able to introduce other database drivers into core without any difficulties, e.g. SQLite, Oracle and DB2. These abstraction are always need, in whatever form of implementation :-)

@chx: Hmmm... so answer it once again: we may use function to wrap the return string, we can use OOP as object method, we can use predefined constant, etc. I choose predefined constant right now since it is the most simple as strict forward solution. We may seems it as temporary and reroll to a better solution whenever it is suitable.

Or may I have your suggested answer? If you hope to reroll it as function callback style, I am please to do so :-)

The detail of '%s' stuff:

1. We can't use %% syntax because this is the requirement of PDO prepare statement (see Example#11 for more detail). This idea also applied to the replace of '%s' as %s.
2. We need %s for empty string since some database will handle this incorrectly, says MSSQL, DB2 and Oracle.

   In case of MSSQL, PHP driver will return a space ( ) for NULL value stored within database. This is a well know bug for PHP MSSQL driver, which due to the incorrect handling of MSSQL dll provided by Microsoft. In case of Oracle, empty string will seems as NULL in SQL level. This means Oracle will need IS NULL for normal != '' checking. The is a bug of Oracle by design.

   BTW, both of the above bug can be handled within database driver implementation internally. We can handle these edge case within db_escape_string(). So the use of %s for empty string is important, as this will trigger the call of db_escape_string() for further more special handling.

3. The second point is also apply to empty BLOB handling. Some database need to use EMPTY_BLOB() as placeholder during INSERT/UPDATE queries. We will then get its empty BLOB pointer, and feed in BLOB value. E.g. Oracle and DB2 both need this hook. So we will need to use %b for empty BLOB value, in order to trigger db_encode_blob() for special handling.

Wait! Up to this point, please forget those hidden and complicated reason, as that would belongs to the duty of database abstraction driver developers and designer. Normal developer only require to follow a simple guideline: always use placeholder correctly, for whatever case (even empty string). DB abstraction layer will handle all of the other crazy stuff :-)

SO D7 *may* without PDO? I don't really think so, and so the %% and '%s' must be clean up. Are they PDO related stuff?

Patch via CVS HEAD.