Problem/Motivation

Moduels with the contact module enabled may fail on updating to D7. contact_update_7001() and contact_update_7002()require the {role_permission} table to be already updated, but this requirement is currently not enforced. The dependency chain is:
contact_update_7002() =>
contact_update_7001() =>
system_update_7007() =>
system_update_7000() =>
user_update_7008() =>
user_update_7006()

Proposed resolution

Patch in #11 adds these dependencies to contact.module.

Remaining tasks

None.

User interface changes

None.

API changes

None.

Original report by @spongecat

Every time I try to update, it fails on #7007 and #7002. Sometimes in just hangs on last two updates. I'm getting really different results, starting every time from exactly the same point. I tested every single build since beta was released. I had one success with dev build from Oct. 10 on local MAMP. Any other attempt with any version, on linux or wamp just fails. Below are error messages.

The following updates returned messages

system module

Update #7007
Failed: PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '4-administer recommender' for key 'PRIMARY': INSERT INTO {role_permission} (rid, permission) VALUES <...snip...>) in system_update_7007() (line 1883 of C:\wamp\www\modules\system\system.install).
contact module

Update #7002
Failed: PDOException: SQLSTATE[42S22]: Column not found: 1054 Unknown column 'module' in 'where clause': SELECT 1 AS expression FROM {role_permission} role_permission WHERE ( (rid = :db_condition_placeholder_0) AND (permission = :db_condition_placeholder_1) AND (module = :db_condition_placeholder_2) ) FOR UPDATE; Array ( [:db_condition_placeholder_0] => 2 [:db_condition_placeholder_1] => access user contact forms [:db_condition_placeholder_2] => contact ) in contact_update_7002() (line 128 of C:\wamp\www\modules\contact\contact.install).
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

catch’s picture

Component: update system » user.module

Moving this to user module. Can you still reproduce?

graytoby’s picture

Not on beta 2. I'll set up test environment and I'll try latest dev again. As a side note, this only happened on linux and win systems, I had no problems on MAMP.

Stevel’s picture

Title: Update fails on #7007 and #7002 » Update fails on system #7007 and contact #7002
Status: Active » Needs review
FileSize
828 bytes

The error in system_update_7007() looks like a corrupt permission table in the drupal 6 install, where a permission appears twice for the same role. Would be easy to fix (by remembering what rid/permission combinations are already processed), but this seems like a case of garbage-in/garbage-out, so not sure it should be fixed.

contact_update_7002() error looks like a unusual but probably correct (according to current hook_update_dependencies) update ordering.

Added a patch for the contact update error.

Dave Reid’s picture

Component: user.module » contact.module

Correct component...

And this needs to depend on user_update_7006() not system_update_7007(). If anything user_update_7007() should depend on system_update_7007().

Starminder’s picture

Help....I am stuck exactly here, not sure how to move on get the rest of the updates to complete...

Starminder’s picture

Priority: Major » Critical

Still stuck here, no idea how to work with the patch file. Would love to get past the errors above. Thanks!

catch’s picture

This could go straight into D7 - there's no new update or anything, just the dependency. I haven't verified, but it looks like sites with contact module enabled may not be able to update to D7 at all, if that's the case I agree it's critical.

Instructions to deal with patch files are here http://drupal.org/patch/apply

Also in this case you can just copy the code into contact.install and remove the + from the beginning of each line, but it's worth taking the time to figure out how to apply a patch.

Starminder’s picture

thanks Catch, I agree i need to learn to do patches and will as soon as i get some of these fires put out.

I slammed the changes into the file (I think in the right place). When I ran update this one didn't come up anymore, but do have a message about some updates no going due to dependency issues.

Remaining core updates that aren't going in are:
taxonomy module

