Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

moodle--eduforge--1.3.3--patch-222 - SQL improvements\n Better SQL pe…

…rformance for get_courses() and get_courses_page() calls, specially from the categories page. This gives much better performance when browsing existing courses
  • Loading branch information...
commit ea9d7e881545ba8d99e016fcf1c88652fe099478 1 parent ee99d4d
martinlanghoff authored
Showing with 56 additions and 70 deletions.
  1. +56 −70 lib/datalib.php
View
126 lib/datalib.php
@@ -1752,53 +1752,44 @@ function get_site () {
* Returns list of courses, for whole site, or category
*
* @param type description
+*
+* Important: Using c.* for fields is extremely expensive because
+* we are using distinct. You almost _NEVER_ need all the fields
+* in such a large SELECT
*/
function get_courses($categoryid="all", $sort="c.sortorder ASC", $fields="c.*") {
-/// This function deliberately uses PHP to do the checking at the end,
-/// because MySQL has been known to really bog down when trying to do a JOIN
-/// on the 'course' and 'user_teachers' table at the same time.
global $USER, $CFG;
-
- $select = array();
-
- if ($categoryid != "all") {
- $select[] = "c.category = '$categoryid'";
+
+ $categoryselect = "";
+ if ($categoryid != "all" && is_numeric($categoryid)) {
+ $categoryselect = "c.category = '$categoryid'";
}
- /// Admins can always see invisible courses
- /// Creators can always see invisible courses
- /// Teachers can only see their own invisible courses <- needs detailed checking
- /// Students can't see invisible courses at all
- /// Guests can't see invisible courses at all
-
+ $teachertable = "";
$visiblecourses = "";
- $showallinvisible = iscreator(); // includes admins
- $hideallinvisible = empty($USER->id) || (!isteacher());
-
- if ($hideallinvisible) {
- $select[] = "c.visible > 0";
+ $sqland = "";
+ if (!empty($categoryselect)) {
+ $sqland = "AND ";
+ }
+ if (!empty($USER->id)) { // May need to check they are a teacher
+ if (!iscreator()) {
+ $visiblecourses = "$sqland ((c.visible > 0) OR t.userid = '$USER->id')";
+ $teachertable = "LEFT JOIN {$CFG->prefix}user_teachers t ON t.course = c.id";
+ }
+ } else {
+ $visiblecourses = "$sqland c.visible > 0";
}
- if ($select) {
- $selectsql = "{$CFG->prefix}course c WHERE ".implode(' AND ', $select);
+ if ($categoryselect or $visiblecourses) {
+ $selectsql = "{$CFG->prefix}course c $teachertable WHERE $categoryselect $visiblecourses";
} else {
- $selectsql = "{$CFG->prefix}course c ";
+ $selectsql = "{$CFG->prefix}course c $teachertable";
}
$courses = get_records_sql("SELECT $fields FROM $selectsql ORDER BY $sort");
- if ($courses and (!$hideallinvisible) and (!$showallinvisible)) { // For ordinary users we need to check visibility
- foreach ($courses as $key => $course) {
- if ($course->visible == 0) { // Invisible course, let's check if we are a teacher
- if (!isteacher($course->id)) { // We shouldn't see this
- unset($courses[$key]);
- }
- }
- }
- }
-
- return $courses;
+ return get_records_sql("SELECT ".((!empty($teachertable)) ? " DISTINCT " : "")." $fields FROM $selectsql ".((!empty($sort)) ? "ORDER BY $sort" : ""));
}
@@ -1810,61 +1801,56 @@ function get_courses($categoryid="all", $sort="c.sortorder ASC", $fields="c.*")
* Similar to get_courses, but allows paging
*
* @param type description
+*
+* Important: Using c.* for fields is extremely expensive because
+* we are using distinct. You almost _NEVER_ need all the fields
+* in such a large SELECT
*/
function get_courses_page($categoryid="all", $sort="c.sortorder ASC", $fields="c.*",
&$totalcount, $limitfrom="", $limitnum="") {
-/// This function deliberately uses PHP to do the checking at the end,
-/// because MySQL has been known to really bog down when trying to do a JOIN
-/// on the 'course' and 'user_teachers' table at the same time.
-
global $USER, $CFG;
- $select = array();
-
+ $categoryselect = "";
if ($categoryid != "all") {
- $select[] = "c.category = '$categoryid'";
+ $categoryselect = "c.category = '$categoryid'";
}
- /// Admins can always see invisible courses
- /// Creators can always see invisible courses
- /// Teachers can only see their own invisible courses <- needs detailed checking
- /// Students can't see invisible courses at all
- /// Guests can't see invisible courses at all
-
+ $teachertable = "";
$visiblecourses = "";
- $showallinvisible = iscreator(); // includes admins
- $hideallinvisible = empty($USER->id) || (!isteacher());
-
- if ($hideallinvisible) {
- $select[] = "c.visible > 0";
- }
-
- if ($select) {
- $selectsql = "{$CFG->prefix}course c WHERE ".implode(' AND ', $select);
+ $sqland = "";
+ if (!empty($categoryselect)) {
+ $sqland = "AND ";
+ }
+ if (!empty($USER)) { // May need to check they are a teacher
+ if (!iscreator()) {
+ $visiblecourses = "$sqland ((c.visible > 0) OR t.userid = '$USER->id')";
+ $teachertable = "LEFT JOIN {$CFG->prefix}user_teachers t ON t.course=c.id";
+ }
} else {
- $selectsql = "{$CFG->prefix}course c ";
+ $visiblecourses = "$sqland c.visible > 0";
}
- $courses = get_records_sql("SELECT $fields FROM $selectsql ORDER BY $sort");
-
- if ($courses and (!$hideallinvisible) and (!$showallinvisible)) { // For ordinary users we need to check visibility
- foreach ($courses as $key => $course) {
- if ($course->visible == 0) { // Invisible course, let's check if we are a teacher
- if (!isteacher($course->id)) { // We shouldn't see this
- unset($courses[$key]);
- }
- }
+ if ($limitfrom !== "") {
+ switch ($CFG->dbtype) {
+ case "mysql":
+ $limit = "LIMIT $limitfrom,$limitnum";
+ break;
+ case "postgres7":
+ $limit = "LIMIT $limitnum OFFSET $limitfrom";
+ break;
+ default:
+ $limit = "LIMIT $limitnum,$limitfrom";
}
+ } else {
+ $limit = "";
}
- $totalcount = count($courses);
+ $selectsql = "{$CFG->prefix}course c $teachertable WHERE $categoryselect $visiblecourses";
- if ($courses and ($limitfrom or $limitnum)) {
- $courses = array_slice($courses, (int)$limitfrom, (int)$limitnum);
- }
+ $totalcount = count_records_sql("SELECT COUNT(DISTINCT c.id) FROM $selectsql");
- return $courses;
+ return get_records_sql("SELECT DISTINCT $fields FROM $selectsql ".((!empty($sort)) ? "ORDER BY $sort" : "")." $limit");
}
Please sign in to comment.
Something went wrong with that request. Please try again.