See http://drupal.org/node/256531 for the D5 version of this... CCK field data for a node is deleted using the nid column, which is not indexed - this has a large impact on performance if deleting multiple nodes with multiple CCK fields.

Thanks.

CommentFileSizeAuthor
#8 content.install.patch2.31 KBjaydub
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

yched’s picture

Priority: Normal » Critical

Hm, true. Let's fix this in D6 first and see about a backport.
I marked #256531: node_delete is very slow because of content_field() deletes using the unindexed 'nid' column as duplicate.

yched’s picture

Priority: Critical » Normal

Fixed in both D5 and D6 branches, including an update.

If the D5 update has already run, and the site is later updated to D6, the D6 update will raise errors because the index already exists. Ideas welcome on the best way to avoid that (our db API has no db_index_exists() function).
Leaving open for this.

yettyn’s picture

when running update.php on latest cck-6.x-2.0-dev I get an error:

Drupal database update

and I wonder where that &$sandbox argument comes from, as I cannot find any referense to it in the api? Anyhow it appear to only find the db name (acmain) and the table prefix (dru_) but not the table it self. Now I am not sure which table that might be, the index is supposed to be added on?

yched’s picture

Hm, strange...

Could you please add the following line in content_update_6004() :

      (...)
      else {
        $table = _content_tablename($field['type_name'], CONTENT_DB_STORAGE_PER_CONTENT_TYPE);
      }
      drupal_set_message($table); // <--- Add this.
      $sandbox['tables'][$table] = $table;
    }
    $sandbox['count'] = count($sandbox['tables']);
  }

  // One pass : add index on one table.
  (...)

and re-run update content_update_6004() (please be sure to make a db backup first)
Then please report the messages you get.

About $context : it's a construct that allows muti-pass updates. The data stored in there persists across multiple calls of the update function.

yettyn’s picture

Sorry for not following up on this before, but I ended up reinstalling everything as I build a completely new site and currently haven't run across any problems.

yched’s picture

Status: Active » Fixed

I added a variable_set to prevent errors when update is run both in D5 (update_1009) and in D6 (update_6004). Should fix most cases. People that update to D6 from a CCK-5.x-1.x-dev from between July 27 and now will see 'index already exists' errors, but that should be few people.

Anonymous’s picture

Status: Fixed » Closed (fixed)

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

jaydub’s picture

Version: 6.x-2.0-rc4 » 5.x-1.10
Status: Closed (fixed) » Active
FileSize
2.31 KB

The Drupal 5 implementation of this seems to be unchanged from the D6 version meaning that it's implemented assuming the Batch API is in place. As a result, the update only actually upgrades a single table with the new index on nid.

  // One pass : add index on one table.
  $table = array_shift($_SESSION['content_update_1009_tables']);
  if (db_table_exists($table)) {
    switch ($GLOBALS['db_type']) {
      case 'mysql':
      case 'mysqli':
        $ret[] = update_sql("ALTER TABLE {". $table ."} ADD INDEX (nid)");
        break;
      case 'pgsql':
        $ret[] = update_sql("CREATE INDEX {". $table ."}_nid_idx ON {". $table ."}(nid)");
        break;
    }
  }
  $ret['#finished'] = 1 - count($_SESSION['content_update_1009_tables']) / $_SESSION['content_update_1009_count'];

Correct me if I'm wrong but the array_shift is only going to pop off a single table and as a result only a single table gets the index. $ret['#finished'] only works when the Batch API is present.

I've attached a patch which adds a new update function that reruns the index add code this time w/o the Batch API assumption.

yched’s picture

Status: Active » Closed (fixed)

#finished existed before batch API. In fact, batch API is 'only' an abstraction of the 'progressive execution' engine that existed in update.php and was limited to updates.
The introduction of batch API as a more generic background engine did not change the syntax and conventions for hook_update_N() functions, so, unless you have facts proving otherwise, that update is expected to work in D5.