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
    • Rank:
      24409

      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.

        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: