Description
Details
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | None | T330640 Migrate Database::update usages to UpdateQueryBuilder | |||
| Resolved | Scardenasmolinar | T331279 Migrate Database::update usages to UpdateQueryBuilder in PageTriage |
Event Timeline
Change 910747 had a related patch set uploaded (by Ladsgroup; author: Amir Sarabadani):
[mediawiki/core@master] Migrate several $db->update() calls to UpdateQueryBuilder
Change 910747 merged by jenkins-bot:
[mediawiki/core@master] Migrate several $db->update() calls to UpdateQueryBuilder
What about InsertQueryBuilder and DeleteQueryBuilder? Treating SELECTs differently than INSERTs, UPDATEs and DELETEs makes sense, as SELECTs are used a lot more, and they also behave differently (they return values). Treating UPDATEs differently than INSERTs and DELETEs, on the other hand, makes no sense to me. Of course, it can temporarily happen that UpdateQueryBuilder is ready but the other two aren’t yet, but then the tasks for those two should exist and preferably being worked on (or, if you did this in your volunteer capacity and don’t have time/motivation to add the missing builders, at least ready to be picked up).
Actually earlier today I made the patch for DeleteQueryBuilder (feel free to review/merge). Insert is more complicated because you have the upsert usecase that I'm not sure how to tackle (a different query builder, same but with extra methods, etc?) I want to bother Tim about this first.
The work for query builders is happening, and it's in my work capacity but we have to do a lot other stuff as well, so it's a bit of slow and steady. Specially since designing new interfaces require more future-looking thinking to avoid mess in the future (unlike refactors and clean ups).
I agree the clean up for write query builders better to be batched and that's why I haven't created subtickets for each WMF-deployed extension yet but the work itself can happen and the more clean ups we do, the better.
I’m not a DB expert, but I’ll take a look at it.
If the work is ongoing, then the problem is discoverability: without any Phabricator tasks, it’s virtually impossible to find the patches unless one knows the Gerrit change IDs or at least the approximate time they were uploaded (which could have been at any time in the last two months). Also, your concerns about upsert are best discussed in a Phabricator ticket – either one focusing on creating InsertQueryBuilder or a parent epic task about all query builders (where it can be decided whether to create tasks for separate InsertQueryBuilder and UpsertQueryBuilder, or create one task for an InsertUpsertQueryBuilder). So I kindly ask you to create one task for DeleteQueryBuilder and another one where the insert/upsert work can be tracked (InsertQueryBuilder-specific or epic). Thanks in advance!
Specially since designing new interfaces require more future-looking thinking to avoid mess in the future (unlike refactors and clean ups).
100% agree; I consider designing “being worked on”. I’m not impatient, I just don’t want to have this remain half-done in the long term.
T335377: Create QueryBuilders for common database queries
Specially since designing new interfaces require more future-looking thinking to avoid mess in the future (unlike refactors and clean ups).
100% agree; I consider designing “being worked on”. I’m not impatient, I just don’t want to have this remain half-done in the long term.
Oh I assure you, it's happening. This has been part of several-year-long roadmap of improving databases in mw (T297633) and is part of my responsibilities to see it through.
Change 919888 had a related patch set uploaded (by Ladsgroup; author: Amir Sarabadani):
[mediawiki/core@master] RollbackPage: Switch to use UpdateQueryBuilder
Change 919888 merged by jenkins-bot:
[mediawiki/core@master] RollbackPage: Switch to use UpdateQueryBuilder
Change 928169 had a related patch set uploaded (by Ladsgroup; author: Amir Sarabadani):
[mediawiki/core@master] Migrate Database::update() to UpdateQueryBuilder
Change 928486 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):
[mediawiki/core@master] Clean up UpdateQueryBuilder usage
Change 928169 merged by jenkins-bot:
[mediawiki/core@master] Migrate Database::update() to UpdateQueryBuilder
Change 928486 merged by jenkins-bot:
[mediawiki/core@master] Clean up UpdateQueryBuilder usage
Change 928505 had a related patch set uploaded (by Ladsgroup; author: Amir Sarabadani):
[mediawiki/core@master] tests: Migrate Database::update usages to UpdateQueryBuilder
Change 928505 merged by jenkins-bot:
[mediawiki/core@master] tests: Migrate Database::update usages to UpdateQueryBuilder
Change 928829 had a related patch set uploaded (by Zabe; author: Zabe):
[mediawiki/core@master] Migrate more usages of Database::update() to UpdateQueryBuilder
Change 928845 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/Wikibase@master] Use UpdateQueryBuilder
Change 928829 merged by jenkins-bot:
[mediawiki/core@master] Migrate more usages of Database::update() to UpdateQueryBuilder
Change 928845 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Use UpdateQueryBuilder
Change 938233 had a related patch set uploaded (by Ladsgroup; author: Amir Sarabadani):
[mediawiki/core@master] tests: Migrate calls to Database::update to UpdateQueryBuilder
Change 938233 merged by jenkins-bot:
[mediawiki/core@master] tests: Migrate calls to Database::update to UpdateQueryBuilder
All patches merged; assuming this is resolved. If not, then please create subtask(s) for whatever codebase(s) are still to update. Thanks!