Ok, then how about we draft up a short list of symbols to test, or not test against. Simplifying down to word breaks or such leaves with to much that could break. As for spaces inside of table names, I'm pretty sure anyone insane enough to use a space inside of a name is already going to be having issues with SQL. If you're using ` on ` as a table name, you're pretty sure things are going to break if you don't go and quote that yourself. Underscores or dashes are always preferred over spaces.
~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 Thu, May 8, 2008 at 4:21 AM, DanTMan dan_the_man@telus.net wrote:
I did think of \b, however I'm wary of it. I tied the test to whitespace or starting/ending of string because of the remote case where someone has something like -on- as their table name. Because \b matches breaks there are a fair number of symbols which it also considers to be word breaks because they are not word characters. And it's possible for some of those to be valid inside of table names.
The table can also be named ` on `. Whereas on the other hand, INNER JOIN table ON(cond) is perfectly valid syntax which your test would get wrong.
But what I don't see a single instance of, is what MediaWiki has done a bit of. Converting the construction of SQL in strings, into a set of helper functions which aid in readability, portability, and cross-compliance of most sql calls. Which even have the amazing bonus of properly escaping things where they come in to eliminate most of the issues of accidentally forgetting to secure something which could be used for SQL injection. Honestly, would you rather call $db->select, or have to remember to mysql_real_escape_string every bit of data manually? The former always escapes cleanly and avoids having you forget to do it.
Actually, I think the SQL queries themselves are most readable, by and large. Escaping can be done printf-style, with query( "SELECT * FROM table WHERE name=%s", $var ), the way Python and C (for instance) do it. (Of course, I imagine that might be fairly tricky with some complicated dynamically-adjusted queries.) The abstraction is maybe the most valuable part, so that things like FORCE INDEX can be included for MySQL and easily removed by other engines -- without having to parse a string.
We may also want to add some helpers for a few things people might want to do in conditions. Basically IN, AND, OR, >, <, not, and a few other things I've found myself needing to construct a mini condition string rather than using the nice assignment that escapes everything.
You can do IN and AND:
$conds = array( 'id' => array( 1, 2, 3 ), 'foreignkey' => 17 )
gives "id IN (1,2,3) AND foreignkey = 17" or equivalent. <, >, OR, and perhaps NOT I occasionally miss, but I don't think there's any good reason to abstract those if it would be awkward (and I bet it would be).
On Thu, May 8, 2008 at 5:56 AM, Roan Kattouw roan.kattouw@home.nl wrote:
Unfortunately, this isn't about INNER JOINs (which I'm smart enough to rewrite), but about LEFT JOINs, which are pretty much impossible to construct cleanly right now. I would very much welcome support for LEFT JOINs in the Database class, so I can port it to ApiQueryBase as well.
Yes, sorry. The first example was in fact an INNER JOIN. The others were LEFT JOINs, which usually in core code seem to end up just using query(). But that's obviously not ideal for anyone. I think with the changes to tableName we can now use LEFT JOIN with select(), yes?
On Thu, May 8, 2008 at 6:11 AM, DanTMan dan_the_man@telus.net wrote:
It looks like Semantic MediaWiki is sending in an ugly "DISTINCT $pagetable.page_title as title, $pagetable.page_namespace as namespace, $pagetable.page_id as id", So now I need to tweak the test to check for DISTINCT and AS or else we'll have yet another bug report.
That's the field list, though, not the table list. Why would your change affect it? I'd imagine that that field list could become array( 'page_title', 'page_namespace', 'page_id' ), with $options []= 'DISTINCT', in any case (and surrounding code changed to not use the aliases). You could commit the change yourself, you know, SMW is in the repo.
Hmm... it is inside of the fields, not the table. Well, I don't know what was wrong, but I do know that when I applied that change SMW suddenly started working for me, so it can't hurt.
What I find horribly disturbing is this:
|$res = $db->query( "SELECT title FROM $tablename", 'SMW::getQueryResult:DEBUG');
|
That's a blatant select statement which should be sent through $db->select.
But I'll look into improvements into Database methods sometime later. I'd honestly like to abstract queries away from raw SQL into using methods. We're doing PHP, not SQL, let the SQL portion of the code handle SQL while we write in PHP. Doing it right could also potentially allow the complex things like SMW, API, and DPL which build complex queries to abstract those as well with the helper functions, or heck... We could come up with some sort of actual Query builder class.
Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l