Last modified: 2014-02-03 20:10:29 UTC
When refreshing a category page, MediaWiki issues a query to my PostgreSQL backend ending in "LOCK IN SHARED MODE". This isn't valid syntax to PGSQL. I have edited my copy of includes/db/Database.php, DatabaseBase::selectSQLText() to include... if ($this->getType() == 'postgres' and (in_array('LOCK IN SHARE MODE', $options, true) or isset($options['LOCK IN SHARE MODE']))) { // just fail //throw new Exception('Cannot use LOCK IN SHARE MODE with Postgresql'); // or just remove the option unset($options['LOCK IN SHARE MODE']); unset($options[array_search('LOCK IN SHARE MODE', $options, true)]); } The lack of a locking operation seems to me to be a better tradeoff than a category page flooded with pg_query error output.
Created attachment 10999 [details] Avoid SQL Syntax Error on Postgres
Add keywords that might help identify how trivial the fix is...
What are the steps to reproduce this? I cannot say I've ran into this issue.
Viewing a category page reproduces it for me after creating about 20 articles in various categories from a clean install. I can uncomment the exception line and generate a backtrace if that's useful. Gentoo Linux (`portage` versions listed) MediaWiki 1.19.1 (not available from this Bugzilla's version selector) PostgreSQL 9.1.4-r1 Apache 2.2.22-r1 PHP 5.4.6
Just to add more comment spam (sorry) it's worth noting that the patch is sane even if this isn't a bug in current code, since the 'LOCK IN SHARED MODE' isn't valid syntax in Postgres, and will always fail. Hence why I left an exception there (but commented) as an option instead of silently discarding the flag, as that particular flag simply doesn't make sense with Postgres and should be treated as a bug. I assume Mediawiki has some sort of assert() construct, though I'm not a MW dev so I'm not aware of it. If so, that might be a smarter approach.
This is was added in r32085 to bring in "Category" object. I wonder if this construct shouldn't really become "SELECT FOR UPDATE" instead? I think we should really patch it like this: iff --git a/includes/db/DatabasePostgres.php b/includes/db/DatabasePostgres.php index 8f8f5e8..457bf38 100644 --- a/includes/db/DatabasePostgres.php +++ b/includes/db/DatabasePostgres.php @@ -1406,9 +1406,6 @@ SQL; if ( isset( $noKeyOptions['FOR UPDATE'] ) ) { $postLimitTail .= ' FOR UPDATE'; } - if ( isset( $noKeyOptions['LOCK IN SHARE MODE'] ) ) { - $postLimitTail .= ' LOCK IN SHARE MODE'; - } if ( isset( $noKeyOptions['DISTINCT'] ) || isset( $noKeyOptions['DISTINCTROW'] ) ) { $startOpts .= 'DISTINCT'; } Since getting the code path in question executed is not easy (I can't reproduce the problem right now), can you see if this patch helps?
Patch filed as Gerrit change #21606
Thank you for your patch. It has been successfully merged into the git repository this night. In the future, if you're willing to submit patch, you can request a developer access, to be able to push code directly in our code review (Gerrit) infrastructure. [Removing Linux platform, issue isn't OS depending. Pruning keywords. Bug assigned to code submitter. Resolved fixed.]
(In reply to comment #8) > In the future, if you're willing to submit patch, you can request a developer > access, to be able to push code directly in our code review (Gerrit) > infrastructure. You can do so at https://www.mediawiki.org/wiki/Developer_access
*** Bug 46594 has been marked as a duplicate of this bug. ***
