Page MenuHomePhabricator

Language#formatExpiry needs database connection
Open, MediumPublic

Description

Language#formatExpiry needs database connection (to get the value of infinity string – $infinity = wfGetDB( DB_REPLICA )->getInfinity();). It should not.

After this is fixed, the FIXME added in https://gerrit.wikimedia.org/r/#/c/80663/ should be removed.


Version: unspecified
Severity: minor

Details

Reference
bz55912

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 2:18 AM
bzimport set Reference to bz55912.
bzimport added a subscriber: Unknown Object (MLST).

Why is this classified as an i18n issue? AFAICT getInfinity() was implemented in the Database class for some reason.

includes/db/Database.php, Class Database, ~line 3800:

/**

  • Find out when 'infinity' is. Most DBMSes support this. This is a special
  • keyword for timestamps in PostgreSQL, and works with CHAR(14) as well
  • because "i" sorts after all numbers. *
  • @return String */

public function getInfinity() {
return 'infinity';
}

Feel free to change the component if you think it's incorrect, I18n felt appropriate as this is an issue with (among others) the Language class.

Krinkle added a subscriber: Krinkle.
Krinkle removed a subscriber: Krinkle.

Given that Database::getInfinity() is hardcoded to just return infinity and has no subclasses that override the method, perhaps we should consider removing the method entirely (along with IDatabase::getInfinity()) in favor of just hardcoding the use of infinity?

For some history:
In 2010[1] the DatabaseMssql class to support MSSQL (Microsoft SQL Server) was added. At the time, MSSQL didn't recognize the 'infinity' keyword, so infinity was defined as '3000-01-31 00:00:00.000' if that was the database being used (DatabaseMssql), and for any other database we just used the keyword 'infinity'
In 2011[2] Database::getInfinity() was added to move the logic out of Block::infinity() - MSSQL still didn't recognize 'infinity', so Database::getInfinity() returned 'infinity' while DatabaseMssql overrode the method to return '3000-01-31 00:00:00.000'
In 2014[3] more support was added for MSSQL (commit was labeled as "Add preliminary MS SQL support", but DatabaseMssql already existed, so its a bit confusing) and in the process the DatabaseMssql::getInfinity() override was removed, and the database field where this was used, ipb_expiry, was changed for mssql tables from DATETIME NOT NULL DEFAULT GETDATE() to varchar(14), meaning that it could now support the string 'infinity' directly

Given that the standardized timestamp field is 14 characters that can be either an actual timestamp (YYYYMMDDHHMMSS) or some other string (like "infinity"), I don't believe that we will need to support different databases having different versions of infinity in the future, and thus suggest we remove the getInfinity() methods in favor of either hardcoding the string or using a constant

[1] https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/core/+/070122e8b93984fe9345fed6eedc195285cc1991
[2] https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/core/+/b61756cdeac238c920093e9ceeb89135226d5f63
[3] https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/core/+/f7174057a4ec7b9c74babfe74290bb000204a581