Last Comment Bug 1196092 - Switch logincookies primary key to auto_incremented id, make cookie a secondary UNIQUE key
: Switch logincookies primary key to auto_incremented id, make cookie a seconda...
Status: RESOLVED FIXED
:
Product: bugzilla.mozilla.org
Classification: Other
Component: General (show other bugs)
: Production
: Unspecified Unspecified
-- normal (vote)
: ---
Assigned To: Dylan Hardison [:dylan]
:
:
Mentors:
Depends on:
Blocks: 1192687
  Show dependency treegraph
 
Reported: 2015-08-18 20:24 PDT by Dylan Hardison [:dylan]
Modified: 2015-08-24 01:18 PDT (History)
4 users (show)
See Also:
Due Date:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
1196092_5.patch (2.38 KB, patch)
2015-08-20 20:27 PDT, Dylan Hardison [:dylan]
glob: review-
Details | Diff | Splinter Review
1196092_6.patch (2.14 KB, patch)
2015-08-21 12:21 PDT, Dylan Hardison [:dylan]
glob: review+
Details | Diff | Splinter Review

Description User image Dylan Hardison [:dylan] 2015-08-18 20:24:42 PDT
This is a required change for bug 1192687, as natural primary keys (such as "cookie") are very difficult to work with, especially when they are secrets.
Comment 1 User image Dylan Hardison [:dylan] 2015-08-20 20:27:16 PDT
Created attachment 8650840 [details] [diff] [review]
1196092_5.patch

This was remarkably hard to get right. bz_column_info_real() is used because bz_column_info() returns our fake, shadow schema...
Comment 2 User image Byron Jones ‹:glob› 2015-08-20 21:16:41 PDT
Comment on attachment 8650840 [details] [diff] [review]
1196092_5.patch

Review of attachment 8650840 [details] [diff] [review]:
-----------------------------------------------------------------

Updating column cookie in table logincookies ...
Old: varchar(22) PRIMARY KEY NOT NULL
New: varchar(22) NOT NULL
DBD::mysql::db do failed: Can't DROP 'PRIMARY'; check that column/key exists [for Statement "ALTER TABLE logincookies DROP PRIMARY KEY"] at Bugzilla/DB.pm line 760.
Comment 3 User image Dylan Hardison [:dylan] 2015-08-21 12:21:45 PDT
Created attachment 8651196 [details] [diff] [review]
1196092_6.patch

It would appear I underestimated our DB schema layer. It can actually drop the primary key itself.
Comment 4 User image Byron Jones ‹:glob› 2015-08-23 22:40:32 PDT
Comment on attachment 8651196 [details] [diff] [review]
1196092_6.patch

Review of attachment 8651196 [details] [diff] [review]:
-----------------------------------------------------------------

this generates the same sql, which fails:

Updating column cookie in table logincookies ...
Old: varchar(22) PRIMARY KEY NOT NULL
New: varchar(22) NOT NULL
DBD::mysql::db do failed: Can't DROP 'PRIMARY'; check that column/key exists [for Statement "ALTER TABLE logincookies DROP PRIMARY KEY"] at Bugzilla/DB.pm line 760.
Comment 5 User image Byron Jones ‹:glob› 2015-08-23 22:52:44 PDT
Comment on attachment 8651196 [details] [diff] [review]
1196092_6.patch

Review of attachment 8651196 [details] [diff] [review]:
-----------------------------------------------------------------

ah, my bad.  for reasons that are unknown logincookies.cookie isn't a PK in the db i'm using, and a mysql quirk had it reporting it incorrectly ("A UNIQUE index may be displayed as PRI if it cannot contain NULL values and there is no PRIMARY KEY in the table").

r=glob

::: Bugzilla/Install/DB.pm
@@ +738,4 @@
>      $dbh->bz_alter_column('logincookies', 'cookie',
> +                            {TYPE => 'varchar(22)', NOTNULL => 1});
> +    $dbh->bz_add_index('logincookies', 'logincookies_cookie_idx',
> +                        {TYPE => 'UNIQUE', FIELDS => ['cookie']});

nit: this line and the previous {TYPE should line up with the '
Comment 6 User image Byron Jones ‹:glob› 2015-08-24 01:18:48 PDT
To ssh://gitolite3@git.mozilla.org/webtools/bmo/bugzilla.git
   0b05719..57f911c  master -> master

Note You need to log in before you can comment on or make changes to this bug.