Other users are experiencing this same bug: drupal.org/node/1565384
When using ->fetchAll() method like this:

$query->fetchAll(PDO::FETCH_CLASS, 'my_class_name');

the second parameter ($fetch_column) is the class name of objects returned into result array.

Actual DatabaseStatementPrefetch.fetchAll() implementation does not work like this, and second parameter is simply ignored (it works only for PDO::FETCH_COLUMN mode).

File /includes/database/prefetch.inc, line 425:

  public function fetchAll($fetch_style = NULL, $fetch_column = NULL, $constructor_args = NULL) {
    $this->fetchStyle = isset($fetch_style) ? $fetch_style : $this->defaultFetchStyle;
    $this->fetchOptions = $this->defaultFetchOptions;
    if (isset($fetch_column)) {
      $this->fetchOptions['column'] = $fetch_column;
    }
   //...

should read:

  public function fetchAll($fetch_style = NULL, $fetch_column = NULL, $constructor_args = NULL) {
    $this->fetchStyle = isset($fetch_style) ? $fetch_style : $this->defaultFetchStyle;
    $this->fetchOptions = $this->defaultFetchOptions;
    if (isset($fetch_column)) {
      if ($fetch_style & PDO::FETCH_COLUMN) {
          $this->fetchOptions['column'] = $fetch_column;
      } else if ($fetch_style & PDO::FETCH_CLASS){
          $this->fetchOptions['class'] = $fetch_column;
      }
    }
   //...

This bug appeared on my side after installing the i18n_strings module, and it prevents nodes deletion.
Function i18n_string_load_multiple in i18n_string.inc(456) calls fetchAll with PDO::FETCH_CLASS parameter, and should return an array of 'i18n_string_object' classes, but returns stdClass instead.
I think this bug is related to Drupal core and not i18n project, because the latter calls ->fetchAll() correctly (but it doesn't work as expected):

// file /sites/all/modules/i18n/i18n_string/i18n_string.inc
return $query->execute()->fetchAll(PDO::FETCH_CLASS, 'i18n_string_object');

The attached patch fixes the bug.

Comments

Status: Needs review » Needs work

The last submitted patch, drupal_fetchAll_classname.patch, failed testing.

nicorac’s picture

StatusFileSize
new871 bytes

Patch format error, resubmitting...

nicorac’s picture

Version: 8.x-dev » 7.14
Priority: Major » Critical

Other user issue link: here

The last submitted patch, drupal_fetchAll_classname_001.patch, failed testing.

nicorac’s picture

Status: Needs work » Needs review
StatusFileSize
new875 bytes

Patch format error, resubmitting...

Status: Needs review » Needs work

The last submitted patch, drupal_fetchAll_classname_002.patch, failed testing.

damien tournoud’s picture

Version: 7.14 » 8.x-dev
Priority: Critical » Major

Apparently this is actually documented now... I'm pretty sure it wasn't when we first wrote this.

So we need:

  • Update this patch to Drupal 8 (bugs are fixed in the most current development version, then backported)
  • Update the docblock for DatabaseStatement::fetchAll()
  • Update the basic statement manipulation tests to cover those cases
nicorac’s picture

Version: 8.x-dev » 7.14
Priority: Major » Critical
Status: Needs work » Needs review
StatusFileSize
new872 bytes

Patch format error, resubmitting...

Status: Needs review » Needs work

The last submitted patch, drupal_fetchAll_classname_003.patch, failed testing.

nicorac’s picture

Status: Needs work » Needs review

Well, I'm full of it!
Can't get where the patch format error is:
--> Output: [fatal: patch fragment without header at line 3: @@ -423,13 +423,17 @@

Maybe someone can get it to work

aspilicious’s picture

Version: 7.14 » 8.x-dev
Priority: Critical » Major

1) Failing because: "Ensure the patch applies to the tip of the chosen code-base"
2) And this needs to be fixed in drupal 8 first...
3) Don't bump issues to ciritcal without any proper reason

aspilicious’s picture

Status: Needs review » Needs work

And we didn't do what Dalien told us to do in #7

nicorac’s picture

Status: Needs work » Needs review
StatusFileSize
new1.01 KB

Update this patch to Drupal 8 (bugs are fixed in the most current development version, then backported)

Fine, will send a patch against 8.x-dev.
But is this a suggestion or a must?
Since I'm required to select a version when sending the patch, I supposed that the testbot will test my patch against the version I choose. And that's confirmed by testbot failure log: "[06:43:04] Main branch [7.14] checkout [complete].".
Again, who's in charge for backporting?

Update the docblock for DatabaseStatement::fetchAll()

Actually v.7.14 and also 8.x-dev have no docblock at all for this function, so I don't think I'm the right person to start it from scratch (a maintainer is better).

Update the basic statement manipulation tests to cover those cases

I have no knowledge about Drupal test suite so I'm not able to create tests at all, sorry.

aspilicious’s picture

Well, yes we need (it's a must) to fix this first in drupal 8. We are al responsible for the backport, you me and the rest of the community. If you can't write the tests you can learn it or wait untill someone is going to write one.

If there are no tests this wont get committed soon.
If you want to push this you should try to look for existing tests

Update the basic statement manipulation tests to cover those cases

and try to add the failing cases.

damien tournoud’s picture

Actually v.7.14 and also 8.x-dev have no docblock at all for this function, so I don't think I'm the right person to start it from scratch (a maintainer is better).

We actually do, but it's in DatabaseStatementInterface, not DatabaseStatement.

nicorac’s picture

Version: 7.14 » 8.x-dev
Priority: Critical » Major

Can't find DatabaseStatementInterface definition, do you mean StatementInterface (which is actually the interface implemented by DatabaseStatement)?.
It seems that DatabaseStatementInterface was renamed and a lot of references left unchanged.

Anyway StatementInterface doesn't have FetchAll() defined.

damien tournoud’s picture

See core/lib/Drupal/Core/Database/StatementInterface.php, the definition of fetchAll() is there (but commented out for technical reasons).

nicorac’s picture

StatusFileSize
new2.14 KB

Added docbook documentation to StatementInterface.

Status: Needs review » Needs work

The last submitted patch, drupal_fetchAll_classname_8.x_005.patch, failed testing.

nicorac’s picture

Status: Needs work » Needs review
StatusFileSize
new2.46 KB

Changed parameter names of StatementInterface.fetchAll() method.

Status: Needs review » Needs work

The last submitted patch, drupal_fetchAll_classname_8.x_006.patch, failed testing.

nicorac’s picture

Status: Needs work » Needs review
StatusFileSize
new3.09 KB

Fixed fetchAll parameters to comply with PHP fetchAll() definition.
Changed both StatementInterface and its implementations.

catch’s picture

Tagging.

catch’s picture

bomoko’s picture

catch’s picture

That was re-opened because it covers more cases, marking this as duplicate then since we don't need two issues for this #1854752: Re-Add PDO::FETCH_PROPS_LATE to PDO::FETCH_CLASS.

chx’s picture

Status: Needs review » Closed (duplicate)

#26 suggests this (and I agree, I guess catch just forgot to actually update the status).

chx’s picture

Issue summary: View changes

fixed link url