Locale upgrade to D7 fails due to index added by i18nstrings module

First observation shows no problem with updating D6->D7 with the locale module installed. Also, there doesn't seem to be an index on {locales_source}.location anywhere.

Could you verify in your D6 database that there is actually an index on {locales_source}.location that is causing this error?

This is caused by i18n (again), which hijacks the location column instead of adding its own.

As a consequence, not critical and not a beta blocker.

Well, we do have to figure out a way to make it work, but it's not high on the priority list.

It looks like the index (textgroup_location) is added by the i18nstrings module (see http://drupal.org/node/563106#comment-2508064), so it's the responsiblity of this module to make sure that the index is updated properly. (possibly using update dependencies so that the index is dropped before locale_update_7000 is run, and recreated afterwards.

It's probably better not to change the schema of tables from another project.

Well, that's what happens when you leave your browser window open for too long :)

Marking this as a duplicate of #803380: locales_source.location index

Moving this one back home.

um what i18n cant fix this as there are databases in the wild with this index that fail. how do you imagine enforcing 'em to upgrade to latest i18n first? the dependency system cant handle this.

Not sure what to do here.

Either just add a special case for i18n.module and check if the index exists and if yes, remove it. But I don't really like that. On the other side, some sort of generic solution that automatically removes indexes when a column is changed to blob sounds relatively complex.

I think we should drop this hijacked index in #803380: locales_source.location index
But I see no response from i18n maintainer about purpose of this index

I'm not sure why this issue is against core, #803380 should be fixed against i18n, then users of that module required to upgrade to the latest version before upgrading core (as happened with D5-6 CCK and many other modules), if people really want to fix this in core, then a db_index_exists(), then dropping it, seems as good as we can do.

Your problem is that even if i18n is removed from the system that index stays there. This is something we need to solve alas.

chx suggested that we re-create the table from scratch, then do a INSERT FROM SELECT to load the data and then remove the old table to get rid of any indexes added by modules or even manually and have a clean state again...

Here's a patch for core, untested. Downgrading from critical though.

We are not going to add special cases for each contrib module modifying core tables out there.

I think catch's patch is really simple and will work fine, I would be happy with that. Besides, for i18n d7 version we may be using a different field, like 'context' or a custom one to store that information and index on that.

However, for a generic good fix of these issues (contrib module clashing with core upgrade), we could provide a hook_upgrade_drupal7, to be into the install file (6.x, 7.x) for all (installed, not enabled) contrib modules that need to do some clean up or to preserve some data before D7 upgrade happens.

The main problem as I see it is that normal upgrade process (disable all contrib, upgrade core...) is that contrib modules never get a chance to fix these kind of issues by themselves and we cannot pretend to fix with core update scripts all the possible issues in our 6000 contrib modules.

Here's some patch to give modules a chance to prepare for drupal7 upgrade.

The module can include a mymodule.upgrade.drupal7.inc with both D6 and D7 versions

Oops, posting a proper patch

This is the good one

I agree with Damien, this should be an update in the i18n module, people will have to upgrade to latest D6 core before upgrading to 7, they'll need to add i18n to that as well.

Still an issue and cannot see it addressed in the i18n project. Cross posted #931534: i18n causing Drupal 7 upgrade problems

Edge-case, but I think Drupal core should handle this, since i18n is a very common contributed module for most non-English sites:

The contributed i18nstrings module of the i18n project adds a 'textgroup_location' index to {locales_source}, which prevents locale_update_7000 from being executed:

locale module
Update #7000

Failed: PDOException: SQLSTATE[42000]: Syntax error or access violation: 1170 BLOB/TEXT column 'location' used in key specification without a key length: ALTER TABLE {locales_source} CHANGE `location` `location` LONGTEXT DEFAULT NULL; Array ( ) in db_change_field() (line 2892 of includes\database\database.inc).

--

While I agree that core can't take any contributed module hacks into account, i18n is installed on lots of non-English sites. Hm.

I think that core maintainers should decide on this.

Well. It wouldn't be the first time we put hacks into core's upgrade path to deal with contrib:

Excerpt from http://api.drupal.org/api/function/system_update_6030/6:

  // Rename the old contrib actions table if it exists so the contrib version
  // of the module can do something with the old data.
  if (db_table_exists('actions')) {
    db_rename_table($ret, 'actions', 'actions_old_contrib');
  }

We do tend to only do this in the case where functionality is moving into core, however.

If there's truly no way to handle this from contrib, then I guess we'll have to do it here. It sets a really dangerous precedent, IMO, though. But in this particular case I think there's probably a 1:1 ratio between people using Locale and people using i18n in contrib, so I guess we can justify it...

This could be handled from i18n module - would require that a new release is produced before D7 comes out, and that users are required to update to that in D6 before updating to D7.

@catch this is a really edge-case, to upgrade D6->D7 i18n module user need upgrade core first but cant.

The issue is:

- i18n has no working D7 release yet. I tried to use what is called 7.x, but that's 6.x code. Generalizing a bit: Users have to wait for an optional contrib module to make core's upgrade work.

- i18n needs that index for performance reasons in D6. Serious slowdown otherwise. The index must be removed "right before" the D7 core upgrade is attempted, but not any earlier or later.

Having just hacked my way through the update system for Upgrade Assist, the only possible way that *might* work may be to put the following into both a new official release of i18n for D6 *and* its D7 branch (regardless of whether it contains D6 or D7 code):

if (DRUPAL_CORE_COMPATIBILITY == '7.x') {
  function i18n_update_dependencies() {
	  // i18n update 6999 needs to run before Locale attempts to upgrade.
	  $dependencies['i18n'][6999] = array(
	    'locale' => 7000,
	  );
	  return $dependencies;
  }

  function i18n_update_6999() {
    if (db_index_exists('locales_source', 'textgroup_location')) {
      db_drop_index('locales_source', 'textgroup_location');
    }
  }
}

Looks like a hack. Is a hack. But it's possible that it may even work.

Slightly cleaner would be to introduce a hook_pre_upgrade(), invoked at the beginning of update_fix_compatibility(); i.e., after core manually prepared system tables, full bootstrap has been reached, $module.install files have been loaded (including D6), but *before* modules are checked for compatibility and are disabled in update_fix_compatibility().

The real problem here is that i18n is a poorly maintained module that hacked a core table without really thinking about the consequences. This index is only necessary because the data model of i18n has been poorly though of.

Let's get #27 in, and forget about this whole mess for good.

i18n is a bit of a mess indeed. I've committed this patch to CVS HEAD. I'm OK with a small hack like this given that it is in the upgrade path. Because it is in the upgrade path, we'll remove these hacks in future versions of Drupal anyway.