Wikibase cannot save properties on SQLite with PHP 8.1
Closed, ResolvedPublicBUG REPORT
Actions

Description

Steps to replicate the issue (include links if applicable):

Install MediaWiki with a Wikibase repository on PHP 8.1 with a SQLite database backend
Attempt to create a property through Special:NewProperty

What happens?:

Creating Properties (and probably everything else) throws the following exception and fails:

[error] [796a23f655bcb0773f725fe4] /wiki/Special:NewProperty   PHP Warning: [data-update-failed]: A data update callback triggered an exception (Fail-safe exception. Avoiding infinite loop due to possibly undetectable existing records in master.
 It may be due to encoding incompatibility between database values and values passed in $neededRecords parameter.) [Called from Wikibase\Repo\Content\DataUpdateAdapter::doUpdate in /srv/mediawiki/extensions/Wikibase/repo/includes/Content/DataUpdateAdapter.php at line 63]
#0 [internal function]: MWExceptionHandler::handleError()
#1 /srv/mediawiki/includes/debug/MWDebug.php(500): trigger_error()
#2 /srv/mediawiki/includes/debug/MWDebug.php(196): MWDebug::sendMessage()
#3 /srv/mediawiki/includes/GlobalFunctions.php(1065): MWDebug::warning()
#4 /srv/mediawiki/extensions/Wikibase/lib/includes/Reporting/LogWarningExceptionHandler.php(28): wfLogWarning()
#5 /srv/mediawiki/extensions/Wikibase/repo/includes/Content/DataUpdateAdapter.php(63): Wikibase\Lib\Reporting\LogWarningExceptionHandler->handleException()
#6 /srv/mediawiki/includes/deferred/DeferredUpdates.php(536): Wikibase\Repo\Content\DataUpdateAdapter->doUpdate()
#7 /srv/mediawiki/includes/deferred/RefreshSecondaryDataUpdate.php(103): DeferredUpdates::attemptUpdate()
#8 /srv/mediawiki/includes/deferred/DeferredUpdates.php(536): RefreshSecondaryDataUpdate->doUpdate()
#9 /srv/mediawiki/includes/deferred/DeferredUpdates.php(419): DeferredUpdates::attemptUpdate()
#10 /srv/mediawiki/includes/deferred/DeferredUpdates.php(229): DeferredUpdates::run()
#11 /srv/mediawiki/includes/deferred/DeferredUpdatesScope.php(264): DeferredUpdates::{closure}()
#12 /srv/mediawiki/includes/deferred/DeferredUpdatesScope.php(196): DeferredUpdatesScope->processStageQueue()
#13 /srv/mediawiki/includes/deferred/DeferredUpdates.php(250): DeferredUpdatesScope->processUpdates()
#14 /srv/mediawiki/includes/MediaWiki.php(1123): DeferredUpdates::doUpdates()
#15 /srv/mediawiki/includes/MediaWiki.php(845): MediaWiki->restInPeace()
#16 /srv/mediawiki/includes/MediaWiki.php(588): MediaWiki->doPostOutputShutdown()
#17 /srv/mediawiki/index.php(53): MediaWiki->run()
#18 /srv/mediawiki/index.php(46): wfIndexMain()
#19 {main}
[warning] [data-update-failed]: A data update callback triggered an exception (Fail-safe exception. Avoiding infinite loop due to possibly undetectable existing records in master.
It may be due to encoding incompatibility between database values and values passed in $neededRecords parameter.) [Called from Wikibase\Repo\Content\DataUpdateAdapter::doUpdate in /srv/mediawiki/extensions/Wikibase/repo/includes/Content/DataUpdateAdapter.php at line 63]

What should have happened instead?:

Creating Properties (and probably everything else) works.

Software version (skip for WMF-hosted wikis like Wikipedia):

Mediawiki 1.38.2
PHP 8.1.9

Other information (browser name/version, screenshots, etc.):

