Image effects table created with incorrect table name in upgrade path

good catch. here's a patch

This still seems to be an issue when upgrading to Drupal 7.0-beta3, and subsequently editing default styles.
Is the patch yet to be committed?

Broken upgrade... can we at least make this major?

+++ modules/image/image.install	6 Oct 2010 18:37:11 -0000
@@ -170,6 +170,19 @@ function image_update_7000() {
+/**
+ * Fix the bad table possibly created from an earlier update.
+ */
+function image_update_7001() {
+  if (db_table_exists('image_effect')) {
+    db_drop_table('image_effect');

Doesn't this count as a HEAD to HEAD update, which we're not doing?

Powered by Dreditor.

Found 14 instances of this in image.module. I'm not sure if I should have, but I replaced them all with 'image_effect' and the big bad errors went away but changes to image style configurations don't seem to take. At this point I'd love to start over with a clean image.module file if there is one.

Also upgraded from d6 to d7 beta 3.

Found 14 instances of this in image.module. I'm not sure if I should have, but I replaced them all with 'image_effect' and the big bad errors went away but changes to image style configurations don't seem to take. At this point I'd love to start over with a clean image.module file if there is one.

Also upgraded from d6 to d7 beta 3.

Looks like image_update_7000() is intended to update the DB schema for people upgrading from 6.x to 7.x that have the image.module installed on their 6.x site. Performing a straight D6 to D7 upgrade without having had image.module installed works just fine.

We should fix the issue in image_update_7000() so that this update works for people that had the old image.module installed, I don't see any reason to add an additional update function to fix the possibly bad previous table name. As was mentioned in #3 this would be a HEAD to HEAD update and probably isn't necessary.

image_update_7000 handles upgrade from oldskool image module? Blimey. There's an upgrade path in that module too...

To be clear the code in image_update_7000 doesn't actually handle the upgrade from image module in D6 to the core image module in D7. What it does is ensure that the schema for the D7 version gets installed when a user already has/had the D6 version installed. Since a module's install hook is only run "once" it is possible to end up in a scenario where you've installed image in D6, upgrade to D7, then turn on the D7 image module and the module's hook_install doesn't get run. The code in image_update_7000 fixes that problem.

The attached patch fixes the table naming problem in image_update_7000, but does not add an additional update hook to rename it for user's who may have already run the update and gotten the wrong table name. I think this is the appropriate fix since once again we're not supporting HEAD to HEAD updates. Someone correct me if I'm wrong though and I can update the patch.

We do support HEAD to HEAD updates past beta, so this will need an extra update function.

Okay, here is a version that fixes image_update_7000(), and adds a image_update_7001() which checks to see if there is a misnamed {image_effect} table from a prior update and then renames it to {image_effects}.

The patch in #1 also still applies (with offset) and solves the problem as well incase that is a better solution. The difference is that that one moves creation of the {image_effects} table into 7001 and this one just renames the already created table.

In fact, this could probably be simplified even further and use just the code from this patch that renames the existing table and not have to much with 7000 at all. I'm not entirely clear on what best practices are here.

#10 looks good to me, except there's no need to link to this issue, we only do that if there's a tonne of background that would otherwise get lost.

Removed link in documentation.

Looks good. tested on an upgraded site w/ the bad table name.

Mmm, bummer but good catch. Committed the fix to CVS HEAD so it will be part of the next Drupal 7 release.