General developer forum

Hi,

well, I agree about being possible to override them (by error) in long queries and so on, but also think it's possible (easier) to lose the order of "?" parameters (by error too), and that isn't preventing us to use them.

In any case (overridden named params or wrong order of ? params), it's a developer bug, like any other bug, and should be fixed. I don't see the "safer" point here really.

Also, talking about SQL standards... uhm... perhaps we should exclusively allow "?" parameters if we want to adhere 100% to it? If I don't remember wrong, from my old SQL sessions... question marks were the official ones.

I think I'll be using "?" all the time... but really the "repeated named ones" sound logic and natural... unless some database driver forbids it. Although that can be bypassed by renaming params internally if necessary.

uhm... uhm... ciao

I'm not convinced it is 'safer' but it makes queries harder to build when you are constructing a complex query in code. For example, here is what I just had to do to a query in lesson:

 $params = array("lessonid" => $lesson->id,"lessonid2" => $lesson->id);

 if (isset($userid)) {
 $params["userid"] = $userid;
 $params["userid2"] = $userid; $user = "AND u.id = :userid";
$fuser = "AND uu.id = :userid2";
 }
 else {
 $user="";
 $fuser="";
 }

 if ($lesson->retake) {
 if ($lesson->usemaxgrade) {
 $sql = "SELECT u.id, u.id AS userid, MAX(g.grade) AS rawgrade
 FROM {user} u, {lesson_grades} g
 WHERE u.id = g.userid AND g.lessonid = :lessonid
 $user
 GROUP BY u.id";
 } else {
 $sql = "SELECT u.id, u.id AS userid, AVG(g.grade) AS rawgrade
 FROM {user} u, {lesson_grades} g
 WHERE u.id = g.userid AND g.lessonid = :lessonid
 $user
 GROUP BY u.id";
 }
 unset($params['lessonid2']);
 unset($params['userid2']);
 } else {
 // use only first attempts (with lowest id in lesson_grades table)
 $firstonly = "SELECT uu.id AS userid, MIN(gg.id) AS firstcompleted
 FROM {user} uu, {lesson_grades} gg
 WHERE uu.id = gg.userid AND gg.lessonid = :lessonid2 $fuser GROUP BY uu.id";
$sql = "SELECT u.id, u.id AS userid, g.grade AS rawgrade
FROM {user} u, {lesson_grades} g, ($firstonly) f
WHERE u.id = g.userid AND g.lessonid = :lessonid AND g.id = f.firstcompleted
 AND g.userid=f.userid $user";
} 

That's a lot of opportunities to screw up (and maybe I did - if you spot one don't tell me, just fix it, please MDL-15877). The code is a whole lot nicer if you're allowed to reuse them, imo. Chance of wanting to do that is hugely greater than the chance of you constructing a long query with two different :userid parameters that you don't notice are the same, imo.

The database support issue is not relevant: Moodle already does loads of stuff before this gets anywhere near a database, right? It would be trivial to create a preprocessing function

munge_multi_params(&$sql,&$params)

that automatically expands the variable names to add a 2 or whatever in the sql and the $params.

All this said, I think it's ok to leave as is if we are really convinced it helps safety, but there needs to be an error message that hits this specific case because everybody is going to do it wrong, even if it's in the documentation. At present you get invalidparams. Instead it should be something like:

'Your database query <pre>SELECT * FROM whatever WHERE x=:apple AND y=:apple</pre> uses the same named parameter <b>:apple</b> more than once. This is not permitted. Please correct your query.'

(Could/probably should be developer debug instead of an actual print_error; print_error could use a generic as at present.)

--sam