Handlers of the LoadExtensionSchemaUpdates hook are supposed to add callbacks to DatabaseUpdater. The callbacks are called during the relevant install stage. Checks for table existence, field existence, etc. are supposed to be done during the callback, not during LoadExtensionSchemaUpdates. Doing the checks late allows the callback to take into account the effect of prior schema updates.
Unfortunately this expectation was not properly documented and is not enforced. Many extensions call tableExists() during hook execution to decide whether or not to add an update. Wikibase actually ran the whole update from the hook, i.e. during object construction, with catastrophic results (T249565). T157651 described this as an issue with sql.php, but the problem is deeper than that.
Code review observations:
- Many extensions call $updater->getDB()->getType()
- Some extensions type-hint the $updater parameter
- Some extensions call $updater->tableExists()
- ContributionTracking optionally gets its own connection to a foreign DB and updates that immediately (not in a callback)
- Echo calls $updater->getDB()->indexExists()
- EmailCapture calls $updater->getDB()->tableExists()
- QuizGame calls $updater->getDB()->fieldExists()
I think the simplest way to deal with this would be to run LoadExtensionSchemaUpdates later: from doUpdates() instead of __construct(). Then it will be safe, if not strictly correct, to access the database. Apparently nothing actually needs to inspect the update list after construction, so there's no need to call the hook so early.
Eventually, LoadExtensionSchemaUpdates should be deprecated. It should be replaced by either a completely declarative interface (e.g. a JSON file registered in extension.json) or a functional interface, in which the extension is only called when it is safe to directly perform conditional updates, or two distinct interfaces serving both styles.