The exception is 100% correct about the cause. Starting with PHP 8.1, integer columns return integers now. This breaks the comparison for the insertion into the wbt_text_in_lang table because the type id is specified as a string in DatabaseTermInLangIdsAcquirer::acquireTermInLangIdsInner. The difference in type causes the calculated hashes for the inserted row and needed row to differ, causing the infinite loop.

I'm not sure what the best way to fix this is as it only affects SQLite when running on PHP >= 8.1. MySQL, PostgreSQL, and PHP < 8.1 still require the type to be a string.

Details

	Subject	Repo	Branch	Lines +/-
	rdbms: Convert SQLite values to string	mediawiki/core	master	+7 -1

Customize query in gerrit

Event Timeline

CtrlZvi created this task.Aug 28 2022, 9:47 PM

Restricted Application added a project: [DEPRECATED] wdwb-tech. · View Herald TranscriptAug 28 2022, 9:47 PM

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Maintenance_bot added a project: Wikidata.Aug 28 2022, 10:45 PM

Digging a bit more, the type mismatch comes from loading the wbt_type table into the cache using the NameTableStore::loadTable() function. This function stores the returned values into an associative array by their key, and although PostgreSQL, MySQL, and SQLite prior to PHP 8.1 returned the ids as strings, the use of them as keys in the associate array results in them being cast to int. This is almost certainly why DatabaseTermInLangIdsAcquirer::acquireTermInLangIdsInner was casting back to a string.

It seems like this might be caused by using PDO for SQLite and not the SQLite3 type, though I don't know if that type would return strings or not.

Although discovered with Wikibase, this is not actually a Wikibase issue. PHP 8.1 introduces a breaking change into the SqliteResultWrapper. Leaving the Wikibase tags for now in case there's a local workaround that's preferred, but I think I now have an idea of how to fix this at the core level and hope to have a patch soon.

Restricted Application added a project: Performance-Team. · View Herald TranscriptAug 29 2022, 2:52 AM

Change 827502 had a related patch set uploaded (by CtrlZvi; author: CtrlZvi):

[mediawiki/core@master] rdbms: Convert SQLite values to string

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

gerritbot added a project: Patch-For-Review.Aug 29 2022, 1:57 PM

Thanks. We'll review the patch and backport to stable REL branches.

I don't think wikibase, or MediaWiki code generally, should be so tightly coupled to the driver returning strings.

Ideally, I somewhat prefer using native PHP integers...but the our sqlite/mysql/postgres Database subclasses should be consistent. We can possibly use ATTR_STRINGIFY_FETCHES for now.

I agree 100% with using the not being coupled to strings and using the native integer types. It seems like a positive change for a whole host of reasons. But that's a much larger change in a system I don't have experience in, and my experiments with trying to support both the strings returned by MySQL and PostgreSQL and the integers returned by SQLite were not successful.

In the specific case I found in Wikibase, the complexity comes from taking the results and using them as keys in an associative array, which autocasts returned strings to integer, then taking those keys and comparing them to what comes from the database in a type sensitive way. MySQL and PostgreSQL require the cast back into string for that comparison, SQLite requires no cast. If that's the only place that's sensitive like that, a local workaround could be used, but I'm assuming if there's one place that breaks, there are others, too.

I wonder if there's benefit in migrating the MySQL and PostgreSQL implementations to PDO? That would probably resolve the issue where they return strings and unify behavior. I suspect it might be at the expense of performance, though?

Addshore added a subscriber: WMDE-leszek.Aug 30 2022, 11:50 AM

Addshore subscribed.

Change 827502 merged by jenkins-bot:

[mediawiki/core@master] rdbms: Convert SQLite values to string

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

ReleaseTaggerBot added a project: MW-1.39-notes (1.39.0-wmf.28; 2022-09-05).Aug 31 2022, 2:00 PM

Maintenance_bot removed a project: Patch-For-Review.Aug 31 2022, 2:30 PM

Krinkle closed this task as Resolved.Sep 4 2022, 7:24 PM