Last modified: 2013-03-01 15:25:21 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 T44321, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 42321 - SQLStore3 notice when running SMW_refreshData.php
SQLStore3 notice when running SMW_refreshData.php
Status: RESOLVED FIXED
Product: MediaWiki extensions
Classification: Unclassified
Semantic MediaWiki (Other open bugs)
unspecified
All All
: High major (vote)
: ---
Assigned To: Jeroen De Dauw
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-11-21 10:32 UTC by Jeroen De Dauw
Modified: 2013-03-01 15:25 UTC (History)
4 users (show)

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


Attachments
Error log (2.31 KB, text/plain)
2012-11-22 15:02 UTC, Nischay Nahata
Details

Description Jeroen De Dauw 2012-11-21 10:32:45 UTC
http://pastebin.com/nHqWzL8Z

Got this for cbdb751c4f678a521fdc70fbe5b86a1e1df7d7ae (although the issue was not introduced in that revision)
Comment 1 Markus Krötzsch 2012-11-22 08:31:52 UTC
Please do not use pastebin for reporting bugs. Content vanishes after some time there and the bug report will be meaningless.
Comment 2 Markus Krötzsch 2012-11-22 08:36:11 UTC
The error as such is not clear to me. Maybe it has to do with the contents of some of the arrays that are diffed there. Is there a way to reproduce this for further tests? What we could use is a dump of the arguments passed to array_diff_assoc() here.
Comment 3 Markus Krötzsch 2012-11-22 08:45:55 UTC
Ok, confirmed. The reason is the stupid implementation of PHP's array_diff_assoc. We use it for comparing two arrays that have entries of the form "key => value" where value is another array and key is a hash that uniquely identifies this array. We use array_diff_assoc since it compares keys and we don't want PHP to compare the rest (we already ensured that this matches if the keys match). PHP, however, compares both the keys and the values, but does this by casting values to string first and then comparing the strings. This leads to a notice in PHP >5.4.0. The result is correct for us, but we should still use another method for computing the diff here.
Comment 4 Nischay Nahata 2012-11-22 09:07:20 UTC
(In reply to comment #3)
> Ok, confirmed. The reason is the stupid implementation of PHP's
Markus, I thought you knew about this. I had used these functions too initially (during GSoC) but removed them later on.

array_diff, array_diff_assoc should never be used ;)
Comment 5 Nischay Nahata 2012-11-22 15:02:28 UTC
Created attachment 11408 [details]
Error log

Attached the error log from Pastebin
Comment 6 Jeroen De Dauw 2013-01-07 12:36:49 UTC
> array_diff, array_diff_assoc should never be used ;)

Why not? I'd say you should use them when you want to diff an array rather then doing it yourself.
Comment 7 Jeroen De Dauw 2013-01-07 13:41:39 UTC
Markus, not sure why you set this to minor. This is a bug that happens all the time for a lot of people. Turning off the notice in production environments is a good idea but does not fix it.

Doing a dump of the relevant values when a warning is triggered:

array (size=1)
  'daffc259e178764afcf104cc040ca62f' => 
    array (size=3)
      's_id' => string '51' (length=2)
      'o_serialized' => string '1/2013/1/7/13/32/4' (length=18)
      'o_sortkey' => float 2456300.0639352

array (size=1)
  'ea9c995574acd1c284d1f28b8e509b09' => 
    array (size=3)
      's_id' => string '51' (length=2)
      'o_serialized' => string '1/2013/1/7/13/29/56' (length=19)
      'o_sortkey' => string '2456300.0624537' (length=15)


Looks like the code is expecting only the inner arrays, and is thus currently not functioning properly. I am afraid that this might lead to things that need updating not getting updated, inc when this is called from the refresh job.
Comment 8 Jeroen De Dauw 2013-01-07 13:42:43 UTC
That dump is of $newData[$tableName] and $oldTableData
Comment 9 Nischay Nahata 2013-01-07 14:25:37 UTC
(In reply to comment #6)
> > array_diff, array_diff_assoc should never be used ;)
> 
> Why not? I'd say you should use them when you want to diff an array rather
> then
> doing it yourself.

because PHP's implementation of these methods is a bit tricky- refer to the comments on the PHP manual.
Enough of off-topic rant now :)
Comment 10 Jeroen De Dauw 2013-01-07 14:33:25 UTC
You need to be aware of what it does and does not do. In my experience, most usecases fall well within what it handles nicely.
Comment 11 Jeroen De Dauw 2013-01-07 14:34:19 UTC
This seems to fix the issue: https://gerrit.wikimedia.org/r/#/c/42547/

Ran into a pile of other issues though. To bad I can't attach a grumpy-cat image to the commit :)
Comment 12 Jeroen De Dauw 2013-01-13 04:46:16 UTC
Making as fixed as I'm not seeing this anymore. Please reopen if you still run into this.
Comment 13 Jeroen De Dauw 2013-03-01 15:25:21 UTC
This issue is still present on 1.8.x. It however is not serious. Just turn notices off. 

At the top of localsettings put:

error_reporting(0);
ini_set("display_errors", 0);

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


Navigation
Links