Page MenuHomePhabricator

Standardise type of timestamp database fields (MySQL)
Open, LowPublic

Description

MediaWiki is using 14 char field as binary or varbinary or data type timestamp. Please clean this up, by using only one data type for all of them.

tables.sql contains a sentence about this:

  • The MySQL table backend for MediaWiki currently uses
  • 14-character BINARY or VARBINARY fields to store timestamps.
  • The format is YYYYMMDDHHMMSS, which is derived from the
  • text format of MySQL's TIMESTAMP fields. --
  • Historically TIMESTAMP fields were used, but abandoned
  • in early 2002 after a lot of trouble with the fields
  • auto-updating.

Following fields are timestamp:

user_newpass_time binary(14),
user_touched binary(14) NOT NULL default '',
user_email_authenticated binary(14),
user_email_token_expires binary(14),
user_registration binary(14),
user_editcount int
user_last_timestamp varbinary(14) NULL default NULL
page_touched binary(14) NOT NULL default '',
rev_timestamp binary(14) NOT NULL default '',
ar_timestamp binary(14) NOT NULL default '',
cl_timestamp timestamp NOT NULL,
ipb_timestamp binary(14) NOT NULL default '',
ipb_expiry varbinary(14) NOT NULL default '',
img_timestamp varbinary(14) NOT NULL default '',
oi_timestamp binary(14) NOT NULL default '',
fa_deleted_timestamp binary(14) default '',
fa_timestamp binary(14) default '',
us_timestamp varbinary(14) NOT NULL,
rc_timestamp varbinary(14) NOT NULL default '',
rc_cur_time varbinary(14) NOT NULL default '',
wl_notificationtimestamp varbinary(14)
exptime datetime
tc_time binary(14) NOT NULL
log_timestamp binary(14) NOT NULL default '19700101000000',
job_timestamp varbinary(14) NULL default NULL,
qci_timestamp binary(14) NOT NULL default '19700101000000'
pr_expiry varbinary(14) NULL,
pt_timestamp binary(14) NOT NULL,
pt_expiry varbinary(14) NOT NULL default '',
mr_timestamp binary(14) NOT NULL

Version: 1.20.x
Severity: normal

Event Timeline

bzimport raised the priority of this task from to Low.Nov 22 2014, 12:50 AM
bzimport set Reference to bz40626.
bzimport added a subscriber: Unknown Object (MLST).

pawanseerwani+bugzilla wrote:

I am willing to work on this.

As I understand, all the above mentioned columns need to altered to timestamp data type of mysql with give constraints and default values(if any).

Have I covered it all or is there anything else to do here ?

pawanseerwani+bugzilla wrote:

I have some questions regarding this bug.

  1. user_editcount int --> Is this really meant to be changed to timestamp?
  2. user_last_timestamp --> no such column exists in user table.(Using mediawiki 1.23)

