Hi everyone,
I wrote a module with some new functionality for our website and now I'm doing a review, before sharing it - I hope it will be accepted by the community. While writing the code, I had a problem with the SQL queries and NULL values - so now I want to ask for help and feedback, before contacting the maintainers and add some inutil work on their schedule.
I found, that drupal function db_query has problems with NULL values in the queries. Perhaps I chose the wrong keywords for searching - but I couldn't find hints on how to solve this problems.
An example:
With the following code, in the database it inserted 0 instead of NULL - and broke the whole database insert, because the $status got 0 as well (It should be 'premium') ...:
if (empty($ts)) $ts = "NULL";
$query = "INSERT INTO {db_table} (nid, status ,ts ) VALUES ( %d, %d, %d)";
db_query($query, $node->nid, $status, $ts);
My solution was:
$query = "INSERT INTO {db_table} (nid, status ,ts ) VALUES ( %d, '$status', ";
$query .= empty($ts) ? "NULL" : $ts;
$query .= ")";
db_query($query, $node->nid);
Now, the coding standards tell me: Use placeholders because SQL-Injections will be filtered with them. But it seems to me, I can't use placeholders, if I want to enter NULL values in the database.
Did I do something wrong? Do I have to add some attribute, to make NULL possible with the db_query function? Is there another function for inserting NULL values? Or - is my code OK and this is the right way to do it?
I'd really appreciate your feedback
anschinsan
Comments
Just scanning the forums for
Just scanning the forums for a reason why all %d numbers are case to integers myself. Looking at the
_db_query_callback
you will see that it is simply not possible with db_query.Why on earth is NULL cast to an integer! This has been done since at least a version 4.7, so simply changing this now would possibly cause a cascading effect of bugs in other areas of the system.
Alan Davison
www.caignwebs.com.au
Alan Davison
You need to escape the
You need to escape the integer value to avoid any sort of SQL injection attack. Use db_escape_string or cast $ts to an int (best).
For example;
Also just noticed that in your code you are using empty($x). If you have done this through out your module, just use 0 for NULL. Both return a TRUE result via the empty function. See http://www.php.net/manual/en/types.comparisons.php for a comparison of the different types of php type functions.
As per my own problem, I've defined up a flag that means NULL. So when I get 0 from the db, I know that it means NULL. If my db table field wasn't unsigned, I'd probably would have used -1 for the flag.
Alan Davison
www.caignwebs.com.au
Alan Davison
my solution
I have changed db_query adding some new placeholders.
I rename it, in order to avoid any conflict.
when you need to insert NULL values, call my function instead of db_query(....)
the new placeholders for fields that could be NULL, are the upper case of actual ones.
Do not put quotation marks around %S.
...
I've done it today, and It's NOT TESTED ENOUGH.
I'll thank any bug reported.
THE CODE:
PD: I know my english sucks, but I've tried to do my best.
another solution
Here is another, simpler solution:
1) Set the database field to be NULL per default
2) Don't add the field to the query, when NULL
ad 1:
When you define your schema in hook_schema, define the field with something like this:
'ts' => array(
'type' => 'int',
'not null' => TRUE,
'default' => NULL,
),
ad 2:
like this:
if (empty($ts)) {
db_query("INSERT INTO {db_table} (nid, status) VALUES ( %d, %d)", $node->nid, $status);
else {
db_query("INSERT INTO {db_table} (nid, status, ts) VALUES ( %d, %d, %d)", $node->nid, $status, $ts);
}
As far as I'm aware this does
As far as I'm aware this does not work when updating the database with a NULL value, only the first time.