happy.melon.wiki wrote:

Should this be "Deprecate Database::query()"??

If so, there are 99 uses in /phase3 (65 of them in /maintenance), and ~380 more in /extensions.

Occurences of "SELECT in /trunk/phase3[0]

Krinkle

[0] http://toolserver.org/~krinkle/wikimedia-svn-search/view.php?id=72&hash=102d19947f018328d3cbcedd14126b21

Is this a tracking bug? Are you guys offering to use this bug as a rallying point for your own efforts?

happy.melon.wiki wrote:

(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...

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

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.

2c..

• 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...

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 :)

Taking this bug for the core, at least in a first time.

(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?

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.

Adding Patch-For-Review as https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/21865/ exists