Page MenuHomePhabricator

SQL casting error in Special:ProtectedPages
Closed, ResolvedPublic

Description

Fix for SQL casting error with Postgres

In Special:ProtectedPages, a text field is compared directly to an integer, which moden versions of Postgres do not like. Error seen:

Query:
SELECT pr_id,page_namespace,page_title,page_len,pr_type,pr_level,pr_expiry,pr_cascade,log_timestamp,log_user,log_comment,log_deleted FROM "page","page_restrictions" LEFT JOIN "log_search" ON (ls_field = 'pr_id' AND (ls_value = pr_id)) LEFT JOIN "logging" ON ((ls_log_id = log_id)) WHERE (pr_expiry > '2014-06-25 01:59:54 GMT'OR pr_expiry IS NULL) AND (page_id=pr_page) AND (pr_type='edit') ORDER BY pr_id LIMIT 51
Function: IndexPager::buildQueryInfo (ProtectedPagesPager)
Error: 42883 ERROR: operator does not exist: text = integer LINE 1: ..."log_search" ON (ls_field = 'pr_id' AND (ls_value = pr_id)) ... ^ HINT: No operator matches the given name and argument type(s). You might need to add explicit type casts.

Yes, Postgres is being pedantic, but it is weird that the where clause has a proper cast right before it:

ON (ls_field = 'pr_id' AND (ls_value = pr_id))

The solution is to quote the pr_id. Diff attached.


Version: unspecified
Severity: normal

Attached:

Related Objects

View Standalone Graph
This task is connected to more than 200 other tasks. Only direct parents and subtasks are shown here. Use View Standalone Graph to show more of the graph.
StatusSubtypeAssignedTask
InvalidNone
ResolvedUmherirrender

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 3:38 AM
bzimport set Reference to bz67065.

Hi! Thanks for your patch!

You are welcome to use Developer access

https://www.mediawiki.org/wiki/Developer_access

to submit this as a Git branch directly into Gerrit:

https://www.mediawiki.org/wiki/Git/Tutorial

Putting your branch in Git makes it easier to review it quickly. If you don't want to set up Git/Gerrit, you can also use https://tools.wmflabs.org/gerrit-patch-uploader/
Thanks again!

Table 'log_search' is a table to match a row of the logging table and corresponding data. The ls_field is used to determine between all the given values.

In this case the value of column ls_value (varchar(255)) is compared to the value of column pr_id (integer) which is needed here to find all the log_ids for protected pages.
Postgres seems to have problems with different data types. So mediawiki needs a new column ls_value_int or a explicit database cast. Quoting is not a option here, because that will change the meaning of the query (ls_value never contains the word 'pr_id').

Before the use of pr_id the log_search table was only used to find user names, so there was no cast problem.

Oh, right, sorry about that. Looks like a sticky problem then. Maybe we can use CAST() as both systems seem to support it?

Except MySQL does not support casting to VARCHAR. Sigh.

Made a gerrit patch here:

https://gerrit.wikimedia.org/r/#/c/164765/

As the comments say, this is a tricky problem, with no immediate general solution I could see. The only commonality seems to be Postgres and Sqlite could both use something like CAST(pr_id AS TEXT). Oracle needs VARCHAR2. MySQL cannot cast to VARCHAR. I can envision some other workarounds, but for the moment, a direct exception for Postgres seems the best answer. Other opinions welcome.

(In reply to Greg Sabino Mullane from comment #5)

Made a gerrit patch here

Thanks! Following http://www.mediawiki.org/wiki/Gerrit/Commit_message_guidelines is welcome which triggers automatic notifications in Bugzilla.

Change 164765 had a related patch set uploaded by Nemo bis:
Force cast the pr_id column (an int) to text

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

Change 288745 had a related patch set uploaded (by Aklapper):
PostgreSQL: Allow comparison of integers to strings.

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

On http://dba.stackexchange.com/questions/82511/how-to-enable-implicit-casts-in-postgresql-9-2 there is a way to allow the cast as implicit cast without extra code in mediawiki, but that needs to be done on each installation, maybe it is a workaround for now or something to add on install through mediawiki

Change 296066 had a related patch set uploaded (by Umherirrender):
Added a Database::buildStringCast and use on int/string join

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

Change 296066 merged by jenkins-bot:
Added a Database::buildStringCast and use on int/string join

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

Change 288745 abandoned by Umherirrender:
PostgreSQL: Allow comparison of integers to strings.

Reason:
Superseded by I3a83276dc65aae58124460af1810d37dff52e943

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

Umherirrender claimed this task.
Jdforrester-WMF subscribed.

Migrating from the old tracking task to a tag for PostgreSQL-related tasks.