Well... I was worried about people with tables like "onsomething" or
whatever... But I suppose I can test strictly for things with whitespace
around them "/(^|\s)(ON|JOIN)(\s|$)/i" that should solve the issue.
But I still think 3 is good. IMHO we should probably do 5 and 3 for best
practice and avoidance of future issues. If someone feels like it,
having 1 done would be great to. Think of it, helper functions for
various types of joins. Unions to!
Actually, I used to do something to the Database class. I actually had a
tweak for select and a few other functions. I basically added a extra
parameter $doQuery to the end which defaulted to true. Basically it
meant that you could explicitly send false and have select return a SQL
statement for you instead. That way you could go as far as to use
select, then manually UNION them together. But what would be absolutely
beautiful would be:
list( $user, $ug, $ugS) = $db->tableNameN('user', 'user_groups', $ug =
$db->tableName("`$wgSharedDB`.`{$wgSharedPrefix}user_groups`");
$res = $db->union(
$db-join( $db->select( $user, conds, etc..., /*doQuery=*/false ),
$ug, conds, etc..., /*doQuery=*/false ),
$db-join( $db->select( $user, conds, etc..., /*doQuery=*/false ),
$ugS, conds, etc..., /*doQuery=*/false ),
);
^_^ Which basically, would have select return sql which join uses to
construct a join statement for. And then union, well... Unions them
together.
~Daniel Friesen(Dantman) of:
-The Gaiapedia (
http://gaia.wikia.com)
-Wikia ACG on
Wikia.com (
http://wikia.com/wiki/Wikia_ACG)
-and
Wiki-Tools.com (
http://wiki-tools.com)
Simetrical wrote:
On Wed, May 7, 2008 at 7:43 PM, DanTMan
<dan_the_man(a)telus.net> wrote:
Oh... I forgot one:
5) Have tableName test for words like ON, JOIN, etc... which signal this
is a sql query, not a table.
This is the obvious way. If anyone has a database table called "JOIN"
they're already severe masochists and will probably enjoy being
punished by breakage of database functions. In particular, this would
enable LEFT JOINs to be done using the wrappers, which I'm pretty sure
is impossible right now.
On Wed, May 7, 2008 at 3:23 PM, Roan Kattouw <roan.kattouw(a)home.nl> wrote:
They're far from unneeded.
Database::select() is broken in that it adds
the backticks wrongly, generating invalid SQL [1]. I'll poke dantman to
fix this, but in the meantime we should have a working API.
You know, every other place in the entire code base where we have
inner joins, we just use implicit joins with WHERE. So in other
words, don't use ON at all. Instead of SELECT * FROM page INNER JOIN
revision ON page_id=rev_page WHERE page_id=731, use SELECT * FROM
page, revision WHERE page_id=731 AND page_id=rev_page (don't ever try
either for arbitrary pages, BTW :)). The queries are totally
equivalent. There should be no reason to use any other syntax for
inner joins, although stylistically I do prefer ON clauses.
I can't patch your code, however, since as usual, the API uses its own
syntax for database queries and non-API developers like me have no
idea how it works.
_______________________________________________
Wikitech-l mailing list
Wikitech-l(a)lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/wikitech-l