r94791 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r94790‎ | r94791 | r94792 >
Date:18:54, 17 August 2011
Author:khorn
Status:resolved (Comments)
Tags:
Comment:
r94720
Removes the backticks for the new database updater.
Modified paths:
  • /trunk/extensions/ContributionTracking/ContributionTracking.sql (modified) (history)

Diff [purge]

Index: trunk/extensions/ContributionTracking/ContributionTracking.sql
——@@ -1,18 +1,18 @@
2 -CREATE TABLE IF NOT EXISTS /*_*/`contribution_tracking` (
3 - `id` int(10) unsigned NOT NULL auto_increment,
4 - `contribution_id` int(10) unsigned default NULL,
5 - `note` text,
6 - `referrer` varchar(4096) default NULL,
7 - `anonymous` tinyint(1) unsigned NOT NULL,
8 - `utm_source` varchar(128) default NULL,
9 - `utm_medium` varchar(128) default NULL,
10 - `utm_campaign` varchar(128) default NULL,
11 - `optout` tinyint(1) unsigned NOT NULL,
12 - `language` varchar(8) default NULL,
13 - `ts` char(14) default NULL,
14 - `owa_session` varchar(255) default NULL,
15 - `owa_ref` int(11) default NULL,
16 - PRIMARY KEY (`id`),
17 - UNIQUE KEY `contribution_id` (`contribution_id`),
18 - KEY `ts` (`ts`)
 2+CREATE TABLE IF NOT EXISTS /*_*/contribution_tracking (
 3+ id int(10) unsigned NOT NULL auto_increment,
 4+ contribution_id int(10) unsigned default NULL,
 5+ note text,
 6+ referrer varchar(4096) default NULL,
 7+ anonymous tinyint(1) unsigned NOT NULL,
 8+ utm_source varchar(128) default NULL,
 9+ utm_medium varchar(128) default NULL,
 10+ utm_campaign varchar(128) default NULL,
 11+ optout tinyint(1) unsigned NOT NULL,
 12+ language varchar(8) default NULL,
 13+ ts char(14) default NULL,
 14+ owa_session varchar(255) default NULL,
 15+ owa_ref int(11) default NULL,
 16+ PRIMARY KEY (id),
 17+ UNIQUE KEY contribution_id (contribution_id),
 18+ KEY ts (ts)
1919 ) /*wgDBTableOptions*/;

Follow-up revisions

RevisionCommit summaryAuthorDate
r94854r94791...khorn01:28, 18 August 2011
r94911Suggested changes to the contribution_tracking table definition (r94791).khorn17:19, 18 August 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r94720In the process of adding an API to ContributionTracking, I ended up refactori...khorn00:58, 17 August 2011

Comments

#Comment by Awjrichards (talk | contribs)   00:51, 18 August 2011

We'll also need to perform an ALTER TABLE to cover the case where a database was generated before this commit and have it handled by the udpater.

#Comment by 😂 (talk | contribs)   01:43, 18 August 2011

A patch file should be checked in along with the schema change so we don't have to figure out the difference :)

#Comment by Khorn (WMF) (talk | contribs)   01:02, 18 August 2011

Isn't that handled in patch-owa.sql? http://svn.wikimedia.org/viewvc/mediawiki/trunk/extensions/ContributionTracking/patch-owa.sql?revision=75026&view=markup I tested both the clean install and the alter case after Reedy committed the new database updater changes in his r94771.

#Comment by Awjrichards (talk | contribs)   01:14, 18 August 2011

er yeah :) didn't see that. but as you just told me in person, make sure the data types match :p

#Comment by Khorn (WMF) (talk | contribs)   01:30, 18 August 2011

Roger. Made 'em both match in r94854, killed and rebuilt the relevant table with update.php, and all my automated tests are still OK.

#Comment by 😂 (talk | contribs)   01:46, 18 August 2011

I know this isn't the fault of this rev, but this is a perfect time to fix it (and it's the first time I've seen it): 

+  PRIMARY KEY  (id),
+  UNIQUE KEY contribution_id (contribution_id),
+  KEY ts (ts)

We define our secondary keys outside of the CREATE TABLE statement, doing something like: 

CREATE INDEX /*i*/cr_repo_status_author ON /*_*/code_rev (cr_repo_id, cr_status, cr_author);

Also, get rid of the PRIMARY KEY(id) and just add PRIMARY KEY to contribution_tracking.id's definition. These changes are good practice and make it compatible with Sqlite.

#Comment by Khorn (WMF) (talk | contribs)   17:20, 18 August 2011

Done. (94911) :)

Status & tagging log