(In reply to comment #2)

  1. user_last_timestamp --> no such column exists in user table.(Using

mediawiki 1.23)

Wrong table? https://www.mediawiki.org/wiki/Manual:User_newtalk_table

pawanseerwani+bugzilla wrote:

(In reply to comment #3)

(In reply to comment #2)

  1. user_last_timestamp --> no such column exists in user table.(Using

mediawiki 1.23)

Wrong table? https://www.mediawiki.org/wiki/Manual:User_newtalk_table

Thanks ! But don't you think this table has conflicting column prefix? (Similar to User table)

(In reply to comment #0)

exptime datetime

I dint understand this line. Could you explain this?

Change 110949 had a related patch set uploaded by Gerrit Patch Uploader:
Add cleanTimeStampFields.php

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

user_editcount is wrong here, sorry for that.

user_last_timestamp is part of user_newtalk, this column has a wrong prefix

exptime is part of the objectcache table

You have to call the sql from the updaters. You can found the updaters in includes/installer.

pawanseerwani+bugzilla wrote:

(In reply to db from comment #6)

user_last_timestamp is part of user_newtalk, this column has a wrong prefix

Done

exptime is part of the objectcache table

Done

You have to call the sql from the updaters. You can found the updaters in
includes/installer.

There is no good documentation of installers/updaters, though I have a added a function to call applyPatch() in MysqlUpdater.php
Anything else to be done?

Danny_B moved this task from Unsorted to Cleanup on the Schema-change board.May 4 2016, 3:10 AM
Krinkle moved this task from Untriaged to Schema changes on the Wikimedia-Rdbms board.
Krinkle added a subscriber: Krinkle.

Looks like a schema change patch exists at https://gerrit.wikimedia.org/r/110949.

Unsure about its status and/or opinion from DBAs, and in relation to other on-going schema changes.

Marostegui added a subscriber: Marostegui.

Looks like a schema change patch exists at https://gerrit.wikimedia.org/r/110949.

Unsure about its status and/or opinion from DBAs, and in relation to other on-going schema changes.

Not much to say, we can stick to whatever is decided to be the standard.
Once the patch is merged just a Blocked-on-schema-change tag and we will take care of it.

Looks like we currently have 17 uses of storing timestamps as binary(14) and 13 uses as varbinary(14).

@Marostegui Any preference for one over the other?

The current patch seems to be standardising on varbinary(14). Quick glance at MySQL docs and stackoverflow:

  • binary() - fixed size in schema. Value stored without recording the value size alongside each value (saves 2 bytes per row/value). If a shorter value is stored, it is padded.
  • varbinary() - variable size, recorded in each field. Allows shorter values to be stored without padding, but internal field is appended to indicate actual size (2 bytes extra for each row/value).

Looking at our schema more widely (not limited to timestamps) we almost exclusively use varbinary(), but that's because we store a lot of variable text, where varbinary() is chosen as alternative to varchar(), not as alternative to binary(). In 2011 we switched from varchar to varbinary to avoid certain issues with collations and encodings. For timestamps, however, we don't need variable length. Would binary() be a better fit?

Krinkle renamed this task from clean up timestamp database fields to Standardise type of timestamp database fields (MySQL).Dec 1 2017, 1:05 AM

This is only tangently related: T212529: Standardize datetimes/timestamps in the Data Lake.

We prefer UTC ISO-8601 (Z suffixed) strings where possible. This is just an FYI, I'm not proposing we change Mediawiki. :)

I encountered this when doing T230428: Migrate tables.sql to abstract schema and probably do it with this task (or do it later) but I have a question. What about Postgres? Right now, mysql uses (var)binary but postgres uses TIMESTAMPTZ. Should I change it to varbinary for consistency or do hacks to make MySQL varbinary and Postgres TIMESTAMPTZ?

Krinkle removed a subscriber: Krinkle.Jun 28 2020, 5:42 PM
Ladsgroup claimed this task.Aug 9 2020, 7:34 PM

It seems I need to do this to achieve the abstract schema.

Restricted Application added a project: User-Ladsgroup. · View Herald TranscriptAug 9 2020, 7:34 PM

Change 626854 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/core@master] Introduce TimestampType for handling custom db type in doctrine

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

Change 626854 merged by jenkins-bot:
[mediawiki/core@master] Introduce TimestampType for handling custom db type in doctrine

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

Change 630955 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/core@master] Migrate page_restrictions to abstract schema

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

Change 631936 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/core@master] Trim whitespace from $expiry in Database::decodeExpiry()

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

Change 631937 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/core@master] Migrate user_groups to abstract schema

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

Change 634767 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/core@master] Add custom option called "allowInfinite" for Timestamp

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

Change 634767 merged by jenkins-bot:
[mediawiki/core@master] Add custom option called "allowInfinite" for Timestamp

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

Change 630955 merged by jenkins-bot:
[mediawiki/core@master] Migrate page_restrictions to abstract schema

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

Change 631937 merged by jenkins-bot:
[mediawiki/core@master] Migrate user_groups to abstract schema

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

Change 641701 had a related patch set uploaded (by Ammarpad; owner: Ammarpad):
[mediawiki/core@master] Migrate job to abstract schema

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

Change 642685 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/core@master] Add patch to set wl_notificationtimestamp BINARY(14)

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

Change 641701 merged by jenkins-bot:
[mediawiki/core@master] Migrate job to abstract schema

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

Change 642685 merged by jenkins-bot:
[mediawiki/core@master] Add patch to set wl_notificationtimestamp BINARY(14)

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