Page MenuHomePhabricator

DatabaseMysql(Base)::buildStringCast should probably actually cast
Closed, ResolvedPublic

Description

IDatabase::buildStringCast() exists to build an expression to cast a field to a string type, e.g. CAST( field AS TEXT ). The current implementations in our code base are:

  • Database: Returns field, relying on implicit conversion.
  • DatabaseSqlite: CAST( field AS TEXT )
  • DatabasePostgres: field::text
  • DatabaseOracle: CAST( field AS VARCHAR2 )
  • MySQL and MSSQL inherit the no-op from Database.

While the SQL standard doesn't seem to require the implicit conversion to exist, all our supported database engines except PostgreSQL do seem to do implicit conversion (at least where integer→string is needed).

However, in T216183: Special:ProtectedPages times out on enwiki for Module namespace we discovered that MySQL does not make effective use of the index on a string column when comparing with an integer-valued column. In that case the index is correctly used when an explicit cast of the integer column to a string type is made. Thus this task.

Subtasks:

  • Audit (and, if possible, fix) existing calls to buildStringCast() for anything that might break
    • in MySQL if it starts producing CAST( field AS BINARY ).
    • (optional) in MSSQL if it starts producing CAST( field AS NVARCHAR ).
  • Implement buildStringCast() in the remaining subclasses
    • in DatabaseMysqlBase to produce CAST( field AS BINARY ).
    • in DatabaseMssql to produce either field (preserving the current behavior) or CAST( field AS NVARCHAR ).
  • Update Database::buildStringCast() to generically cast too, or make it abstract.

Event Timeline

Anomie created this task.Feb 15 2019, 3:10 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 15 2019, 3:10 PM
Anomie updated the task description. (Show Details)Feb 15 2019, 8:59 PM

I thought there would be more uses, but there's only two in core, and one weird one in Wikibase that none the less won't break.

Anomie updated the task description. (Show Details)Feb 20 2019, 7:55 PM

Change 491844 had a related patch set uploaded (by Anomie; owner: Anomie):
[mediawiki/core@master] Database: Have buildStringCast() actually cast for MySQL, MSSQL

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

Change 491844 merged by jenkins-bot:
[mediawiki/core@master] Database: Have buildStringCast() actually cast for MySQL, MSSQL

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

Anomie updated the task description. (Show Details)Feb 20 2019, 10:30 PM
Anomie closed this task as Resolved.
Anomie claimed this task.