Add primary key on {project_issue_comments}.cid (D6 regression)

Yup, sounds like a good idea. Moving to the right queue and giving a more accurate title/category... Feel free to provide a patch for this. Let's just leave this issue for the changes to project_issue_schema() and a DB update function to add the primary key to existing sites. We can do things like drupal_write_record() in followup issues + patches. Thanks.

(sorry, wishful thinking typo) ;)

d.o already has this field as the primary key for this table. ;) pretty trivial patch. not sure if there's a trivial way to test for if the table already has a primary key or not.

i'm confused. describe project_issue_comments; in my local project install shows that cid is already the primary key on that table -- lord knows that i didn't do that manually. but i don't see the primary key in the schema, so what gives??

Probably a regression during the D6 port.

Yup. In D5, we have this:

 db_query("CREATE TABLE IF NOT EXISTS {project_issue_comments} (
          nid int(11) default NULL,
          cid int(11) default NULL,
          rid int(11) NOT NULL default '0',
          component varchar(255) NOT NULL default '',
          category varchar(255) NOT NULL default '',
          priority int(11) NOT NULL default '0',
          assigned int(11) NOT NULL default '0',
          sid int(11) NOT NULL default '0',
          pid int(10) unsigned NOT NULL default '0',
          title varchar(255) NOT NULL,
          timestamp int(10) unsigned NOT NULL,
          comment_number int(10) NOT NULL default '0',
          PRIMARY KEY(cid),
          INDEX nid_timestamp (nid, timestamp),
          INDEX comment_number (comment_number)
        ) /*!40100 DEFAULT CHARACTER SET utf8 */");

There's no easy, portable way to find out if this table already has a primary key in D6 (and probably not D7, either). I don't feel like duplicating 100 lines of schema.module for different DBs to do this, so let's just tell the user they can ignore warnings and failures in this update if they happen. 3 options:

A) Show everything, including the failed query, but drupal_set_message() that everything's okay.

B) If the query fails, return success with a message they can ignore warnings. This way, at least the update isn't listed as a failure, but I fear people will see the red warning at the top of the page and miss the message they can ignore it.

C) If the query fails, drupal_set_message() that they can ignore the warning, and just say the update didn't run any queries at all.

Patches and screenies for each attached...

I think I prefer (C).

a) and c) are bad ideas, as the drupal_set_message() gets clobbered if the update batches. it sucks i know, but it's just the way it is -- i've already been bitten by that in another module i maintain. b) is the most reliable way to go...

Committed B to HEAD, though I wrapped the message in <strong> tags to be more visible.

Should deploy this for real, just so no one worries about the pending DB update or something...

Synced code to SVN and deployed revision 1660 on d.o.