7005 - Migrate {taxonomy_term_node} table to field storage. @todo: This function can possibly be made much faster by wrapping a transaction around all the inserts.
7006 - Add {taxonomy_term_data}.format column.
7007 - Add index on {taxonomy_term_data}.name column to speed up taxonomy_get_term_by_name().
7008 - Change the weight columns to normal int.
7009 - Change {taxonomy_term_data}.format into varchar.
7010 - Change {taxonomy_index}.created to support signed int.

If I can find a way past the above I'll be in much better shape. I posted under taxonomy but no replies yet. Any ideas?

bfroehle’s picture

FileSize
826 bytes

From #73874-57: Normalize permissions table, as to why system_update_7007() is in system.install, not user.install.

Also, thinking a little more about the patch this morning, it seems that perhaps the update should be in system.install rather than user.install. Normally I'd want to keep core updates with the respective module, but if I recall correctly, system.install's updates are always run first. Thus, any other module updating its permissions could use the new, easier table structure for updates if the update is done there.

This isn't true, AFAIK, but oh well.

And this needs to depend on user_update_7006() not system_update_7007(). If anything user_update_7007() should depend on system_update_7007().

This is not possible, as system_update_7000() already depends on user_update_7008()! (See user_update_dependencies()).

Note that contact_update_7002() uses the {role_permission} table which finally gets populated in system_update_7007.

But the patch in #3 is STILL not right! As contact_update_7001 also requires the {role_permission} table to be properly populated!

I've added the following requirements:
* contact_update_7001() depends on system_update_7007()
* contact_update_7002() depends on user_update_7006()

(The second requirement here is vacuous, as we already have the dependency chain
contact_update_7002() => contact_update_7001() => system_update_7007() => system_update_7000() => user_update_7008() => user_update_7006() --- but for the sake of transparency I'm including it).

JoshOrndorff’s picture

This post is mostly to subscribe, but I thought I would also mention an issue that I opened over at #1133024: System Update #7018 Fails on Upgrade. That issue is mostly about a system update that failed, but as you can see I get stuck on contact_update_7002 as well. I'm not sure if that helps sort this out or not, but I figured it wouldn't hurt.

It sounds like if I disable and uninstall the contact module, upgrade to D7, and then re-enable the contact module, I can get around this issue. Is that correct?

catch’s picture

Issue tags: +D7 upgrade path

#11 looks good to me.

Either we need someone to test it manually and report back, or we need to find a way to reproduce the bug with simpletest and write a test for it.

I think manual testing and report back is enough for this since the update doesn't run at all without the patch - as opposed to it being a data loss issue or similar where it can be harder to detect regressions, would be good if some of the people reporting this issue could try the patch - see http://drupal.org/patch/apply for instructions if you've not used patches before.

carlos8f’s picture

subscribing

stefanwray’s picture

I had the same errors as spongecat - the originator of this thread. Failed on System Update #7007 and failed on Contact Update #7002.

I just applied the patch in #11.

This patch eliminated the Contact Update #7002 failure.

But I'm still getting:

system module

Update #7007
Failed: PDOException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '4-' for key 'PRIMARY': INSERT INTO {role_permission} (rid, permission) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1), (:db_insert_placeholder_2, :db_insert_placeholder_3), (:db_insert_placeholder_4, :db_insert_placeholder_5), (:db_insert_placeholder_6, :db_insert_placeholder_7), (:db_insert_placeholder_8, :db_insert_placeholder_9), (:db_insert_placeholder_10, :db_insert_placeholder_11), (:db_insert_placeholder_12, :db_insert_placeholder_13), (:db_insert_placeholder_14, :db_insert_placeholder_15), (:db_insert_placeholder_16, :db_insert_placeholder_17), (:db_insert_placeholder_18, :db_insert_placeholder_19), (:db_insert_placeholder_20, :db_insert_placeholder_21), (:db_insert_placeholder_22, :db_insert_placeholder_23), (:db_insert_placeholder_24, :db_insert_placeholder_25), (:db_insert_placeholder_26, :db_insert_placeholder_27), (:db_insert_placeholder_28, :db_insert_placeholder_29), (:db_insert_placeholder_30, :db_insert_placeholder_31), (:db_insert_placeholder_32,

. . .

[:db_insert_placeholder_6267] => delete own merci_audio_mic_lavalier content [:db_insert_placeholder_6268] => 41 [:db_insert_placeholder_6269] => [:db_insert_placeholder_6270] => 41 [:db_insert_placeholder_6271] => [:db_insert_placeholder_6272] => 41 [:db_insert_placeholder_6273] => [:db_insert_placeholder_6274] => 41 [:db_insert_placeholder_6275] => edit own merci_conference_room content [:db_insert_placeholder_6276] => 41 [:db_insert_placeholder_6277] => delete own merci_conference_room content [:db_insert_placeholder_6278] => 42 [:db_insert_placeholder_6279] => suspend MERCI access ) in system_update_7007() (line 1887 of /usr/local/share/drupal/v7/core/drupal-7.2/modules/system/system.install).

