Comments

damien tournoud’s picture

Status: Active » Needs review
StatusFileSize
new15.2 KB

Here.

One call in drupal_web_test_case.php can't be converted right now, because we don't support data subqueries on INSERT right now:

    db_query('INSERT INTO {registry} SELECT * FROM ' . $this->originalPrefix . 'registry');
    db_query('INSERT INTO {registry_file} SELECT * FROM ' . $this->originalPrefix . 'registry_file');

I suggest we go ahead and deal with the rest first.

damien tournoud’s picture

StatusFileSize
new61.52 KB

Now with every inline query converted.

I also removed a test:

-  /**
-   * ANSI standard allows for double quotes to escape field names.
-   */
-  function testQuotes() {
-    $result = db_query('SELECT "name" FROM {test} WHERE age = :age', array(':age' => 25));
-    $this->assertIdentical($result->fetchField(), 'John', t('ANSI field quoting works.'));
-  }

... for two reasons:

(1) We don't support any form of quoting in db_query(), so we shouldn't promote bad examples,
(2) It doesn't matter anyway, because PDO is taking care of quoting

dries’s picture

I reviewed this patch and it looks good. No comments. Just waiting for the test results from the test bot before committing this patch. Thanks Damien.

Status: Needs review » Needs work

The last submitted patch failed testing.

chx’s picture

damien tournoud’s picture

Status: Needs work » Needs review
StatusFileSize
new60.54 KB

Rerolled.

berdir’s picture

Status: Needs review » Needs work
+      ->fields(array(
+        'uid' => $node->uid,
+      ))

According to Crell, arrays with only a single value should be on the same line: #394484-3: DBTNG: Node module

+      $query = db_insert('role_permission')
+        ->fields(array(
+          'rid',
+          'permission',
+        ));

According to the documentation, this can be written on a single line, because it's only one method and the array just contains the values: http://drupal.org/node/310079

+        $query->values(array(
+          $role->rid,
+          $permission_string,
+        ));

But this should be array with keys, does this actually work ?

-    $this->assertEqual(db_query("SELECT COUNT(*) FROM {watchdog} WHERE type = 'system_test' AND message = 'hook_boot'")->fetchField(), $calls, t('hook_boot called with disabled cache.'));
-    $this->assertEqual(db_query("SELECT COUNT(*) FROM {watchdog} WHERE type = 'system_test' AND message = 'hook_exit'")->fetchField(), $calls, t('hook_exit called with disabled cache.'));
+    $this->assertEqual(db_query('SELECT COUNT(*) FROM {watchdog} WHERE type = :type AND message = :message', array(':type' => 'system_test', ':message' => 'hook_boot'))->fetchField(), $calls, t('hook_boot called with disabled cache.'));
+    $this->assertEqual(db_query('SELECT COUNT(*) FROM {watchdog} WHERE type = :type AND message = :message', array(':type' => 'system_test', ':message' => 'hook_exit'))->fetchField(), $calls, t('hook_exit called with disabled cache.'));

Just wondering if it is really necessary to pass in static args as parameters, I've never done that.

     $num_updated = db_update('test')->fields(array('name' => 'Tiffany'))->condition('id', 1)->execute();

You don't touch that yet, but that should be multi-line.

+    $r = db_query('SELECT * FROM {test_one_blob} WHERE id = %d', $id)->fetchAssoc();

You converted it to single quotes, but it still contains a %d param.

Most of the above things occur more than once in the patch, I've just picked an example. Also, you convert all queries to single quotes. It's not a bad thing, it just makes the patch huge :)

berdir’s picture

Status: Needs work » Needs review
StatusFileSize
new68.63 KB

Uh oh, that one got even bigger :)

Changed my own points in #7 and also fixed a few DBTNG Coding Style issues, mostly with chaining and arrays.

dries’s picture

Status: Needs review » Fixed

Committed to CVS HEAD. Thanks!

Status: Fixed » Closed (fixed)
Issue tags: -DBTNG Conversion

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