SQL cleanup: block

minor question: should we change the handling of the following lines? seems we are now inserting a imploded integer values as string, and use '%s' for its handling. shouldn't we first implode a %d placeholder within SQL (inline), and further more using an array of integer values (plus something else) for substitution?

<?php
 @@ -379,7 +379,7 @@ function block_list($region) {
   static $blocks = array();
 
   if (!count($blocks)) {
-    $result = db_query(db_rewrite_sql("SELECT DISTINCT b.* FROM {blocks} b LEFT JOIN {blocks_roles} r ON b.module = r.module AND b.delta = r.delta WHERE b.theme = '%s' AND b.status = 1 AND (r.rid IN (%s) OR r.rid IS NULL) ORDER BY b.region, b.weight, b.module", 'b', 'bid'), $theme_key, implode(',', array_keys($user->roles)));
+    $result = db_query(db_rewrite_sql("SELECT DISTINCT b.* FROM {blocks} b LEFT JOIN {blocks_roles} r ON b.module = r.module AND b.delta = r.delta WHERE b.theme = '%s' AND b.status = %d AND (r.rid IN (%s) OR r.rid IS NULL) ORDER BY b.region, b.weight, b.module", 'b', 'bid'), $theme_key, 1, implode(',', array_keys($user->roles)));
     while ($block = db_fetch_object($result)) {
       if (!isset($blocks[$block->region])) {
         $blocks[$block->region] = array()
?>

@mooffie: first of all, a very simple idea: we did this abstraction by using %d|%f|%s|%b, and it is proved as meaningful for cross database compatible (before we ship to PHP5.x PDO). i guess we should work out a good example within out core implementation, and so for our contribute developers.

more over, writing our query in correct syntax can greatly help about other database driver development (http://drupal.org/node/114774#db-query-standard). even though Oracle and DB2 driver may not become official supported within D6 life cycle (according to the dead-lock of CLOB handling: http://drupal.org/node/147947. since not much MySQL developers can understand how important of this, as MySQL provide TEXT (BTW, max. 4GB only, but Oracle CLOB can support for max. 128TB ;p)), keep this kindly consideration in mind should always be welcome.

P.S. i don't really think this as a critical issue: there are still a lot of other difficulties we need to overcome, before successfully implement our database abstraction layer with consideration for other databases (http://drupal.org/node/172541). we can simply left this issue behind in D6, but seems also no point to ignore it if we are able to handle them on time?

Sounds not necessary based on my latest research result, if we are able to enclose all table/column/constraint name with [] syntax. That should belongs to D7, so we don't need to fix this issue right now :)