From 27c76fa4ae07b6a1404bad269698d1af2679cc43 Mon Sep 17 00:00:00 2001 From: Eddie Greiner-Petter Date: Mon, 8 May 2017 21:31:54 +0200 Subject: [PATCH] Overhaul site_stats table The site stats table holds a bunch of metric fields, two of which are of data type "bigint unsigned", 3 are "bigint" (signed) and one is int (signed). Also the default values differ widely: It is 0 on the "unsigned" fields and the "int" field, but -1 on the three others. This patch makes all of this more consistent: Set all fields (except the ss_row_id, which isn't changed) data type to "bigint unsigned". Also set NULL as the default value for all those fields. Obviously -1 isn't a possible default value any more. Also, 0 can easily be mistaken for a real value (e.g. ss_active_users=0 --> "there is nobody active on this wiki"). NULL, by it's definition, is the value of choice for a value to insert into fields of which we don't know a correct value. The respective patch files were tested locally against MySql, Sqlite, Postgres and SQL Server 2016. Neither oracle nor the upgrade with update.php was tested. Bug: T56888 Change-Id: I7d42aae434852a56b6f8dd559d8a5f3bce416021 --- includes/installer/MssqlUpdater.php | 1 + includes/installer/MysqlUpdater.php | 1 + includes/installer/OracleUpdater.php | 1 + includes/installer/PostgresUpdater.php | 1 + includes/installer/SqliteUpdater.php | 1 + .../archives/patch-site_stats-modify.sql | 7 ++++ .../archives/patch-site_stats-modify.sql | 32 +++++++++++++++++ maintenance/mssql/tables.sql | 26 ++++++-------- .../archives/patch-site_stats-modify.sql | 7 ++++ maintenance/oracle/tables.sql | 12 +++---- .../archives/patch-site_stats-modify.sql | 7 ++++ maintenance/postgres/tables.sql | 14 ++++---- .../archives/patch-site_stats-modify.sql | 35 +++++++++++++++++++ maintenance/tables.sql | 26 ++++++-------- 14 files changed, 128 insertions(+), 43 deletions(-) create mode 100644 maintenance/archives/patch-site_stats-modify.sql create mode 100644 maintenance/mssql/archives/patch-site_stats-modify.sql create mode 100644 maintenance/oracle/archives/patch-site_stats-modify.sql create mode 100644 maintenance/postgres/archives/patch-site_stats-modify.sql create mode 100644 maintenance/sqlite/archives/patch-site_stats-modify.sql diff --git a/includes/installer/MssqlUpdater.php b/includes/installer/MssqlUpdater.php index 908dc940c2..694cd294c5 100644 --- a/includes/installer/MssqlUpdater.php +++ b/includes/installer/MssqlUpdater.php @@ -120,6 +120,7 @@ class MssqlUpdater extends DatabaseUpdater { [ 'addTable', 'actor', 'patch-actor-table.sql' ], [ 'migrateActors' ], [ 'modifyField', 'revision', 'rev_text_id', 'patch-rev_text_id-default.sql' ], + [ 'modifyTable', 'site_stats', 'patch-site_stats-modify.sql' ], ]; } diff --git a/includes/installer/MysqlUpdater.php b/includes/installer/MysqlUpdater.php index 710cebfda5..9427596565 100644 --- a/includes/installer/MysqlUpdater.php +++ b/includes/installer/MysqlUpdater.php @@ -340,6 +340,7 @@ class MysqlUpdater extends DatabaseUpdater { [ 'addTable', 'actor', 'patch-actor-table.sql' ], [ 'migrateActors' ], [ 'modifyField', 'revision', 'rev_text_id', 'patch-rev_text_id-default.sql' ], + [ 'modifyTable', 'site_stats', 'patch-site_stats-modify.sql' ], ]; } diff --git a/includes/installer/OracleUpdater.php b/includes/installer/OracleUpdater.php index 43b74f1f38..cb0399c6c0 100644 --- a/includes/installer/OracleUpdater.php +++ b/includes/installer/OracleUpdater.php @@ -140,6 +140,7 @@ class OracleUpdater extends DatabaseUpdater { [ 'migrateArchiveText' ], [ 'addTable', 'actor', 'patch-actor-table.sql' ], [ 'migrateActors' ], + [ 'modifyTable', 'site_stats', 'patch-site_stats-modify.sql' ], // KEEP THIS AT THE BOTTOM!! [ 'doRebuildDuplicateFunction' ], diff --git a/includes/installer/PostgresUpdater.php b/includes/installer/PostgresUpdater.php index ba6e9683e3..c9a50863fe 100644 --- a/includes/installer/PostgresUpdater.php +++ b/includes/installer/PostgresUpdater.php @@ -533,6 +533,7 @@ class PostgresUpdater extends DatabaseUpdater { [ 'addPgIndex', 'logging', 'logging_actor_type_time', '( log_actor, log_type, log_timestamp )' ], [ 'addPgIndex', 'logging', 'logging_actor_time', '( log_actor, log_timestamp )' ], [ 'migrateActors' ], + [ 'modifyTable', 'site_stats', 'patch-site_stats-modify.sql' ], ]; } diff --git a/includes/installer/SqliteUpdater.php b/includes/installer/SqliteUpdater.php index 7ed6f8680d..7a2cc0d803 100644 --- a/includes/installer/SqliteUpdater.php +++ b/includes/installer/SqliteUpdater.php @@ -204,6 +204,7 @@ class SqliteUpdater extends DatabaseUpdater { [ 'addTable', 'actor', 'patch-actor-table.sql' ], [ 'migrateActors' ], [ 'modifyField', 'revision', 'rev_text_id', 'patch-rev_text_id-default.sql' ], + [ 'modifyTable', 'site_stats', 'patch-site_stats-modify.sql' ], ]; } diff --git a/maintenance/archives/patch-site_stats-modify.sql b/maintenance/archives/patch-site_stats-modify.sql new file mode 100644 index 0000000000..c70dd001ad --- /dev/null +++ b/maintenance/archives/patch-site_stats-modify.sql @@ -0,0 +1,7 @@ +ALTER TABLE /*_*/site_stats + ALTER ss_total_edits SET DEFAULT NULL, + ALTER ss_good_articles SET DEFAULT NULL, + MODIFY COLUMN ss_total_pages bigint unsigned DEFAULT NULL, + MODIFY COLUMN ss_users bigint unsigned DEFAULT NULL, + MODIFY COLUMN ss_active_users bigint unsigned DEFAULT NULL, + MODIFY COLUMN ss_images bigint unsigned DEFAULT NULL; diff --git a/maintenance/mssql/archives/patch-site_stats-modify.sql b/maintenance/mssql/archives/patch-site_stats-modify.sql new file mode 100644 index 0000000000..b2de94833c --- /dev/null +++ b/maintenance/mssql/archives/patch-site_stats-modify.sql @@ -0,0 +1,32 @@ +/* Delete old default constraints */ +DECLARE @sql nvarchar(max) +SET @sql='' + +/* IMHO: A DBMS where you have to do THIS to change a default value sucks. */ +SELECT @sql= @sql + 'ALTER TABLE site_stats DROP CONSTRAINT ' + df.name + '; ' +FROM sys.default_constraints df +JOIN sys.columns c + ON c.object_id = df.parent_object_id + AND c.column_id = df.parent_column_id +WHERE + df.parent_object_id = OBJECT_ID('site_stats');-- + +EXEC sp_executesql @sql; + +/* Change data type of ss_images from int to bigint. + * All other fields (except ss_row_id) already are bigint. + * This MUST happen before adding new constraints. */ +ALTER TABLE site_stats ALTER COLUMN ss_images bigint; + +/* Add new default constraints. + * Don't ask me why I have to repeat ALTER TABLE site_stats + * instead of using commas, for some reason SQL Server 2016 + * didn't accept it in any other way. Maybe I just don't know + * enough about mssql, but this works. + */ +ALTER TABLE site_stats ADD CONSTRAINT col_ss_total_edits DEFAULT NULL FOR ss_total_edits; +ALTER TABLE site_stats ADD CONSTRAINT col_ss_good_article DEFAULT NULL FOR ss_good_articles; +ALTER TABLE site_stats ADD CONSTRAINT col_ss_total_pages DEFAULT NULL FOR ss_total_pages; +ALTER TABLE site_stats ADD CONSTRAINT col_ss_users DEFAULT NULL FOR ss_users; +ALTER TABLE site_stats ADD CONSTRAINT col_ss_active_users DEFAULT NULL FOR ss_active_users; +ALTER TABLE site_stats ADD CONSTRAINT col_ss_images DEFAULT NULL FOR ss_images; diff --git a/maintenance/mssql/tables.sql b/maintenance/mssql/tables.sql index ddc5517734..977666d47b 100644 --- a/maintenance/mssql/tables.sql +++ b/maintenance/mssql/tables.sql @@ -591,26 +591,22 @@ CREATE TABLE /*_*/site_stats ( ss_row_id int NOT NULL CONSTRAINT /*i*/ss_row_id PRIMARY KEY, -- Total number of edits performed. - ss_total_edits bigint default 0, + ss_total_edits bigint default NULL, - -- An approximate count of pages matching the following criteria: - -- * in namespace 0 - -- * not a redirect - -- * contains the text '[[' - -- See Article::isCountable() in includes/Article.php - ss_good_articles bigint default 0, + -- See SiteStatsInit::articles(). + ss_good_articles bigint default NULL, - -- Total pages, theoretically equal to SELECT COUNT(*) FROM page; except faster - ss_total_pages bigint default '-1', + -- Total pages, theoretically equal to SELECT COUNT(*) FROM page. + ss_total_pages bigint default NULL, - -- Number of users, theoretically equal to SELECT COUNT(*) FROM user; - ss_users bigint default '-1', + -- Number of users, theoretically equal to SELECT COUNT(*) FROM user. + ss_users bigint default NULL, - -- Number of users that still edit - ss_active_users bigint default '-1', + -- Number of users that still edit. + ss_active_users bigint default NULL, - -- Number of images, equivalent to SELECT COUNT(*) FROM image - ss_images int default 0 + -- Number of images, equivalent to SELECT COUNT(*) FROM image. + ss_images bigint default NULL ); diff --git a/maintenance/oracle/archives/patch-site_stats-modify.sql b/maintenance/oracle/archives/patch-site_stats-modify.sql new file mode 100644 index 0000000000..1c784d9bc4 --- /dev/null +++ b/maintenance/oracle/archives/patch-site_stats-modify.sql @@ -0,0 +1,7 @@ +ALTER TABLE /*_*/site_stats + ALTER ss_total_edits SET DEFAULT NULL, + ALTER ss_good_articles SET DEFAULT NULL, + ALTER ss_total_pages SET DEFAULT NULL, + ALTER ss_users SET DEFAULT NULL, + ALTER ss_active_users SET DEFAULT NULL, + ALTER ss_images SET DEFAULT NULL; diff --git a/maintenance/oracle/tables.sql b/maintenance/oracle/tables.sql index 058ef15321..8551fb9975 100644 --- a/maintenance/oracle/tables.sql +++ b/maintenance/oracle/tables.sql @@ -457,12 +457,12 @@ CREATE UNIQUE INDEX &mw_prefix.iwlinks_ui02 ON &mw_prefix.iwlinks (iwl_prefix, i CREATE TABLE &mw_prefix.site_stats ( ss_row_id NUMBER NOT NULL PRIMARY KEY, - ss_total_edits NUMBER DEFAULT 0, - ss_good_articles NUMBER DEFAULT 0, - ss_total_pages NUMBER DEFAULT -1, - ss_users NUMBER DEFAULT -1, - ss_active_users NUMBER DEFAULT -1, - ss_images NUMBER DEFAULT 0 + ss_total_edits NUMBER DEFAULT NULL, + ss_good_articles NUMBER DEFAULT NULL, + ss_total_pages NUMBER DEFAULT NULL, + ss_users NUMBER DEFAULT NULL, + ss_active_users NUMBER DEFAULT NULL, + ss_images NUMBER DEFAULT NULL ); CREATE SEQUENCE ipblocks_ipb_id_seq; diff --git a/maintenance/postgres/archives/patch-site_stats-modify.sql b/maintenance/postgres/archives/patch-site_stats-modify.sql new file mode 100644 index 0000000000..1c784d9bc4 --- /dev/null +++ b/maintenance/postgres/archives/patch-site_stats-modify.sql @@ -0,0 +1,7 @@ +ALTER TABLE /*_*/site_stats + ALTER ss_total_edits SET DEFAULT NULL, + ALTER ss_good_articles SET DEFAULT NULL, + ALTER ss_total_pages SET DEFAULT NULL, + ALTER ss_users SET DEFAULT NULL, + ALTER ss_active_users SET DEFAULT NULL, + ALTER ss_images SET DEFAULT NULL; diff --git a/maintenance/postgres/tables.sql b/maintenance/postgres/tables.sql index 1e1c434a47..9a5420a14b 100644 --- a/maintenance/postgres/tables.sql +++ b/maintenance/postgres/tables.sql @@ -370,13 +370,13 @@ CREATE INDEX langlinks_lang_title ON langlinks (ll_lang,ll_title); CREATE TABLE site_stats ( ss_row_id INTEGER NOT NULL PRIMARY KEY DEFAULT 0, - ss_total_edits INTEGER DEFAULT 0, - ss_good_articles INTEGER DEFAULT 0, - ss_total_pages INTEGER DEFAULT -1, - ss_users INTEGER DEFAULT -1, - ss_active_users INTEGER DEFAULT -1, - ss_admins INTEGER DEFAULT -1, - ss_images INTEGER DEFAULT 0 + ss_total_edits INTEGER DEFAULT NULL, + ss_good_articles INTEGER DEFAULT NULL, + ss_total_pages INTEGER DEFAULT NULL, + ss_users INTEGER DEFAULT NULL, + ss_active_users INTEGER DEFAULT NULL, + ss_admins INTEGER DEFAULT NULL, + ss_images INTEGER DEFAULT NULL ); diff --git a/maintenance/sqlite/archives/patch-site_stats-modify.sql b/maintenance/sqlite/archives/patch-site_stats-modify.sql new file mode 100644 index 0000000000..8d267a62f0 --- /dev/null +++ b/maintenance/sqlite/archives/patch-site_stats-modify.sql @@ -0,0 +1,35 @@ +DROP TABLE IF EXISTS /*_*/site_stats_tmp; + +-- Create the temporary table. The following part +-- is copied & pasted from the changed tables.sql +-- file besides having an other table name. +CREATE TABLE /*_*/site_stats_tmp ( + ss_row_id int unsigned NOT NULL PRIMARY KEY, + ss_total_edits bigint unsigned default NULL, + ss_good_articles bigint unsigned default NULL, + ss_total_pages bigint unsigned default NULL, + ss_users bigint unsigned default NULL, + ss_active_users bigint unsigned default NULL, + ss_images bigint unsigned default NULL +) /*$wgDBTableOptions*/; + +-- Move the data from the old to the new table +INSERT OR IGNORE INTO /*_*/site_stats_tmp ( + ss_row_id, + ss_total_edits, + ss_good_articles, + ss_total_pages, + ss_active_users, + ss_images +) SELECT + ss_row_id, + ss_total_edits, + ss_good_articles, + ss_total_pages, + ss_active_users, + ss_images +FROM /*_*/site_stats; + +DROP TABLE /*_*/site_stats; + +ALTER TABLE /*_*/site_stats_tmp RENAME TO /*_*/site_stats; diff --git a/maintenance/tables.sql b/maintenance/tables.sql index 90c559ab7f..9aa6c20a69 100644 --- a/maintenance/tables.sql +++ b/maintenance/tables.sql @@ -988,26 +988,22 @@ CREATE TABLE /*_*/site_stats ( ss_row_id int unsigned NOT NULL PRIMARY KEY, -- Total number of edits performed. - ss_total_edits bigint unsigned default 0, + ss_total_edits bigint unsigned default NULL, - -- An approximate count of pages matching the following criteria: - -- * in namespace 0 - -- * not a redirect - -- * contains the text '[[' - -- See Article::isCountable() in includes/Article.php - ss_good_articles bigint unsigned default 0, + -- See SiteStatsInit::articles(). + ss_good_articles bigint unsigned default NULL, - -- Total pages, theoretically equal to SELECT COUNT(*) FROM page; except faster - ss_total_pages bigint default '-1', + -- Total pages, theoretically equal to SELECT COUNT(*) FROM page. + ss_total_pages bigint unsigned default NULL, - -- Number of users, theoretically equal to SELECT COUNT(*) FROM user; - ss_users bigint default '-1', + -- Number of users, theoretically equal to SELECT COUNT(*) FROM user. + ss_users bigint unsigned default NULL, - -- Number of users that still edit - ss_active_users bigint default '-1', + -- Number of users that still edit. + ss_active_users bigint unsigned default NULL, - -- Number of images, equivalent to SELECT COUNT(*) FROM image - ss_images int default 0 + -- Number of images, equivalent to SELECT COUNT(*) FROM image. + ss_images bigint unsigned default NULL ) /*$wgDBTableOptions*/; -- -- 2.20.1