Moodle
  1. Moodle
  2. MDL-29529

Error reading from database - Unknown column 'status' in 'order clause'

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 1.9.14, 2.0.5, 2.1.2, 2.2
    • Fix Version/s: 1.9.15, 2.0.6, 2.1.3
    • Component/s: Assignment (2.2)
    • Labels:
    • Environment:
      Moodle 2.1.1+, build 20110916, upgraded from 2.1.
      Debian GNU/Linux 6.0.2 (squeeze); Apache/2.2.16 (Debian); PHP 5.3.3-7+squeeze3 with Suhosin-Patch; mysql Ver 14.14 Distrib 5.1.49.
    • Database:
      MySQL
    • Testing Instructions:
      Hide

      1 Click on an Assignment activity with submitted assignments.
      2 Click on the "View n submitted assignments" link. A summary table appears.
      3 Click on the 'Status' column header, to sort the table by status.
      4 Verify that the column sorts by status.
      5 Click on any entry in the status column.
      6 Verify that the page loads without error.

      Show
      1 Click on an Assignment activity with submitted assignments. 2 Click on the "View n submitted assignments" link. A summary table appears. 3 Click on the 'Status' column header, to sort the table by status. 4 Verify that the column sorts by status. 5 Click on any entry in the status column. 6 Verify that the page loads without error.
    • Workaround:
      Hide

      No workaround, but initially the table works, and can be sorted by other criteria without this issue.

      Show
      No workaround, but initially the table works, and can be sorted by other criteria without this issue.
    • Affected Branches:
      MOODLE_19_STABLE, MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE, MOODLE_20_STABLE, MOODLE_21_STABLE
    • Pull from Repository:
    • Rank:
      19006

      Description

      When ordering submitted assignments by status, an error is received:

      "Error reading from database - Unknown column 'status' in 'order clause'"

      Using the back button on this page causes the page which previously worked fine to fail with an identical error.

      Full error:

      Debug info: Unknown column 'status' in 'order clause'
      SELECT u.id,u.picture,u.firstname,u.lastname,u.imagealt,u.email,
      s.id AS submissionid, s.grade, s.submissioncomment,
      s.timemodified, s.timemarked FROM mdl_user u LEFT JOIN mdl_assignment_submissions s ON u.id = s.userid
      AND s.assignment = 591 WHERE u.id IN (631,868,2051,2057,2058,2070,2071,2087,2094,2095,2096,2097,2098,2099,2100,2102,2434,2436,2833,2836,2838,3112,3113,3115,3226,3373,3967,915,3930,4226) ORDER BY status ASC, lastname ASC
      [array (
      )]
      Stack trace:
      
          line 394 of /lib/dml/moodle_database.php: dml_read_exception thrown
          line 795 of /lib/dml/mysqli_native_moodle_database.php: call to moodle_database->query_end()
          line 1325 of /mod/assignment/lib.php: call to mysqli_native_moodle_database->get_records_sql()
          line 649 of /mod/assignment/lib.php: call to assignment_base->display_submissions()
          line 57 of /mod/assignment/submissions.php: call to assignment_base->submissions()

      Replication steps:

      1. Click on an Assignment activity with submitted assignments.
      2. Click on the "View n submitted assignments" link. A summary table appears. Note that at this point, everything is fine. The URL is along the lines of: http://moodle.domain.com/mod/assignment/submissions.php?id=29526
      3. Click on the 'Status' column header, to sort the table by status. At this point, the above error is received. (This page uses the URL: http://moodle.comain.com/mod/assignment/submissions.php?id=29526&currentgroup=&tsort=status)
      4. Clicking the browser's back button to get to the previously working page (with the URL as in 2, above) also now results in this error and stack trace.
      5. Logging out and in again (clearing the user session?) appears to slightly fix the issue: at least the page in 2, above, can be reached again.

        Issue Links

          Activity

          Hide
          Michael de Raadt added a comment -

          Thanks for reporting this.

          I've put it on our backlog and we'll try to get to it as soon as we can.

          In the meantime feel free to help us work on this issue.

          Show
          Michael de Raadt added a comment - Thanks for reporting this. I've put it on our backlog and we'll try to get to it as soon as we can. In the meantime feel free to help us work on this issue.
          Hide
          Garrett Novotny added a comment -

          Until this is resolved you can add this line to /mod/assignment/lib.php...

          $table->no_sorting('status');

          after the other $table->no_sorting lines

          -Garrett

          Show
          Garrett Novotny added a comment - Until this is resolved you can add this line to /mod/assignment/lib.php... $table->no_sorting('status'); after the other $table->no_sorting lines -Garrett
          Hide
          Paul Vaughan added a comment -

          Thanks, Garrett. Not sorting this column is a pain, but it's the lesser of two evils!

          Thanks,

          Paul.

          Show
          Paul Vaughan added a comment - Thanks, Garrett. Not sorting this column is a pain, but it's the lesser of two evils! Thanks, Paul.
          Hide
          Stephen Bourget added a comment -

          I just had a teacher report the same issue. This fix should address the issue (It should work for Moodle 2.0, 2.1 and 2.2Dev)

          Show
          Stephen Bourget added a comment - I just had a teacher report the same issue. This fix should address the issue (It should work for Moodle 2.0, 2.1 and 2.2Dev)
          Hide
          Jason Fowler added a comment -

          Hi Stephen, the code looks fine, just wondering of you are able to port the patch to the 2.0 and 2.2dev branches too? If not, I can do it for you, but it would make things simpler if they were all from the one source.

          Show
          Jason Fowler added a comment - Hi Stephen, the code looks fine, just wondering of you are able to port the patch to the 2.0 and 2.2dev branches too? If not, I can do it for you, but it would make things simpler if they were all from the one source.
          Hide
          Stephen Bourget added a comment -

          I've added the patches for Moodle 2.0 and Master

          Show
          Stephen Bourget added a comment - I've added the patches for Moodle 2.0 and Master
          Hide
          Eloy Lafuente (stronk7) added a comment - - edited

          Hi,

          I was reviewing this and I have this 2 comments:

          1) Why do we need to do all those SIGN operations? Sure I'm missing something but isn't something like this really easier and shows the logic better:

          $select = "SELECT $ufields,
                     s.id AS submissionid, s.grade, s.submissioncomment,
                     s.timemodified, s.timemarked,
                     CASE WHEN s.timemarked > 0 AND s.timemarked >= s.timemodified THEN 1
                          ELSE 0 END AS status ";
          

          2) No matter of which query finally lands, if we go back to calculate the status in the query, we need to delete the PHP code the right now is calculating the status:

          $auser->status = ($auser->timemarked > 0) && ($auser->timemarked >= $auser->timemodified);
          

          BTW, if you see, the logic in PHP is exactly the same I'm proposing for the SQL.

          Reopening this... I'll revisit it later to see if some comment has arrived and we can go ahead this week.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - - edited Hi, I was reviewing this and I have this 2 comments: 1) Why do we need to do all those SIGN operations? Sure I'm missing something but isn't something like this really easier and shows the logic better: $select = "SELECT $ufields, s.id AS submissionid, s.grade, s.submissioncomment, s.timemodified, s.timemarked, CASE WHEN s.timemarked > 0 AND s.timemarked >= s.timemodified THEN 1 ELSE 0 END AS status "; 2) No matter of which query finally lands, if we go back to calculate the status in the query, we need to delete the PHP code the right now is calculating the status: $auser->status = ($auser->timemarked > 0) && ($auser->timemarked >= $auser->timemodified); BTW, if you see, the logic in PHP is exactly the same I'm proposing for the SQL. Reopening this... I'll revisit it later to see if some comment has arrived and we can go ahead this week. Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Just to ensure my theory a bit more, I've executed the query above (with some u.* fields out, for clarity) against 4 servers, each one running one DB, with the same test course and I'm getting correct results in all the 4 DBs (mysql, postgres, mssql and oracle).

          The query:

              SELECT u.id, s.id AS submissionid, s.grade, s.timemodified, s.timemarked,
                     CASE WHEN s.timemarked > 0 AND s.timemarked >= s.timemodified THEN 1
                     ELSE 0 END AS status
                FROM mdl_user u
           LEFT JOIN mdl_assignment_submissions s ON u.id = s.userid AND s.assignment > 0
               WHERE u.id > 0  ORDER BY status DESC, timemarked DESC
          

          And the results:

           id | submissionid | grade | timemodified | timemarked | status 
          ----+--------------+-------+--------------+------------+--------
            4 |            4 |     3 |   1281032346 | 1281032412 |      1
            3 |            3 |    70 |   1280859008 | 1280859008 |      1
            4 |            2 |    90 |   1280859008 | 1280859008 |      1
            1 |              |       |              |            |      0
            5 |              |       |              |            |      0
            2 |              |       |              |            |      0
            4 |            1 |     4 |   1298229269 | 1281038089 |      0
          
          Show
          Eloy Lafuente (stronk7) added a comment - Just to ensure my theory a bit more, I've executed the query above (with some u.* fields out, for clarity) against 4 servers, each one running one DB, with the same test course and I'm getting correct results in all the 4 DBs (mysql, postgres, mssql and oracle). The query: SELECT u.id, s.id AS submissionid, s.grade, s.timemodified, s.timemarked, CASE WHEN s.timemarked > 0 AND s.timemarked >= s.timemodified THEN 1 ELSE 0 END AS status FROM mdl_user u LEFT JOIN mdl_assignment_submissions s ON u.id = s.userid AND s.assignment > 0 WHERE u.id > 0 ORDER BY status DESC, timemarked DESC And the results: id | submissionid | grade | timemodified | timemarked | status ----+--------------+-------+--------------+------------+-------- 4 | 4 | 3 | 1281032346 | 1281032412 | 1 3 | 3 | 70 | 1280859008 | 1280859008 | 1 4 | 2 | 90 | 1280859008 | 1280859008 | 1 1 | | | | | 0 5 | | | | | 0 2 | | | | | 0 4 | 1 | 4 | 1298229269 | 1281038089 | 0
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Here it is my proposal (for master):

          https://github.com/stronk7/moodle/compare/master...MDL-29529

          (should be easily back-portable) Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Here it is my proposal (for master): https://github.com/stronk7/moodle/compare/master...MDL-29529 (should be easily back-portable) Ciao
          Hide
          Stephen Bourget added a comment -

          Eloy,

          Thanks for looking at the issue.
          Your changes look like they should work. I'll grab your changes from Github in a few minutes and test it with my local site.

          Thanks,

          -Steve

          Show
          Stephen Bourget added a comment - Eloy, Thanks for looking at the issue. Your changes look like they should work. I'll grab your changes from Github in a few minutes and test it with my local site. Thanks, -Steve
          Hide
          Stephen Bourget added a comment -

          I tested you proposed solution on MySQL for master and it works great. How do we proceed from here?

          -Steve

          Show
          Stephen Bourget added a comment - I tested you proposed solution on MySQL for master and it works great. How do we proceed from here? -Steve
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Surely it's better if you use that commit to provide the the 2.1, 2.0 (cherrypick?) and 1.9 fixes (this last won't apply clear, for sure) and I'll be happy to integrate them for testing.

          Show
          Eloy Lafuente (stronk7) added a comment - Surely it's better if you use that commit to provide the the 2.1, 2.0 (cherrypick?) and 1.9 fixes (this last won't apply clear, for sure) and I'll be happy to integrate them for testing.
          Hide
          Stephen Bourget added a comment -

          Eloy,

          I updated the branches for Moodle 2.1 and Moodle 1.9
          Please cherry pick the Moodle 2.1 fix to Moodle 2.0 and master

          The commit is 4aef860aec0c86b405fac54b551ffc257b08118c

          Thanks,

          -Steve

          Show
          Stephen Bourget added a comment - Eloy, I updated the branches for Moodle 2.1 and Moodle 1.9 Please cherry pick the Moodle 2.1 fix to Moodle 2.0 and master The commit is 4aef860aec0c86b405fac54b551ffc257b08118c Thanks, -Steve
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Yay, thanks Stephen... doing...

          Show
          Eloy Lafuente (stronk7) added a comment - Yay, thanks Stephen... doing...
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated, thanks!
          Hide
          Adrian Greeve added a comment -

          Tested with version 2.1 and 1.9. No error messages being displayed and columns are now sortable by status.

          Show
          Adrian Greeve added a comment - Tested with version 2.1 and 1.9. No error messages being displayed and columns are now sortable by status.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Yes, you got this finally upstream, just in time for Moodle 2.2beta. Congrats and thanks!

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Yes, you got this finally upstream, just in time for Moodle 2.2beta. Congrats and thanks! Ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: