This brings bootstrap.inc up to DBTNG. All bootstrap tests pass, and the relevant system tests.

CommentFileSizeAuthor
#11 bootstrap.patch4.81 KBCrell
#8 bootstrap.patch4.71 KBCrell
#6 issue-313156.patch4.87 KBlilou
#4 issue-313156.patch4.86 KBlilou
dbtng-bootstrap.patch4.58 KBCrell

Comments

damien tournoud’s picture

damien tournoud commented

This is all sound.

Just one little thing:

     $result = db_query('SELECT * FROM {variable}');
-    while ($variable = db_fetch_object($result)) {
+    foreach ($result as $variable) {
       $variables[$variable->name] = unserialize($variable->value);
     }

This could become:

$variables = array_map('unserialize', db_query('SELECT name, variable FROM {variable}')->fetchAllKeyed());
Crell’s picture

Crell commented

Does array_map() preserve associative arrays? *checks PHP manual* Hey, it looks like it does! Sweet, I like array_map() even more now. :-) Unless webchick has an objection I'm in favor of using the array_map idiom in these cases.

The alternative is to have a fetchAllKeyed() parameter that is a callback, but I think that's pushing just too much functionality into the DB system when the array_map above works just as well and is easier to grok, too.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

array_map() looks *much* more legible. +1!

lilou’s picture

lilou commented
StatusFileSize
new issue-313156.patch4.86 KB

array_map() is more effective.

Crell’s picture

Crell commented

Spiffy, thanks lilou! Per http://drupal.org/node/313152#comment-1031542 , can you also break the db_delete() in variable_set() across multiple lines? If not, I'll get to it later tonight. Thanks!

lilou’s picture

lilou commented
StatusFileSize
new issue-313156.patch4.87 KB 
 function variable_del($name) {
   global $conf;
 
-  db_query("DELETE FROM {variable} WHERE name = '%s'", $name);
+  db_delete('variable')
+    ->condition('name', $name)
+    ->execute();
   cache_clear_all('variables', 'cache');
moshe weitzman’s picture

moshe weitzman commented

code looks good to me. nice work.

Crell’s picture

Crell commented
StatusFileSize
new bootstrap.patch4.71 KB

grumble grumble windows line breaks.

The patch in #6 also breaks the variable_init() query, so this version fixes it and the line breaks. All bootstrap tests pass.

dries’s picture

dries commented

Is this chunck correct?

 function registry_cache_path_files() {
     foreach ($used_code as $type => $names) {
-      $type_sql[] = "(name IN (" . db_placeholders($names, 'varchar') . ") AND type = '%s')";
-      $params = array_merge($params, $names);
-      $params[] = $type;
-    }
-    $res = db_query("SELECT DISTINCT filename FROM {registry} WHERE " . implode(' OR ', $type_sql), $params);
-    while ($row = db_fetch_object($res)) {
-      $files[] = $row->filename;
+      $select->condition('name', $names, 'IN');
     }

The AND type = '%s'" stuff seem to be missing?

Crell’s picture

Crell commented
Status: Needs review » Needs work

I believe it is, yes. At least it passes all tests. :-) One of the nice things about DBTNG is that a lot of the things you used to have to loop on are now consolidated into primitive actions. You're right, though, that it looks like the type part went missing. That should be just another condition call. I'll roll a new patch after I finish fighting with simpletest over another patch. :-)

Crell’s picture

Crell commented
Status: Needs work » Needs review
StatusFileSize
new bootstrap.patch4.81 KB

And back again, now with the type restored.

dries’s picture

dries commented
Status: Needs review » Fixed

Committed to CVS HEAD. Thanks.

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.

Crell’s picture

Crell commented
Issue tags: +DBTNG Conversion

Tagging.