Last modified: 2014-02-28 14:56:54 UTC

Wikimedia Bugzilla is closed!

Wikimedia has migrated from Bugzilla to Phabricator. Bug reports should be created and updated in Wikimedia Phabricator instead. Please create an account in Phabricator and add your Bugzilla email address to it.
Wikimedia Bugzilla is read-only. If you try to edit or create any bug report in Bugzilla you will be shown an intentional error message.
In order to access the Phabricator task corresponding to a Bugzilla report, just remove "static-" from its URL.
You could still run searches in Bugzilla or access your list of votes but bug reports will obviously not be up-to-date in Bugzilla.
Bug 26670 - Remove direct calls to Database::query() and use inbuilt methods instead.
Remove direct calls to Database::query() and use inbuilt methods instead.
Status: NEW
Product: MediaWiki
Classification: Unclassified
Database (Other open bugs)
All All
: Normal enhancement (vote)
: ---
Assigned To: Nobody - You can work on this!
Depends on:
Blocks: code_quality
  Show dependency treegraph
Reported: 2011-01-11 19:35 UTC by Sam Reed (reedy)
Modified: 2014-02-28 14:56 UTC (History)
8 users (show)

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


Description Sam Reed (reedy) 2011-01-11 19:35:49 UTC
In 99% of cases, there should be no benefit from using raw sql over the nicer inbuilt methods which build the queries more programmatically.

Essentially a prequisite to doing any sort of DB schema abstraction (as Chad wanted to do in the near future).

Goes for both phase3 and extensions...
Comment 1 Happy-melon 2011-01-17 11:46:39 UTC
Should this be "Deprecate Database::query()"??

If so, there are 99 uses in /phase3 (65 of them in /maintenance), and ~380 more in /extensions.
Comment 2 Krinkle 2011-01-17 12:34:41 UTC
Occurences of "SELECT in /trunk/phase3[0]


Comment 3 Mark A. Hershberger 2011-01-22 04:25:49 UTC
Is this a tracking bug?  Are you guys offering to use this bug as a rallying point for your own efforts?
Comment 4 Happy-melon 2011-01-22 10:04:43 UTC
(In reply to comment #3)
> Is this a tracking bug?  Are you guys offering to use this bug as a rallying
> point for your own efforts?

That's why I asked.  Something specific like "deprecate Database::query()" would be a specific achievable goal; the current title sounds more like a tracking bug...
Comment 5 Sam Reed (reedy) 2011-01-23 23:27:52 UTC
Deprecating it isn't quite going to work, I think. In the end, it'll end up being probably a protected method, as we the way we do it is build a query string from the array, and then dump that out, maybe some refactoring relatedly.

It's sort of a tracking bug, but also not. Unless we're gonna start doing numerous bugs "Fix raw sql building in X", "Fix raw sql building in Y".

I know Roan and myself (among others), are slowly working through them

Also not sure if Chad has logged a feature request to move to a more abstract schema.. Either way, this would be a blocking bug for it

Certainly, having a more targeted approach wouldn't be a bad thing
Comment 6 Adam Wight 2011-02-20 09:33:37 UTC
Can I just say,
I was in your trenches a minute ago and the OP was not judging too harshly.  A few recommendations for minor recoding are listed below.

Great work fighting bloat in general--the code is not in any risk of becoming a BusinessObjectModel, leave that powerpoint in the suitcase thank you.

* Get all that DB_SLAVE logic outta here.  Instead, use flags which indicate when stale data is tolerable.
* Separating "bean" accessors and factory stuffing from app logic.
* Do object fields and database rows really have to be named differently??  If you accepted the dual object/row convention, here is one small, immediate win in not reenumerating SELECT fields:
  function Article::pageData() {
    $row = $dbr->selectRow(page_table_name, Article::newNullArticle(), $conditions);
(And then the real fun can be had: giving internals and calculated fields crazy, hidden names... ___'s the limit!)

* Type checking and coercion don't seem necessary.  Cannot the database constraints and driver do their job?

and more abstractly,
* Imagine the poor guy trying to write an extension into or onto the data layer.  Reducing the points of entry is key.  I think I saw some GOTO statements somewhere in there...
Comment 7 Niklas Laxström 2011-02-20 09:45:45 UTC
When I read the bug title I was so going to close this bug as wontfix, because suckiness is a favorable feature of raw SQL :)
Comment 8 Dereckson 2012-06-21 15:12:30 UTC
Taking this bug for the core, at least in a first time.
Comment 9 Andre Klapper 2013-01-11 15:26:09 UTC
(In reply to comment #8)
> Taking this bug for the core, at least in a first time.

Dereckson: Still working on this, or should this be reset to NEW/nobody?
Comment 10 Andre Klapper 2014-02-28 14:56:54 UTC
Dereckson: I am resetting the assignee of this issue to default because there has been no progress in the last months. Feel free to take it again when you are actually planning to fix this. Thanks.

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