tlangston’s picture

subscribe

steinmb’s picture

@stefanwray did you start the migration all over again? To me it seems you are trying to continue from where it failed just with the patch installed, and that does normally not work.

stefanwray’s picture

@steinmb yes, i went back to the backed up version of the database and tried to re-install D7 again and got that same error

catch’s picture

@ stefanwray: you have over 6200 rows being inserted by this update function, that seems like a lot of permissions to me. Could you dump the copy of your Drupal 6 {permission} table and upload the contents here?

kreynen’s picture

Status: Needs review » Needs work

The system #7007 error was caused by duplicate permission strings in the D6 permission table in @stefanwray's case. The cause of the dups in D6 isn't exactly clear, but I wrote this PHP to help him find and remove the dups before the upgrade. Might be helpful if anyone else runs into this in the future. If you find '=> 2' or '=> 3' in the output, you can manually remove one the strings from the perms field of the permission table.

<?php

$result = db_query("SELECT * FROM {permission}");
while ($aperm = db_fetch_object($result)) {
  if ($aperm->perm) {
    $perm_array = explode(',', $aperm->perm);
    print '</p><hr><p><b>' . $aperm->pid . ' - pid of permission table</b></p><p>';
    print_r(array_count_values($perm_array));
  }
}
EvanDonovan’s picture

Status: Needs work » Needs review

Shouldn't the patch at #11 still be at "needs review"? Why bumped down to "needs work"?

tlangston’s picture

Thanks for the script but after running and finding no duplicates there...ran update and still failing for this error.

David_Rothstein’s picture

Title: Update fails on system #7007 and contact #7002 » Update fails on contact #7002

This issue is filed against the Contact module so let's focus it on that (which the patch in #11 already does).

Anything that touches system module update dependencies is part of a larger issue which we're working on at #717834: The dependencies declared in core's hook_update_dependencies() implementations aren't actually correct.

Anyway, #11 looks good to me, but it's hard to properly test without doing something about #717834: The dependencies declared in core's hook_update_dependencies() implementations aren't actually correct first, so maybe we should focus efforts there and then see if the Contact fix here still works.

xjm’s picture

Tagging issues not yet using summary template.

catch’s picture

#11: 939562-maybe.patch queued for re-testing.

catch’s picture

Status: Needs review » Reviewed & tested by the community

Still passes with the other patch in and the dependencies here are straightforward. I ran a 6-7 upgrade on a site with contact enabled but couldn't reproduce this. The worst this should do is enforce an execution order that already happens by accident, so rtbc.

xjm’s picture

Issue summary: View changes

Updated issue summary.

xjm’s picture

Summary added.

catch’s picture

#11: 939562-maybe.patch queued for re-testing.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Sounds good, let's try this. Committed and pushed to 7.x. Thanks!

Status: Fixed » Closed (fixed)
Issue tags: -D7 upgrade path

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

Anonymous’s picture

Issue summary: View changes

Updated issue summary.