API page: https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Extension...

I'm making this a major, since these docs should be canonical source of how to run a batched hook update, and the current code will crash people's servers.

    // This must be the first run. Initialize the sandbox.
    $sandbox['progress'] = 0;
    $sandbox['current_pk'] = 0;
    $sandbox['max'] = Database::getConnection()
      ->query('SELECT COUNT(myprimarykey) FROM {mytable1}')
      ->fetchField() - 1;
  }

// SNIP

  $sandbox['#finished'] = empty($sandbox['max']) ? 1 : $sandbox['progress'] / $sandbox['max'];

Suppose the count query gets 0 records. $sandbox['max'] is then set to 0 - 1 = -1.

$sandbox['#finished'] is then set to 0/-1 = 0, which tells the update system that this hook is not yet finished running. So the hook will be run again, with exactly the same result and will never terminate.

The -1 is incorrect and should be removed.

CommentFileSizeAuthor
#3 3051889-3.patch702 bytesidebr
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joachim created an issue. See original summary.

xjm’s picture

Priority: Major » Normal

Thanks @joachim!

Downgrading to normal since it doesn't meet the criteria for https://www.drupal.org/core/issue-priority#major-bug.

idebr’s picture

Status: Active » Needs review
FileSize
702 bytes

Attached patch removes the incorrect -1.

pandaski’s picture

Status: Needs review » Reviewed & tested by the community

Nice catch, RTBC to me

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Credited @joachim for filing the issue.

Committed and pushed d948f9cbac to 8.8.x and 8a41b70425 to 8.7.x. Thanks!

  • alexpott committed d948f9c on 8.8.x
    Issue #3051889 by idebr, joachim: $sandbox['max'] count in docs for...

  • alexpott committed 8a41b70 on 8.7.x
    Issue #3051889 by idebr, joachim: $sandbox['max'] count in docs for...
alexpott’s picture

Version: 8.8.x-dev » 8.7.x-dev

Status: Fixed » Closed (fixed)

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