Improving db_query() (was: [drupal-devel] Checking coding style of contributed modules)

[Gerhard Killesreiter]

On Sun, 20 Feb 2005, Gerhard Killesreiter wrote:

> Now we will get complaints about db queries that look like these:
>
> db_query("INSERT INTO {node} (". implode(", ", $k) .") VALUES(". implode(", ", $s) .")", $v);
>
> db_query("UPDATE {node} SET ". implode(', ', $q) ." WHERE nid = '$node->nid'", $v);
>
> The $node->nid in the last one is easy to fix, but the implodes for SET

Actually, we cannot do the simple change this to

db_query("UPDATE {node} SET ". implode(', ', $q) ." WHERE nid = %d", $v, $node->nid);

because if we have an array as first argument, the rest of the arguments
is discarded.

> or VALUES aren't unless we simply assume that all fields need to be
> updated/inserted. This would be possible in this case (and require some
> additional changes for the update case) but not in other cases where
> similar constructs are used.
>
> The question is: can we extend our db wrapper to support a syntax like

I've some ideas here, but while implementing them, I ran across the
following code in node_save:

    foreach ($node as $key => $value) {
      if (in_array((string) $key, $fields)) {
        $k[] = db_escape_string($key);
        $v[] = $value;
        $s[] = "'%s'";
      }
    }

That is, we treat each field as a string although there are numeric
fields in $fields (changed, uid, etc.). Why isn't that a problem? I'd at
least expect postgreSQL to choke at this. I'd appreciate some feedback.

Cheers,
	Gerhard

P.S.: Dries, there is some dead code in node_save:

    $keysfmt = implode(', ', $s);
    // We need to quote the placeholders for the values.
    $valsfmt = "'". implode("', '", $s) ."'";

No patch, CVS was down.