This would be a new function that returns query results as a complete array.

Typically, I would see this being used in something like "SELECT id, value FROM {table}..." and would return an associative array where 'id => value' that could be used as an option list on a form or other cases where a simple list of values would be used.

It could also return 'id => array(value, value, value, ...)'.

For example, from one of my modules:

  $result = db_query("SELECT r.rid, r.name FROM {role} r JOIN {permission} p USING (rid) WHERE p.perm LIKE '%%answer question%%'");
  $role_list = array();
  while ($role = db_fetch_array($result)) {
    $role_list[$role['rid']] = $role['name'];
  }
...
  $form['roles'] = array(
    '#type' => 'select',
    '#options' => $role_list,
...

Could be shortened to:

  $form['roles'] = array(
    '#type' => 'select',
    '#options' => db_result_array(db_query("SELECT r.rid, r.name FROM {role} r JOIN {permission} p USING (rid) WHERE p.perm LIKE '%%answer question%%'")),
...


There are examples of this type of processing all over core; for (a probably poor) example, from "user_admin_perm":

  if (is_numeric($rid)) {
    $result = db_query('SELECT rid, name FROM {role} r WHERE r.rid = %d ORDER BY name', $rid);
  }
  else {
    $result = db_query('SELECT rid, name FROM {role} ORDER BY name');
  }

  $role_names = array();
  while ($role = db_fetch_object($result)) {
    $role_names[$role->rid] = $role->name;
  }

Could be shortened to:

  if (is_numeric($rid)) {
    $query = db_query('SELECT rid, name FROM {role} r WHERE r.rid = %d ORDER BY name', $rid);
  }
  else {
    $result = db_query('SELECT rid, name FROM {role} ORDER BY name');
  }

  $role_names = db_result_array($result);

I've attached patches based on both 5.5 and 6-rc1. I have tested with MySql 5.0. I don't have mysqli or postgres, but the functions are so similar (according to the php manual) that they should work exactly the same.

CommentFileSizeAuthor
#15 db_result_array_6.patch3.99 KBnancydru
#15 db_result_array_5.patch3.99 KBnancydru
db_result_array_6.patch3.99 KBnancydru
db_result_array_5.patch3.99 KBnancydru

Comments

kbahey’s picture

kbahey commented

Nancy

I am in favor of such a patch, but would like to put a safe guard in it.

Suppose someone writes a module that uses it, tests it and it works fine. Then someone downloads the module, but has a huge data set, as opposed to the smaller one that was tested. Now, these queries will because of memory exhaustion. Worse, if they do not fail, but use too much memory causing all sort of interesting issues.

So, I propose putting a LIMIT in the query with a reasonable default for it (so your syntax remains exactly the same), but can be overridden/increased if needed (with an extra argument to the query).

Crell’s picture

Crell commented
Status: Needs review » Closed (won't fix)

If you're using buffered queries, which is the default for both ext/mysql and ext/mysqli, then PHP is already pulling in the entire data set to memory. So doing it this way shouldn't have any additional cost over doing it manually, since it's the same code (at the C level). Using mysqlnd would have a memory improvement in those scenarios, but that's completely engine-space.

That said, the planned move to PDO in Drupal 7 should make these obsolete as PDO already has a ->fetchAll() method. See example #3.

So since this is too late for Drupal 6 and would be redundant for Drupal 7, I am marking won't fix. Sorry.

nancydru’s picture

nancydru
she/her/hers
Primary language English
Location Boston
commented

Thanks, Larry. That's fine. As you can see I opened this against D7 anyway. I may still code around it to simplify my modules.

I re-read http://drupal.org/gophp5 and don't see any mention of PDO in it. IMHO, it might be a good idea, starting with D6, to add a line to the status report that indicates whether PDO is available, since it is optional even in php5.2. I just checked my host's installation (5.1.6) and they do not have PDO enabled. I checked my DevSideNet installation on my PC (5.2.0) and PDO is not enabled in it. And even the new hosting service that I'm about to move to shows '--disable-pdo'. [And, BTW, I got my new host to register as a PHP5 provider as that article mentioned.]

If we are going to need PDO, we need to not only remind people to push for PHP5, but PDO as well. And the time to start that is now.

Crell’s picture

Crell commented

PDO is optional but part of the default PHP 5 package. It can be disabled, but then the same is true of, for instance, PCRE. I believe chx did a brief informal survey a while back that suggested that assuming PHP 5.2 => PDO was reasonably safe. ext/mysql and ext/mysqli are also optional as of PHP 5.0, and require extra work to enable, not extra work to disable.

Although, to be honest, a host that would go out of their way to disable PDO on PHP 5.2 is one I'd want to avoid like the plague. That sounds very odd to me.

How do you propose we emphasize PDO as well as PHP 5?

Also, have a look at the helpers module. It's woefully unmaintained, but it includes a 4.7-targeted db_helpers.module that includes various useful db_* functions. You can just rename it to db_helpers.inc and include it from one of your own modules (or upgrade helpers itself). I find the functions there quite useful, and wrote a few of them myself.

nancydru’s picture

nancydru
she/her/hers
Primary language English
Location Boston
commented

I'll take a look at that module. Thanks.

This is a relatively new host, and I suspect that as soon as I suggest I want to use PDO, they will enable it.

As for promoting php 5, a start would be to include a short blurb on PDO in the existing post and then change the authoring date to bump it back up to the top again. It has been a while since anyone has seen it (I had to go way down the list to find it again). Once D6 is out (well, okay maybe 6.1), then make it sticky.

nancydru’s picture

nancydru
she/her/hers
Primary language English
Location Boston
commented
Status: Closed (won't fix) » Active

I don't know much about mysqlnd (and especially regarding Drupal), but it does not appear to be installed on any of my hosts. In terms of performance, it looks promising though.

... the planned move to PDO in Drupal 7 should make these obsolete ...

Okay, I've been playing with PDO and am so far unimpressed with the coding.

What does seem apparent, however, is that it does NOT do what my suggested code does. [ I'm going to assume that Drupal will take care of the creation and dropping of the database handler for me, as it does with the current connections.]

For example:

  $sql = 'SELECT rid, name FROM role';

  $sth = $dbh->query($sql);
  $x = $sth->fetchall(PDO::FETCH_NUM);
  print_r ($x);

Produces:

Array ( [0] => Array ( [0] => 1 [1] => anonymous user ) [1] => Array ( [0] => 2 [1] => authenticated user ) [2] => Array ( [0] => 3 [1] => administrator ) [3] => Array ( [0] => 7 [1] => expert ) )

And any other "fetch_style" seems even farther from what I was looking for here. The default even has the data duplicated in the resulting array.

Whereas: [and also notice ONE line of code, compared to three]

  print_r (db_result_array(db_query($sql)));

Produces:

Array ( [1] => anonymous user [2] => authenticated user [3] => administrator [7] => expert )

And this array is ready to be used, for example, as an option list in a form, where the above array is useless without further processing. I don't see, in the php manual, any way to change this behavior.

I must, therefore, respectfully request that this suggestion be reconsidered.

Crell’s picture

Crell commented

From the fetchAll() manual page, at the bottom:


$sth = $dbh->prepare("SELECT name, colour FROM fruit");
$sth->execute();

/* Group values by the first column */
var_dump($sth->fetchAll(PDO::FETCH_COLUMN|PDO::FETCH_GROUP));

array(3) {
  ["apple"]=>
  array(2) {
    [0]=>
    string(5) "green"
    [1]=>
    string(3) "red"
  }
  ["pear"]=>
  array(2) {
    [0]=>
    string(5) "green"
    [1]=>
    string(6) "yellow"
  }
  ["watermelon"]=>
  array(1) {
    [0]=>
    string(5) "green"
  }
}

That's what you're after, isn't it?

nancydru’s picture

nancydru
she/her/hers
Primary language English
Location Boston
commented

Close, but no cigar. That list could not be used as an option list. That result could be useful in other circumstances, but not what I was trying to achieve here.

Array ( [1] => anonymous user
        [2] => authenticated user
        [3] => administrator
        [7] => expert )

Your example produces:

Array ( [1] => Array ( [0] => anonymous user )
 [2] => Array ( [0] => authenticated user )
 [3] => Array ( [0] => administrator )
 [7] => Array ( [0] => expert ) )
Crell’s picture

Crell commented

Ah, I see what you mean. Yes, that is different. I agree it should be added to the API, though. I'll add it to my todo list for the new database code. (See my sandbox for the current draft.) Or feel free to submit a new method for that, too. :-)

nancydru’s picture

nancydru
she/her/hers
Primary language English
Location Boston
commented

Wonderful, thank you. I don't know how you're implementing PDO; do I also need to work out a patch to do this with PDO?

Do you know what the "official" status of mysqlnd is (it was unclear to me in the reference I read)? Would I need another patch for that or would the mysqli patch work?

Crell’s picture

Crell commented

mysqlnd is an engine replacement for libmysql. It lives under mysql/mysqli/PDO, and should have no impact on user-space code at all. The official status, last I heard, is that it can be compiled into 5.2 with a patch and will be in PHP 5.3 by default, but maybe not enabled by default.

The PDO rewrite is being done all OOP. Such a feature would just be a new method on our statement subclass, next to the others in that class. Look at fetchAllAssoc() in database.inc for a similar skeleton.

nancydru’s picture

nancydru
she/her/hers
Primary language English
Location Boston
commented
Status: Active » Needs review

setting back to CNR

nancydru’s picture

nancydru
she/her/hers
Primary language English
Location Boston
commented

This feature will be backported to 5.x and 6.x in the Helpers package, see http://drupal.org/node/216386.

Oh, and darn you, Crell, for getting me involved with another module! Helpers is now maintained.

Crell’s picture

Crell commented

Haha! Excellent. :-)

nancydru’s picture

nancydru
she/her/hers
Primary language English
Location Boston
commented
StatusFileSize
new db_result_array_5.patch3.99 KB
new db_result_array_6.patch3.99 KB

I found a missing "i" in the mysqli code. Interesting that it was not a problem with 5.x but caused an error in 6.x.

Crell’s picture

Crell commented
Version: 7.x-dev » 6.x-dev
Status: Needs review » Fixed

The DatabaseStatement class in the new database layer includes this functionality. Marking this as "won't fix", since that's how we're handling backports, even though it "is fixed".

I don't know where we're tracking backports... If there is a central list somewhere, can someone add this? :-)

Crell’s picture

Crell commented
Status: Fixed » Closed (won't fix)
nancydru’s picture

nancydru
she/her/hers
Primary language English
Location Boston
commented

I didn't, and don't expect this to be back-ported.