D6 upload.install creates {upload}, which is created as {file_revisions} in D5 install.

system_update_6022() is renaming file_revisions to upload, so there is the upload table created. Because file_revisions is a system table in D5 and upload is a module table in D6 (the data stored there is only used by upload module, the system itself stores less), upload module should recognize if there is an upload table and do not create that on install.

Patch with appropriate code comments.

Tested:

1.) Installing DRUPAL-5 CVS does not create an {upload} table.
2.) Switching codebase to DRUPAL-6 CVS and running update.php creates an {upload} table even though upload.module was never enabled.
3.) Enabling upload.module obviously tries to recreate said table.

Now checking for the system_update_N that does this.

...

Found it. #6022 is the culprit.

/**
 * Update files tables to associate files to a uid by default instead of a nid.
 * Rename file_revisions to upload since it should only be used by the upload
 * module used by upload to link files to nodes.
 */
function system_update_6022() {
   // ...
   db_rename_table($ret, 'file_revisions', 'upload');

So the problem can be explained like this:

Drupal 5 creates a {file_revision} table as part of its core schema. Along the road to Drupal 6 it was decided that this table is used by nobody but upload.module. Therefore it was renamed {upload} and removed from the core schema, only to be created when upload.module is enabled. This means that Drupal 5 users who never used upload.module have an obsolete {file_revisions} table, which becomes {upload} on upgrading to Drupal 6. upload.module, which was never installed, is unaware of this and attempts to recreate the table.

I presume that the renamed {upload} and the newly created {upload} have the same schema, but will make sure of that.

If so, we have two possibilities for fixing the upgrade path:

Delete obsolete table

Make the D5 -> D6 upgrade path (either #6022 or a new update hook) check for the schema version of upload.module. If it was never installed, assume {file_revisions}/{upload} to be obsolete and delete it. The table will then be recreated if upload.module is ever enabled.

Pro:

No obsolete tables in the schema

Contra:

Destructive. Might potentially destroy data if a contrib module happened to use this table (is this possible?)

Do not recreate table

Make upload.module check whether {upload} already exists, and only install it if it does not.

Pro:

All user data is safe.

Contra:

Overhead; undefined state of the Drupal database schema - we don't know for sure which core tables exist and which do not until upload.module is enabled.

Does that about summarize it?

Next time I'll know to be less verbose and faster on the submit button instead. =P

...

Are we sure that the upload table created in the update is empty? Then why was it created at all. I think those who wrote update 6022 preserved that table for some reason.

Indeed, I didn't watch the distinction between "this table's data is now only used by upload.module" and "this table was only ever used by upload.module". Preserving it is obviously the right choice.

Also, since file_revisions was technically a system table in Drupal 5, it does not guarantee that there was no data in it that upload module was not turned on (any other module could have used it).

The patch in #3 is not the perfect thing to do, because both system_update_6022() and system_update_6040() need to run to be in the state where upload module expects the system to be for it to be on that schema version. Unfortunately these updates were not tied to upload module, but written into the system update stream (due to a rule of thumb going around at that time).

Moving all those updates around now would complicate RC updates even more. Bleh. So #3 does what is required in situations except overly edge cases, when not all updates were run before upload module was enabled.

http://drupal.org/node/115267#comment-503623 says that contributed modules have in the past attempted to use the file_revisions (now renamed to upload) table, so we can't simply delete it if the upload module it not installed.

If we run into a non-core "upload" table during the 5.x --> 6.x update, the update will fail, so we should be able to safely assume that if a table named upload exists, it should be there because the update created it.

The patch #3 should be safe.

I tried to think of an edge case that would leave your site actually broken, but I could only imagine someone running enough updates to get the site functional, breaking off the update process, and enabling the upload.module. Still, if that happened, the site would remind the user to run the rest of the updates, which should get the upload table into its proper state...

The patch applies and worked (got rid of the MySQL error on enabling upload.module) with a fresh D5 install upgraded to D6. I'll leave it up to you whether a second review is needed.

All right, committed #3 then. RTBC for 7.x.

Committed to CVS HEAD. Thanks.