Moodle
  1. Moodle
  2. MDL-5670

get_courses_page function doesn't return the list of courses - is DISTINCT needed?

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: 1.6
    • Fix Version/s: 1.7
    • Component/s: Course
    • Labels:
      None
    • Environment:
      Windows XP
    • Database:
      MySQL
    • Affected Branches:
      MOODLE_16_STABLE
    • Fixed Branches:
      MOODLE_17_STABLE
    • Rank:
      33808

      Description

      When I view the list of courses per category on the /moodle/course/category.php page, it shows me no courses, even though I've added the courses to it before and i can see them on the front page.

      After looking at get_courses_page function, and running the generate SQL inside of mysql console I get the following error: #1037 - Out of memory; restart server and try again (needed 65528 bytes)

      The problem lies with the DISTICT keyword in the generated SQL statement (SELECT DISTINCT c.id,c.sortorder,c.shortname,c.fullname,c.summary,c.visible,c.teacher,c.guest,c.password FROM mdl_course c WHERE c.category = '4' ORDER BY c.sortorder ASC LIMIT 0,20), and after the removal of the statement, the SQL runs fine, with the correct number of courses displayed.

      MySQL version I am running is 5.0.18, php version 4.4.1 (Its the XAMP installation.

        Activity

        Hide
        Martin Dougiamas added a comment -

        From Petr Skoda (skodak at centrum.cz) Thursday, 1 June 2006, 03:31 AM:

        DISTINCT is needed because (courseid, userid) are not guaranteed to be unique in user_teachers table.

        I guess you should try to tweak your MySQL server configuration. How much RAM and courses do you have?

        From Taras (tdak at yorku.ca) Thursday, 1 June 2006, 03:51 AM:

        I have 1G of RAM and 2 courses, so I don't see the the ram being a problem. There should a better way to make sure that the course ID and user ID are unique, i.e by specifying UNIQUE keyword in table defintion for those two columns.

        From Petr Skoda (skodak at centrum.cz) Thursday, 1 June 2006, 04:54 AM:

        I was wrong there is unique key (course,userid). Hmm, it might be optimized I guess.

        Anyway the query seems OK, there is no reason why it should run out of memory with only two courses and it works for other large sites.

        I will ask our database gurus...

        From Petr Skoda (skodak at centrum.cz) Thursday, 1 June 2006, 05:23 AM:

        Assigning to Martín L, he might be interested in improving performace.

        Marking as feature request, because it is most probably a server bug. Me and Eloy think that the DISCTINCT should not be needed.

        From Eloy: perhaps some mysql variable, like sort_buffer_size could be the responsible.... although, by default, XAMPP leaves values higher......Uhm, yep! sort buffer size is 65528 by default!

        Maybe you can try to tweak this or other variables...

        From Taras (tdak at yorku.ca) Thursday, 1 June 2006, 09:23 PM:

        I've increased the sort_buffer_size limit to 1000000 and it seems to be working fine, however I am going to run some tests retreiving a hundred courses with that sort_buffer_size limit to make sure it works fine. Also I would suggest if you want to use DISTINCT keyword, set the sort buffer size using the following command, somewhere in the setup.php file in the lib dir. SET GLOBAL sort_buffer_size=1000000, SESSION sort_buffer_size=1000000;

        That might be of help and reduce the number of bug requests reguard courses not showing up. I'll let you know how the tests go.

        From Petr Skoda (skodak at centrum.cz) Thursday, 1 June 2006, 10:20 PM:

        Thanks for the feedback!!

        Martin is looking on the code. I guess DISCTINCT might be removed in 1.6.1 or 1.7dev, we agreed it would be risky to make this change just days before 1.6 release. Please post here any relevant information you find.

        thanks again

        From Taras (tdak at yorku.ca) Thursday, 1 June 2006, 11:09 PM:

        Tested with 50 courses in two categories created by 3 differnt users, with the sort_buffer_size set to 1 Mg. everything seems to be working fine.

        From Martin Langhoff (martin at catalyst.net.nz) Wednesday, 14 June 2006, 12:29 PM:

        Reviewing this now – sorry about the delay. According to pickaxe'ing datalib, I wrote the current implementation of that function...

        From Martin Langhoff (martin at catalyst.net.nz) Tuesday, 18 July 2006, 12:39 PM:

        On MOODLE_16_STABLE and HEAD:

        Close MDL-5670 - get_courses_page() - remove costly and redundant DISTINCT

        get_courses_page() uses get_records_sql() which means that it will actually break if the first field requested isn't c.id. This in turn means that the DISTINCT is not needed at all.

        So let's go fast again!

        Show
        Martin Dougiamas added a comment - From Petr Skoda (skodak at centrum.cz) Thursday, 1 June 2006, 03:31 AM: DISTINCT is needed because (courseid, userid) are not guaranteed to be unique in user_teachers table. I guess you should try to tweak your MySQL server configuration. How much RAM and courses do you have? From Taras (tdak at yorku.ca) Thursday, 1 June 2006, 03:51 AM: I have 1G of RAM and 2 courses, so I don't see the the ram being a problem. There should a better way to make sure that the course ID and user ID are unique, i.e by specifying UNIQUE keyword in table defintion for those two columns. From Petr Skoda (skodak at centrum.cz) Thursday, 1 June 2006, 04:54 AM: I was wrong there is unique key (course,userid). Hmm, it might be optimized I guess. Anyway the query seems OK, there is no reason why it should run out of memory with only two courses and it works for other large sites. I will ask our database gurus... From Petr Skoda (skodak at centrum.cz) Thursday, 1 June 2006, 05:23 AM: Assigning to Martín L, he might be interested in improving performace. Marking as feature request, because it is most probably a server bug. Me and Eloy think that the DISCTINCT should not be needed. From Eloy: perhaps some mysql variable, like sort_buffer_size could be the responsible.... although, by default, XAMPP leaves values higher......Uhm, yep! sort buffer size is 65528 by default! Maybe you can try to tweak this or other variables... From Taras (tdak at yorku.ca) Thursday, 1 June 2006, 09:23 PM: I've increased the sort_buffer_size limit to 1000000 and it seems to be working fine, however I am going to run some tests retreiving a hundred courses with that sort_buffer_size limit to make sure it works fine. Also I would suggest if you want to use DISTINCT keyword, set the sort buffer size using the following command, somewhere in the setup.php file in the lib dir. SET GLOBAL sort_buffer_size=1000000, SESSION sort_buffer_size=1000000; That might be of help and reduce the number of bug requests reguard courses not showing up. I'll let you know how the tests go. From Petr Skoda (skodak at centrum.cz) Thursday, 1 June 2006, 10:20 PM: Thanks for the feedback!! Martin is looking on the code. I guess DISCTINCT might be removed in 1.6.1 or 1.7dev, we agreed it would be risky to make this change just days before 1.6 release. Please post here any relevant information you find. thanks again From Taras (tdak at yorku.ca) Thursday, 1 June 2006, 11:09 PM: Tested with 50 courses in two categories created by 3 differnt users, with the sort_buffer_size set to 1 Mg. everything seems to be working fine. From Martin Langhoff (martin at catalyst.net.nz) Wednesday, 14 June 2006, 12:29 PM: Reviewing this now – sorry about the delay. According to pickaxe'ing datalib, I wrote the current implementation of that function... From Martin Langhoff (martin at catalyst.net.nz) Tuesday, 18 July 2006, 12:39 PM: On MOODLE_16_STABLE and HEAD: Close MDL-5670 - get_courses_page() - remove costly and redundant DISTINCT get_courses_page() uses get_records_sql() which means that it will actually break if the first field requested isn't c.id. This in turn means that the DISTINCT is not needed at all. So let's go fast again!
        Hide
        Luis Peralta added a comment -

        Hi Martin,

        The DISTINCT is needed or not every course will be displayed, as Petr says. Students will not be able to browse all courses, as there will be less than perpage courses per page.

        Show
        Luis Peralta added a comment - Hi Martin, The DISTINCT is needed or not every course will be displayed, as Petr says. Students will not be able to browse all courses, as there will be less than perpage courses per page.
        Hide
        Petr Škoda added a comment -

        1.7 has different implementation of get_course_page(), I guess it was fixed in 1.6.x too. Closing now...

        Show
        Petr Škoda added a comment - 1.7 has different implementation of get_course_page(), I guess it was fixed in 1.6.x too. Closing now...

          People

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

            Dates

            • Created:
              Updated:
              Resolved: