Last modified: 2014-09-24 01:23:54 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 T10859, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 8859 - Database::update should take array of tables too
Database::update should take array of tables too
Status: REOPENED
Product: MediaWiki
Classification: Unclassified
Database (Other open bugs)
unspecified
All All
: Low enhancement with 1 vote (vote)
: ---
Assigned To: Nobody - You can work on this!
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-02-02 11:38 UTC by Andrew Dunbar
Modified: 2014-09-24 01:23 UTC (History)
3 users (show)

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


Attachments
Patch for Database::update array of tables (741 bytes, patch)
2007-02-28 00:26 UTC, Andrew Dunbar
Details

Description Andrew Dunbar 2007-02-02 11:38:42 UTC
Most of the Database:: wrapper functions for the various SQL queries allow
arrays for most of their arguments (tables, where, etc). Database::update does
not support multiple tables. This is commonly needed in SQL if not so far needed
in MediaWiki. (My extension does need it).
Comment 1 Andrew Dunbar 2007-02-28 00:26:04 UTC
Created attachment 3275 [details]
Patch for Database::update array of tables
Comment 2 p858snake 2011-04-30 00:09:53 UTC
*Bulk BZ Change: +Patch to open bugs with patches attached that are missing the keyword*
Comment 3 Olivier Finlay Beaton 2011-10-02 18:52:42 UTC
Patch from 2007, 4 years ago. Really Database::update should take $tables array and $join_conds, just like Database::select does.

My scenario, update touch on all pages belonging to a category: UPDATE page INNER JOIN categorylinks SET page_touched = CURRENT_TIMESTAMP() WHERE page.page_id = categorylinks.cl_from AND categorylinks.cl_to = 'MyCategoryName';
Comment 4 Sumana Harihareswara 2011-11-10 02:52:44 UTC
Thank you for the patch, Andrew.  Adding need-review keyword to signal developers to review it.  I'm sorry for the wait.
Comment 5 Sam Reed (reedy) 2011-11-19 20:17:50 UTC
Applied in r103706, also added to relevant places
Comment 6 Brion Vibber 2011-11-23 02:13:11 UTC
This lacks join conditions etc as they're used on Database::select(). Is this actually useful as is without that?

The scenario in comment 3 with an inner join ..... should I think be able to get away with just what's in the where clause. But outer joins etc would need more...
Comment 7 Olivier Finlay Beaton 2011-11-23 03:07:26 UTC
I can't do multiple tables unless parameter 1 lets me do $table=array(), which it did not. You're correct, once I have $table with an array, I can do INNER JOIN using $conds as usual. But until then nada.
Comment 8 Daniel Friesen 2011-11-24 04:13:47 UTC
Seams like we should keep this open until $join_conds is added to Database::update.
Comment 9 Sam Reed (reedy) 2011-12-02 00:30:46 UTC
Reverted in r104926

Not coming back

Only seemingly supported by MySQL, so unless we did workarounds for non MySQL backends... Err, yeah - no.
Comment 10 Olivier Finlay Beaton 2011-12-02 01:11:19 UTC
This is not MySQL-specific. It's supported by Microsoft SQL Server and PostgreSQL. But only through sub-queries on SQLite and Oracle. So since support is at best 50/50 without workarounds, I can see why MW shouldn't include pure join support.

It seems to me like the sub-query method is universally supported by everyone, instead of a join syntax. It can be a tiny bit complicated (there may be issues with keys depending on how you write the query?), is it worth standardizing it and hiding the implementation detail from the user so they don't have to write SQL? If it's a lot more taxing and should be discouraged, perhaps a separate function?

So what's the recommended workaround/method? Do your SELECT with joins ahead of time, cache the results, then run a massive ugly UPDATE statement?

My actual current example: UPDATE page
INNER JOIN categorylinks SET page_touched = CURRENT_TIMESTAMP() WHERE
page.page_id = categorylinks.cl_from AND categorylinks.cl_to =
'MyCategoryName';

Since the API didn't let me do it, and didn't tell my WHY I couldn't do it, I just went ahead and did it anyways, this was the wrong option. If we have an abstraction API with clear limitations, we should be clearly pointing out why certain functionality isn't included (and pointing them to how they should do it, say on a wiki page).

Even if this is a documentation problem I think it's far from resolved.

Personally I think even if we have to run multiple statements, we should just do it for the user auto-magically instead of require them to jump through hoops. It makes a ton of sense API wise and it's already confused a lot of people and it would be safer.

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


Navigation
Links