API page: http://api.drupal.org/api/drupal/includes--database--database.inc/functi...

Describe the problem you have found:

Again there not described correctly what will be returned when nothing is found. I had this construct:

$result = db_select('flag_content', 'fc')
->fields('fc')
->condition('content_type', 'node')
->condition('content_id', $nid)
->condition('uid', $uid)
->execute()
->fetchAssoc()
;

and got a bool(false) returned. This is not mentioned in the documentation of fetchAssoc();

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jhodgdon’s picture

Title: Documentation problem with DatabaseStatementInterface::fetchAssoc » DatabaseStatementInterface::fetchAssoc - document what empty return value is
Version: 7.0 » 8.x-dev
Status: Active » Postponed (maintainer needs more info)
Issue tags: +Needs backport to D7

Are you saying that if there are no results, you get boolean FALSE returned, not an empty array?

Agreed that this function's doc should tell what happens if the query returns an empty result set. Needs to be fixed in d8 first and ported back to d7.

ro-no-lo’s picture

@jhodgdon: try this in any module:

  var_dump(db_select('node', 'n')
    ->fields('n')
    ->condition('nid', 10000) // a node number which does not exists
    ->execute()
    ->fetchAssoc('nid')
  );

I get an bool(false) returned. [Also with ->fetchAssoc()]

jhodgdon’s picture

Status: Postponed (maintainer needs more info) » Active
Issue tags: +Novice

Thanks! I traced this down -- fetchAssoc() is basically a wrapper for PDOStatement::fetch(), which is documented to return FALSE on failure (which I assume means including if there is nothing there):
http://www.php.net/manual/en/pdostatement.fetch.php

So we should add that to the doc for fetchAssoc(). Thanks for reporting this! Good project for a novice doc contributor...

droplet’s picture

accepts parameters fetch* methods are return ARRAY. (personal views)

jhodgdon’s picture

droplet: I have no idea what you are trying to say in that comment. Can you please clarify?

droplet’s picture

@jhodgdon,
forgot it, just found that is not true.

these will return array() when nothing is found
DatabaseStatementBase::fetchAllAssoc
DatabaseStatementBase::fetchAllKeyed
PDOStatement->fetchAll

needs update all of them. :)

jhodgdon’s picture

I don't think that is correct. I think they will all return FALSE when nothing is found -- see original report above and comments 1,2,3

droplet’s picture

#1~3 is right. fetchAssoc is return FALSE

BUT following return empty array()
DatabaseStatementBase::fetchAllAssoc
DatabaseStatementBase::fetchAllKeyed
PDOStatement->fetchAll

ref:
http://api.drupal.org/api/drupal/includes--database--database.inc/functi...

jhodgdon’s picture

Title: DatabaseStatementInterface::fetchAssoc - document what empty return value is » DatabaseStatementInterface::fetch* - document what empty return value is

Right - I was confused between the fetch* and fetchAll* functions.

OK, so we need to document that:
fetch() [well, we don't document that -- it is PDOStatement], fetchAssoc() -- return FALSE if there is nothing there
fetchAllAssoc(), fetchAllKeyed() - return an empty array if there is nothing there

I'm not sure about what fetchAll() [which is a PDOStatement method, so we don't have to document it anyway - http://php.net/manual/en/pdostatement.fetchall.php] or fetchCol() do. The php site doesn't actually say what fetch() does, and fetchCol() just calls fetchAll(). So if someone wants to test out fetchAll() and fetchCol() that would be helpful.

I'm also not sure about fetchField(), which is a wrapper for http://us2.php.net/manual/en/pdostatement.fetchcolumn.php -- which also has no documentation for what the empty return value is, so if someone wants to investigate, that would also be helpful.

droplet’s picture

okay. I do a real test.

  $q = db_select('node', 'n')->fields('n')->range(0,2);
 
  dpm(gettype($q->execute()->fetchAllAssoc('nid')), 'fetchAllAssoc');
  dpm(gettype($q->execute()->fetchAllKeyed()), 'fetchAllKeyed');
  dpm(gettype($q->execute()->fetchAssoc()), 'fetchAssoc');
  dpm(gettype($q->execute()->fetchCol()), 'fetchCol');
  dpm(gettype($q->execute()->fetchField()), 'fetchField');
  dpm(gettype($q->execute()->rowCount()), 'rowCount');

return:

fetchAllAssoc => array

fetchAllKeyed => array

fetchAssoc => array

fetchCol => array

fetchField => string

rowCount => integer

NO RESULT:

  $q = db_select('node', 'n')->fields('n')->range(0,2)->condition('nid', 10000);
 
  dpm(gettype($q->execute()->fetchAllAssoc('nid')), 'fetchAllAssoc');
  dpm(gettype($q->execute()->fetchAllKeyed()), 'fetchAllKeyed');
  dpm(gettype($q->execute()->fetchAssoc()), 'fetchAssoc');
  dpm(gettype($q->execute()->fetchCol()), 'fetchCol');
  dpm(gettype($q->execute()->fetchField()), 'fetchField');
  dpm(gettype($q->execute()->rowCount()), 'rowCount');
  

return:
fetchAllAssoc => array

fetchAllKeyed => array

fetchAssoc => boolean

fetchCol => array

fetchField => boolean

rowCount => integer

jhodgdon’s picture

Perfect, thanks!

So to be documented:
fetchAssoc(), fetchField() -- return FALSE if there is nothing there
fetchAllAssoc(), fetchAllKeyed(), fetchCol() - return an empty array if there is nothing there

*now* it's probably a good project for a novice. Just needs a patch!

droplet’s picture

Status: Active » Needs review
FileSize
808 bytes
jhodgdon’s picture

Status: Needs review » Needs work

Good try, but this patch has a couple of problems:
- Doesn't document all 5 functions listed in #11.
- I am not all that happy with the wording:

+ * A single field from the next record or FALSE is returned on failure.

Should be more like "A single field from the next record, or FALSE if there is no next record." I think that would be better?

kathyh’s picture

Assigned: Unassigned » kathyh
kathyh’s picture

Updated patch to document expected empty return values for fetchAssoc(), fetchField() fetchAllAssoc(), fetchAllKeyed() and fetchCol()

kathyh’s picture

Status: Needs work » Needs review
jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

Thanks very much -- that looks good to me! Should be D7/8.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.x. Thanks!

jhodgdon’s picture

Version: 8.x-dev » 7.x-dev
Status: Fixed » Reviewed & tested by the community

I guess it should still be backported to 7.x? Probably it will apply... I'll set version and then hit retest just in case.

jhodgdon’s picture

#15: 1167218-db_fetch_doc-02.patch queued for re-testing.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Oops. This is why I shouldn't commit patches before coffee. :) I committed/pushed to 7.x too. no idea why that didn't get to my fingers. :P

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