Last modified: 2012-08-04 20:49:01 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T15453, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 13453 - rebuildrecentchanges broken on PostgreSQL
rebuildrecentchanges broken on PostgreSQL
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
Database (Other open bugs)
1.16.x
All All
: Normal minor (vote)
: ---
Assigned To: Nobody - You can work on this!
:
Depends on: 18291
Blocks: postgres
  Show dependency treegraph
 
Reported: 2008-03-20 16:45 UTC by OverlordQ
Modified: 2012-08-04 20:49 UTC (History)
3 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments
Backtract + output (2.07 KB, text/plain)
2009-06-17 05:00 UTC, OverlordQ
Details

Description OverlordQ 2008-03-20 16:45:54 UTC
====
Loading from page and revision tables...
$wgRCMaxAge=604800 (7 days)
Updating links and size differences...
Warning: pg_query(): Query failed: ERROR:  invalid input syntax for integer: "NULL" in /home/wiki/includes/DatabasePostgr   es.php on line 553
A database error has occurred
Query: UPDATE recentchanges SET rc_last_oldid=0,rc_new=1,rc_type=1,rc_old_len='NULL',rc_new_len='490' WHERE rc_cur_id=60    AND rc_this_oldid=102
Function:
Error: 1 ERROR:  invalid input syntax for integer: "NULL"

Backtrace:
#0 /home/wiki/includes/Database.php(799): DatabasePostgres->reportQueryError('ERROR:  invalid...', 1, 'UPDATE recentch...   ', '', false)
#1 /home/wiki/maintenance/rebuildrecentchanges.inc(108): Database->query('UPDATE recentch...')
#2 /home/wiki/maintenance/rebuildrecentchanges.inc(12): rebuildRecentChangesTablePass2()
#3 /home/wiki/maintenance/rebuildrecentchanges.php(18): rebuildRecentChangesTable()
#4 {main}
====

Problem is ~103-106 of maintenance/rebuildrecentchanges.inc

====
$size = $size ? $size : 'NULL';
....
rc_old_len='$lastSize',rc_new_len='$size' "
====

NULL shouldn't be in quotes.
Comment 1 Greg Sabino Mullane 2008-03-20 16:49:38 UTC
Fixed in r32226, thanks for the report.
Comment 2 OverlordQ 2008-03-20 16:57:34 UTC
===
"rc_old_len='$lastSize',rc_new_len=$size "
===

quotes around $lastSize will still throw the error as shown the SQL in comment 0, call it line 93 of same file if the page is new.
Comment 3 OverlordQ 2008-03-20 17:03:37 UTC
Working now :)
Comment 4 OverlordQ 2009-06-17 04:59:48 UTC
This seems to have regressed to being broken again.

I'll attach the output
Comment 5 OverlordQ 2009-06-17 05:00:56 UTC
Created attachment 6234 [details]
Backtract + output
Comment 6 OverlordQ 2009-06-17 05:08:25 UTC
Seems to be r49112 that causes this.  

'rc_cur_id'     => 'COALESCE(page_id, 0)'

was there a problem with that field being NULL vs 0?
Comment 7 Roan Kattouw 2009-06-30 10:51:03 UTC
(In reply to comment #6)
> Seems to be r49112 that causes this.  
> 
> 'rc_cur_id'     => 'COALESCE(page_id, 0)'
> 
> was there a problem with that field being NULL vs 0?
> 

Has to be I guess, but it seems weird to me because we use 0 in this role in a lot of other places as well (e.g. rev_user, which is either a valid user_id or 0). Is there even any benefit from adding these relations between tables in the postgres schema? The MySQL schema doesn't have them.
Comment 8 OverlordQ 2009-06-30 11:30:23 UTC
The benefit is getting warnings like this that your software is trying to insert data that doesn't make sense. Without strict mode MySQL is known to do some really silly things when inserting data and foreign keys would catch that.

The hackish way would be to create a stub page like is done for anonymous users and user_id 0 but that would likely do odd things elsewhere. 

I guess without doing something strange like splitting up the log table into actions against pages and actions against users, would be to drop the constraint.
Comment 9 Roan Kattouw 2009-06-30 19:09:25 UTC
(In reply to comment #8)
> The benefit is getting warnings like this that your software is trying to
> insert data that doesn't make sense.
That's not true. rc_cur_id=0 DOES make sense, it has a well-defined meaning. PostgreSQL should not try to enforce its conception of what makes sense over MW's. For that reason, the constraint should be dropped IMO, as you suggest.
Comment 10 OverlordQ 2009-10-28 18:24:35 UTC
Fixed in r58269

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links