Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-27009

Regression: Statistics won't display after selecting a course due to bad sql

    Details

    • Database:
      PostgreSQL
    • Testing Instructions:
      Hide

      You need to test this on a Postgres database + at least one other DB.

      1. Log into your site as an admin
      2. If stats is not already enabled enable
        1. Search for and turn on enablestats
        2. Search for statsfirstrun and set to 1 month
        3. Search for statsruntimestarthour and set it to something close to the current time
        4. Visit admin/cron.php and make sure stats runs (should be obvious)
      3. In your browser head into a course which will have had recent activity
      4. In navigation under that course expand Reports and select Statistics
      5. On the next page click view.
      6. Make sure the page loads without any errors.
      Show
      You need to test this on a Postgres database + at least one other DB. Log into your site as an admin If stats is not already enabled enable Search for and turn on enablestats Search for statsfirstrun and set to 1 month Search for statsruntimestarthour and set it to something close to the current time Visit admin/cron.php and make sure stats runs (should be obvious) In your browser head into a course which will have had recent activity In navigation under that course expand Reports and select Statistics On the next page click view. Make sure the page loads without any errors.
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE
    • Pull from Repository:

      Description

      This may just be an issue with postgres, but when trying to view statistics for any course, after selecting the course from the drop down and clicking 'view', you get:

      [Tue Mar 29 12:06:04 2011] [error] [client 131.172.34.113] Default exception handler: Error reading from database Debug: ERROR: column "r.id" must appear in the GROUP BY clause or be used in an aggregate function at character 8\nSELECT r.id, r.name FROM mdl_role r JOIN mdl_stats_daily s ON s.roleid = r.id WHERE s.courseid = $1 GROUP BY s.roleid\n[array (\n 0 => '35',\n)]\n* line 391 of /lib/dml/moodle_database.php: dml_read_exception thrown\n* line 232 of /lib/dml/pgsql_native_moodle_database.php: call to moodle_database->query_end()\n* line 672 of /lib/dml/pgsql_native_moodle_database.php: call to pgsql_native_moodle_database->query_end()\n* line 1337 of /lib/statslib.php: call to pgsql_native_moodle_database->get_records_sql()\n* line 18 of /course/report/stats/report.php: call to stats_get_report_options()\n* line 65 of /course/report/stats/index.php: call to require()\n, referer: https://lmsdev.latrobe.edu.au/course/report/stats/index.php

      Basically, an sql query introduced in MDL-25822 uses 'group by' but isn't grouping/aggregating everything it's selecting.

      I've put a fix for this on github as follows:
      https://github.com/aolley/moodle/commit/0960f2f417b4e95fe9ae46bad103c114c57dc9cc

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            tsala Helen Foster added a comment -

            Adam, thanks for your report with patch. I just tried http://qa.moodle.net/ and didn't obtain an error message, so you may be right in saying that it's just an issue with postgres.

            Show
            tsala Helen Foster added a comment - Adam, thanks for your report with patch. I just tried http://qa.moodle.net/ and didn't obtain an error message, so you may be right in saying that it's just an issue with postgres.
            Hide
            tlevi Tony Levi added a comment -

            Please apply this patch asap, it resolves the issue and affects all 2.0.2+ moodle2 instances.

            Show
            tlevi Tony Levi added a comment - Please apply this patch asap, it resolves the issue and affects all 2.0.2+ moodle2 instances.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Thanks for the fix, I've submitted it for integration next week. In the mean time could you, plz:

            1) rebase your MDL-27009 branch with updated MOODLE_20_STABLE branch.
            2) Add some testing instructions, so tester will know, exactly, the steps to follow. I'd recommend you to suggest in the testing instructions about to perform the tests under all DBs.

            Many thanks, ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Thanks for the fix, I've submitted it for integration next week. In the mean time could you, plz: 1) rebase your MDL-27009 branch with updated MOODLE_20_STABLE branch. 2) Add some testing instructions, so tester will know, exactly, the steps to follow. I'd recommend you to suggest in the testing instructions about to perform the tests under all DBs. Many thanks, ciao
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            NOTE for integrators. The 20_STABLE fix must be applied to master.

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - NOTE for integrators. The 20_STABLE fix must be applied to master.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Thanks Adam, this has been integrated now.
            It was merged for MOODLE_20_STABLE and I've cherry-picked the commit to master.
            Cheers
            Sam

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks Adam, this has been integrated now. It was merged for MOODLE_20_STABLE and I've cherry-picked the commit to master. Cheers Sam
            Hide
            salvetore Michael de Raadt added a comment -

            Sorry, I don't have a PostgreSQL DB.

            Show
            salvetore Michael de Raadt added a comment - Sorry, I don't have a PostgreSQL DB.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            This was tested during integration and passed - if no one has a chance to test it then my +1 to pass.

            Show
            samhemelryk Sam Hemelryk added a comment - This was tested during integration and passed - if no one has a chance to test it then my +1 to pass.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            I've executed the query against the 4 DBs and is working ok (note I've not run the complete testing istructions, just the offending query).

            So based on that, an previous comments... passing this. Thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - I've executed the query against the 4 DBs and is working ok (note I've not run the complete testing istructions, just the offending query). So based on that, an previous comments... passing this. Thanks!
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            One less regression. Yay! Many thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - One less regression. Yay! Many thanks!

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  1/Aug/11