Page MenuHomePhabricator

Investigate potential signed int references to rev_id
Open, Needs TriagePublic

Description

Following up on T364681, Codesearch shows several other database columns that look like they might reference the revision ID and yet be signed 32-bit integers (which means they can’t represent all revisions on Wikidata anymore):

(Note if you’re looking through those Codesearch results: ignore any postgres SQL files, UNSIGNED doesn’t even exist there.)

Wikimedia deployed

Extension:Translate

sql/mysql/translate_reviews.sql
CREATE TABLE /*_*/translate_reviews (
  trr_user INT NOT NULL,
  trr_page INT NOT NULL,
  trr_revision INT NOT NULL,

Other

Extension:ContentStabilization

Note: I couldn’t find a Phabricator tag for this extension.

db/stable_file_points.sql
CREATE TABLE IF NOT EXISTS /*$wgDBprefix*/stable_file_points (
	`sfp_revision` INT NOT NULL PRIMARY KEY,
db/stable_file_transclusions.sql
CREATE TABLE IF NOT EXISTS /*$wgDBprefix*/stable_file_transclusions (
	`sft_revision` INT NOT NULL,
	`sft_page` INT NOT NULL,
	`sft_file_revision` INT NOT NULL,
db/stable_points.sql
CREATE TABLE IF NOT EXISTS /*$wgDBprefix*/stable_points (
	`sp_revision` INT NOT NULL PRIMARY KEY,
db/stable_transclusions.sql
CREATE TABLE IF NOT EXISTS /*$wgDBprefix*/stable_transclusions (
	`st_revision` INT NOT NULL,
	`st_page` INT NOT NULL,
	`st_transclusion_revision` INT NOT NULL,

Extension:MathSearch

db/math_review_list.sql
CREATE TABLE /*_*/math_review_list (
  revision_id INT(11)     NOT NULL,

Extension:PageCheckout

Note: I couldn’t find a Phabricator tag for this extension.

db/page_checkout_event.sql
CREATE TABLE IF NOT EXISTS /*$wgDBprefix*/page_checkout_event (
    `pce_action` VARCHAR(255) NOT NULL,
    `pce_actor_id` INT NOT NULL,
    `pce_page_id` INT NOT NULL,
    `pce_revision_id` INT NOT NULL,

Event Timeline

Change #1034471 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/Translate@master] Make translate_reviews columns unsigned and bigint where appropriate

https://gerrit.wikimedia.org/r/1034471

Change #1034471 merged by jenkins-bot:

[mediawiki/extensions/Translate@master] Make translate_reviews columns unsigned and bigint where appropriate

https://gerrit.wikimedia.org/r/1034471

Change #1043157 had a related patch set uploaded (by Physikerwelt; author: Physikerwelt):

[mediawiki/extensions/MathSearch@master] CI test

https://gerrit.wikimedia.org/r/1043157

Change #1043157 merged by jenkins-bot:

[mediawiki/extensions/MathSearch@master] db: update datatype of FK relation

https://gerrit.wikimedia.org/r/1043157

As I did deploy the MathSearch extension on quite a few instances, it makes sense to add a patch. From looking at the Translation patch, I guess it would SET foreign_key_checks = 0; before running the updater and reactivate it after it is completed.

Change #1043761 had a related patch set uploaded (by Physikerwelt; author: Physikerwelt):

[mediawiki/extensions/MathSearch@master] WIP Add db patches

https://gerrit.wikimedia.org/r/1043761

Change #1043761 merged by jenkins-bot:

[mediawiki/extensions/MathSearch@master] Add DB patches

https://gerrit.wikimedia.org/r/1043761

https://gerrit.wikimedia.org/r/1043761

This patch did not help.

Modifying rev_id field of table revision...Wikimedia\Rdbms\DBQueryError from line 1195 of /var/www/html/includes/libs/rdbms/database/Database.php: Error 1833: Cannot change column 'rev_id': used in a foreign key constraint 'mathindex_ibfk_1' of table 'my_wiki.mathindex'
Function: Wikimedia\Rdbms\Database::sourceFile( /var/www/html/maintenance/archives/patch-revision-cleanup.sql )
Query: ALTER TABLE `revision`
 CHANGE rev_id rev_id BIGINT UNSIGNED AUTO_INCREMENT NOT NULL,
 CHANGE rev_comment_id rev_comment_id BIGINT UNSIGNED NOT NULL,
 CHANGE rev_actor rev_actor BIGINT UNSIGNED NOT NULL,
 CHANGE rev_parent_id rev_parent_id BIGINT UNSIGNED DEFAULT NULL

I wonder if this patch

https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Translate/+/1034471

was tested, and I don't see a good reason why one patch would work and the other would not work.

We don't tend to use FK (and even, actively suggest against doing it), so Translate wouldn't have had any issues.

FOREIGN KEY ( /*i*/mathindex_revision_id ) REFERENCES revision( rev_id ) ON DELETE CASCADE

Change #1047511 had a related patch set uploaded (by Physikerwelt; author: Physikerwelt):

[mediawiki/extensions/MathSearch@master] Remove foreign key constraints

https://gerrit.wikimedia.org/r/1047511

Change #1047511 merged by jenkins-bot:

[mediawiki/extensions/MathSearch@master] Remove foreign key constraints

https://gerrit.wikimedia.org/r/1047511