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@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@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@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l