Moodle
  1. Moodle
  2. MDL-15198

non-standard sql in reportlib.php

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.9.1
    • Fix Version/s: 1.9.2
    • Component/s: Quiz
    • Labels:
      None
    • Database:
      Microsoft SQL
    • Affected Branches:
      MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE

      Description

      the function quiz_report_qm_filter_subselect in mod/quiz/report/reportlib.php on line 159 bulids a sql query with the MySQL-ism LIMIT. This query breaks on other databases.

        Gliffy Diagrams

          Activity

          Hide
          Tim Hunt added a comment -

          Yes, Jamie, there is a function in dmllib for doing the correct limit syntax on different databases, if I recall correctly.

          Show
          Tim Hunt added a comment - Yes, Jamie, there is a function in dmllib for doing the correct limit syntax on different databases, if I recall correctly.
          Hide
          Jamie Pratt added a comment -

          There are no longer any functions in dmllib of course in HEAD. And I can't see anything in the $DB object that would do the equivalent for adding a LIMIT to a subquery.

          sql_paging_limit($page, $recordsperpage) in dmllib in 1.9 was deprecated in Moodle 1.7 and won't do anything different.

          Show
          Jamie Pratt added a comment - There are no longer any functions in dmllib of course in HEAD. And I can't see anything in the $DB object that would do the equivalent for adding a LIMIT to a subquery. sql_paging_limit($page, $recordsperpage) in dmllib in 1.9 was deprecated in Moodle 1.7 and won't do anything different.
          Hide
          Jamie Pratt added a comment -

          Added Eloy as a watcher of this issue.

          How should I do a LIMIT on a sub query in a cross db compatible way?

          Do we need a new method on $DB??

          Can't find any methods to return the sql for the LIMIT clause. The main queries all pass the LIMIT params through to adodb it seems.

          Show
          Jamie Pratt added a comment - Added Eloy as a watcher of this issue. How should I do a LIMIT on a sub query in a cross db compatible way? Do we need a new method on $DB?? Can't find any methods to return the sql for the LIMIT clause. The main queries all pass the LIMIT params through to adodb it seems.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          This cannot be solved. Limit clauses in subqueries aren't supported at all.

          And cannot be emulated (like ADOdb does for DBs not having them natively), but only for main queries.

          The only way I see is to execute the subquery as one main query so you can use the $limitfrom, $limitnum parameters). And then build a list of IDs to be used with one IN (...) clause in the original query. Not optimal but cannot imagine another way.

          Note that my first thought was "wow, limit in subqueries, what a crazy coding style". Does it really have sense? (I haven't looked at code, but it's the first time I hear about one logic needing that). Sincerely.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - This cannot be solved. Limit clauses in subqueries aren't supported at all. And cannot be emulated (like ADOdb does for DBs not having them natively), but only for main queries. The only way I see is to execute the subquery as one main query so you can use the $limitfrom, $limitnum parameters). And then build a list of IDs to be used with one IN (...) clause in the original query. Not optimal but cannot imagine another way. Note that my first thought was "wow, limit in subqueries, what a crazy coding style". Does it really have sense? (I haven't looked at code, but it's the first time I hear about one logic needing that). Sincerely. Ciao
          Hide
          Jamie Pratt added a comment -

          mssql seems to support select top 1 in sub queries. Is there not something similar on other dbs.

          I'm using this to get data about the attempt that is actually graded. I want to select one attempt for each user for instance if the grading method is highest grade then I want to select the attempt with the highest grade, AND if there is more than one attempt with this grade, the first of them. It is the second part that is difficult.

          Show
          Jamie Pratt added a comment - mssql seems to support select top 1 in sub queries. Is there not something similar on other dbs. I'm using this to get data about the attempt that is actually graded. I want to select one attempt for each user for instance if the grading method is highest grade then I want to select the attempt with the highest grade, AND if there is more than one attempt with this grade, the first of them. It is the second part that is difficult.
          Hide
          Jamie Pratt added a comment -

          Wrote new sql that doesn't require a LIMIT clause.

          It looks like this :

          $qmselect = "qa.$field1 = (SELECT $aggregator1(qa2.$field1) FROM {$CFG->prefix}quiz_attempts qa2 WHERE qa2.quiz = $quizidsql AND qa2.userid = $useridsql) AND " .
          "qa.$field2 = (SELECT $aggregator2(qa3.$field2) FROM {$CFG->prefix}quiz_attempts qa3 WHERE qa3.quiz = $quizidsql AND qa3.userid = $useridsql AND qa3.$field1 = qa.$field1)";

          Where for example for quizzes with grading method 'hishest grade' the variables are :

          $field1 = 'sumgrades';
          $field2 = 'timestart';
          $aggregator1 = 'MAX';
          $aggregator2 = 'MIN';

          Hopefully this will work across all dbs.

          Show
          Jamie Pratt added a comment - Wrote new sql that doesn't require a LIMIT clause. It looks like this : $qmselect = "qa.$field1 = (SELECT $aggregator1(qa2.$field1) FROM {$CFG->prefix}quiz_attempts qa2 WHERE qa2.quiz = $quizidsql AND qa2.userid = $useridsql) AND " . "qa.$field2 = (SELECT $aggregator2(qa3.$field2) FROM {$CFG->prefix}quiz_attempts qa3 WHERE qa3.quiz = $quizidsql AND qa3.userid = $useridsql AND qa3.$field1 = qa.$field1)"; Where for example for quizzes with grading method 'hishest grade' the variables are : $field1 = 'sumgrades'; $field2 = 'timestart'; $aggregator1 = 'MAX'; $aggregator2 = 'MIN'; Hopefully this will work across all dbs.
          Hide
          Alan Trick added a comment - - edited

          Not fixed, this SQL still doesn't work on MSSQL.

          I'm getting the following:

          SELECT ... as uniqueid,
          qa.timestart = (
          SELECT MAX(qa2.timestart)
          FROM mdl_quiz_attempts qa2
          WHERE qa2.quiz = qa.quiz AND qa2.userid = qa.userid) AND qa.id = (
          SELECT MAX(qa3.id)
          FROM mdl_quiz_attempts qa3
          WHERE qa3.quiz = qa.quiz AND qa3.userid = qa.userid AND qa3.timestart = qa.timestart
          ) AS gradedattempt,
          ...

          (...'s are snipped bits of code). I don't think that's normal.

          Show
          Alan Trick added a comment - - edited Not fixed, this SQL still doesn't work on MSSQL. I'm getting the following: SELECT ... as uniqueid, qa.timestart = ( SELECT MAX(qa2.timestart) FROM mdl_quiz_attempts qa2 WHERE qa2.quiz = qa.quiz AND qa2.userid = qa.userid) AND qa.id = ( SELECT MAX(qa3.id) FROM mdl_quiz_attempts qa3 WHERE qa3.quiz = qa.quiz AND qa3.userid = qa.userid AND qa3.timestart = qa.timestart ) AS gradedattempt, ... (...'s are snipped bits of code). I don't think that's normal.
          Hide
          Tim Hunt added a comment -

          The problem is wanting to have the result of a boolean test as one of the fields selected. A simpler example that would fail on MSSQL is

          SELECT id, (name = 'frog') AS thisisthefrog FROM table

          The solution for MSSQL server is

          SELECT id, (CASE WHEN (name = 'frog') THEN 1 ELSE 0 END) AS thisisthefrog FROM table

          Do we need another sql_XXX compatibility function to deal with this? Something like sql_select_condition($condition) that in this case we would call with $condition = "name = ?"

          Show
          Tim Hunt added a comment - The problem is wanting to have the result of a boolean test as one of the fields selected. A simpler example that would fail on MSSQL is SELECT id, (name = 'frog') AS thisisthefrog FROM table The solution for MSSQL server is SELECT id, (CASE WHEN (name = 'frog') THEN 1 ELSE 0 END) AS thisisthefrog FROM table Do we need another sql_XXX compatibility function to deal with this? Something like sql_select_condition($condition) that in this case we would call with $condition = "name = ?"
          Hide
          Eloy Lafuente (stronk7) added a comment -

          I'm not sure if we need this sql_xxx() helper function. AFAIK, "CASE" expressions are full cross-db, so something like:

          SELECT onefield,
          CASE
          WHEN anotherfield = 'value1' THEN 'result1'
          WHEN anotherfield = 'value2' THEN 'result2'
          ....
          ELSE 'defaultresult'
          END
          FROM sometable

          should work fine under any DB (not really tested).

          But note I don't know how that will "fix" the problematic query above. Not sure if you can mix it with subqueries and so.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - I'm not sure if we need this sql_xxx() helper function. AFAIK, "CASE" expressions are full cross-db, so something like: SELECT onefield, CASE WHEN anotherfield = 'value1' THEN 'result1' WHEN anotherfield = 'value2' THEN 'result2' .... ELSE 'defaultresult' END FROM sometable should work fine under any DB (not really tested). But note I don't know how that will "fix" the problematic query above. Not sure if you can mix it with subqueries and so. Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Just for confirmation... I've executed this:

          SELECT id, username,
          CASE
          WHEN username = 'admin' THEN 'ADMIN'
          WHEN username = 'alu1' THEN 'ALU1'
          ELSE 'NO ADMIN NOR ALU1'
          END
          FROM mdl_user;

          against all DBs and have worked ok. Re-ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Just for confirmation... I've executed this: SELECT id, username, CASE WHEN username = 'admin' THEN 'ADMIN' WHEN username = 'alu1' THEN 'ALU1' ELSE 'NO ADMIN NOR ALU1' END FROM mdl_user; against all DBs and have worked ok. Re-ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          And also for confirmation... I just tried this:

          SELECT u.id, u.username,
          CASE
          WHEN u.username = 'admin' THEN (SELECT t.country from mdl_user t where t.username = u.username)
          WHEN u.username = 'alu1' THEN 'ALU1'
          ELSE 'NO ADMIN NOR ALU1'
          END
          FROM mdl_user u;

          And seems to work too under MySQL, PG, Oracle and MSSQL... just in case it helps...re-re-ciao

          Show
          Eloy Lafuente (stronk7) added a comment - And also for confirmation... I just tried this: SELECT u.id, u.username, CASE WHEN u.username = 'admin' THEN (SELECT t.country from mdl_user t where t.username = u.username) WHEN u.username = 'alu1' THEN 'ALU1' ELSE 'NO ADMIN NOR ALU1' END FROM mdl_user u; And seems to work too under MySQL, PG, Oracle and MSSQL... just in case it helps...re-re-ciao
          Hide
          Jamie Pratt added a comment -

          Wrapped quiz grading method select logic in a CASE statement.

          Show
          Jamie Pratt added a comment - Wrapped quiz grading method select logic in a CASE statement.

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: