From 83655bdd5af9d1c434069e0b52e1dad841d68356 Mon Sep 17 00:00:00 2001 From: Adam Olley Date: Mon, 15 Jul 2024 15:48:54 +0930 Subject: [PATCH] Improve performance of statistics queries Without this, the many sub-queries in stats calculations that JOIN the question_references table dont hit any of the indexes on that table. For sites with large question_references tables - this can be horrible for performance as it ends up doing a seqscan on that table. By adding the usingcontextid field, we instead get indexscan's. --- classes/statistics_calculator.php | 33 +++++++++++++++++++++++-------- reportlib.php | 8 ++++---- tests/report_test.php | 2 +- 3 files changed, 30 insertions(+), 13 deletions(-) diff --git a/classes/statistics_calculator.php b/classes/statistics_calculator.php index c7c27939..f4ef4f98 100644 --- a/classes/statistics_calculator.php +++ b/classes/statistics_calculator.php @@ -132,6 +132,7 @@ private static function get_attempt_stat_joins($cmid, $groupid, $excluderoles = JOIN {question_references} qr ON qr.itemid = sqq.id AND qr.component = 'mod_studentquiz' AND qr.questionarea = 'studentquiz_question' + AND qr.usingcontextid = :contextid1 JOIN {question_bank_entries} qbe ON qr.questionbankentryid = qbe.id JOIN {question_versions} qv ON qv.questionbankentryid = qr.questionbankentryid AND qv.version = ( SELECT MAX(version) @@ -163,6 +164,7 @@ private static function get_attempt_stat_joins($cmid, $groupid, $excluderoles = JOIN {question_references} qr ON qr.itemid = sqq.id AND qr.component = 'mod_studentquiz' AND qr.questionarea = 'studentquiz_question' + AND qr.usingcontextid = :contextid2 JOIN {question_bank_entries} qbe ON qr.questionbankentryid = qbe.id JOIN {question_versions} qv ON qv.questionbankentryid = qr.questionbankentryid AND qv.version = ( SELECT MAX(version) @@ -197,6 +199,7 @@ private static function get_attempt_stat_joins($cmid, $groupid, $excluderoles = JOIN {question_references} qr ON qr.itemid = sqq.id AND qr.component = 'mod_studentquiz' AND qr.questionarea = 'studentquiz_question' + AND qr.usingcontextid = :contextid3 JOIN {question_bank_entries} qbe ON qr.questionbankentryid = qbe.id JOIN {question_versions} qv ON qv.questionbankentryid = qr.questionbankentryid AND qv.version = ( SELECT MAX(version) @@ -231,6 +234,7 @@ private static function get_attempt_stat_joins($cmid, $groupid, $excluderoles = JOIN {question_references} qr ON qr.itemid = sqq.id AND qr.component = 'mod_studentquiz' AND qr.questionarea = 'studentquiz_question' + AND qr.usingcontextid = :contextid4 JOIN {question_bank_entries} qbe ON qr.questionbankentryid = qbe.id JOIN {question_versions} qv ON qv.questionbankentryid = qr.questionbankentryid AND qv.version = ( SELECT MAX(version) @@ -259,6 +263,7 @@ private static function get_attempt_stat_joins($cmid, $groupid, $excluderoles = JOIN {question_references} qr ON qr.itemid = sqq.id AND qr.component = 'mod_studentquiz' AND qr.questionarea = 'studentquiz_question' + AND qr.usingcontextid = :contextid5 JOIN {question_bank_entries} qbe ON qr.questionbankentryid = qbe.id JOIN {question_versions} qv ON qv.questionbankentryid = qr.questionbankentryid AND qv.version = ( SELECT MAX(version) @@ -302,7 +307,7 @@ private static function get_attempt_stat_joins($cmid, $groupid, $excluderoles = * @param null|int $userid * @return array */ - private static function get_attempt_stat_joins_params($cmid, $quantifiers = null, $userid = null): array { + private static function get_attempt_stat_joins_params($cmid, $contextid, $quantifiers = null, $userid = null): array { $params = [ 'cmid1' => $cmid, 'cmid2' => $cmid, @@ -311,6 +316,11 @@ private static function get_attempt_stat_joins_params($cmid, $quantifiers = null 'cmid5' => $cmid, 'cmid6' => $cmid, 'cmid7' => $cmid, + 'contextid1' => $contextid, + 'contextid2' => $contextid, + 'contextid3' => $contextid, + 'contextid4' => $contextid, + 'contextid5' => $contextid, 'status1' => question_version_status::QUESTION_STATUS_HIDDEN, 'status2' => question_version_status::QUESTION_STATUS_HIDDEN, 'status3' => question_version_status::QUESTION_STATUS_HIDDEN, @@ -333,10 +343,11 @@ private static function get_attempt_stat_joins_params($cmid, $quantifiers = null /** * Get aggregated studentquiz data * @param int $cmid Course module id of the StudentQuiz considered. + * @param int $cmid Context id of the StudentQuiz considered. * @param int $groupid Group id. * @return \moodle_recordset of paginated ranking table */ - public static function get_community_stats($cmid, $groupid) { + public static function get_community_stats($cmid, $contextid, $groupid) { global $DB; $select = 'SELECT ' .' count(*) participants,' @@ -359,7 +370,7 @@ public static function get_community_stats($cmid, $groupid) { .' COALESCE(sum(lastattempt.last_attempt_correct), 0) last_attempt_correct,' .' COALESCE(sum(lastattempt.last_attempt_incorrect), 0) last_attempt_incorrect'; $attemptstastjoins = self::get_attempt_stat_joins($cmid, $groupid); - $params = self::get_attempt_stat_joins_params($cmid); + $params = self::get_attempt_stat_joins_params($cmid, $contextid); $params += $attemptstastjoins->params; $rs = $DB->get_record_sql("$select {$attemptstastjoins->joins} {$attemptstastjoins->wheres}", $params); return $rs; @@ -369,19 +380,20 @@ public static function get_community_stats($cmid, $groupid) { * Get aggregated studentquiz data * * @param int $cmid Course module id of the StudentQuiz considered. + * @param int $cmid Context id of the StudentQuiz considered. * @param int $groupid Group id. * @param \stdClass $quantifiers ad-hoc class containing quantifiers for weighted points score. * @param int $userid User id. * @return array array of user ranking stats * TODO: use mod_studentquiz_report_record type */ - public static function get_user_stats($cmid, $groupid, $quantifiers, $userid) { + public static function get_user_stats($cmid, $contextid, $groupid, $quantifiers, $userid) { global $DB; $select = self::get_attempt_stat_select(); $attemptstastjoins = self::get_attempt_stat_joins($cmid, $groupid); $addwhere = ' AND u.id = :userid '; $statsbycat = ' ) statspercategory GROUP BY userid'; - $params = self::get_attempt_stat_joins_params($cmid, $quantifiers, $userid); + $params = self::get_attempt_stat_joins_params($cmid, $contextid, $quantifiers, $userid); $params += $attemptstastjoins->params; $rs = $DB->get_record_sql("$select {$attemptstastjoins->joins} {$attemptstastjoins->wheres} $addwhere $statsbycat ", $params); @@ -398,7 +410,7 @@ public static function get_user_stats($cmid, $groupid, $quantifiers, $userid) { * @param int $limitnum return a subset comprising this many records (optional, required if $limitfrom is set). * @return \moodle_recordset of paginated ranking table */ - public static function get_user_ranking_table($cmid, $groupid, $quantifiers, $excluderoles = [], + public static function get_user_ranking_table($cmid, $contextid, $groupid, $quantifiers, $excluderoles = [], $limitfrom = 0, $limitnum = 0) { global $DB; @@ -407,7 +419,7 @@ public static function get_user_ranking_table($cmid, $groupid, $quantifiers, $ex $statsbycat = ' ) statspercategory GROUP BY userid'; $order = ' ORDER BY points DESC, questions_created DESC, questions_approved DESC, rates_average DESC, ' .' question_attempts_correct DESC, question_attempts_incorrect ASC '; - $params = self::get_attempt_stat_joins_params($cmid, $quantifiers); + $params = self::get_attempt_stat_joins_params($cmid, $contextid, $quantifiers); $params += $attemptstastjoins->params; $res = $DB->get_recordset_sql("$select {$attemptstastjoins->joins} {$attemptstastjoins->wheres} $statsbycat $order", $params, $limitfrom, $limitnum); @@ -418,10 +430,11 @@ public static function get_user_ranking_table($cmid, $groupid, $quantifiers, $ex * This query collects aggregated information about the questions in this StudentQuiz. * * @param int $cmid Course module id. + * @param int $contextid Context id. * @param int $groupid Group id. * @return array array of question stats. */ - public static function get_question_stats($cmid, $groupid) { + public static function get_question_stats($cmid, $contextid, $groupid) { global $DB; $sql = "SELECT COUNT(*) AS questions_available, @@ -433,6 +446,7 @@ public static function get_question_stats($cmid, $groupid) { JOIN {question_references} qr ON qr.itemid = sqq.id AND qr.component = 'mod_studentquiz' AND qr.questionarea = 'studentquiz_question' + AND qr.usingcontextid = :contextid1 JOIN {question_bank_entries} qbe ON qr.questionbankentryid = qbe.id JOIN {question_versions} qv ON qv.questionbankentryid = qr.questionbankentryid AND qv.version = ( SELECT MAX(version) @@ -449,6 +463,7 @@ public static function get_question_stats($cmid, $groupid) { JOIN {question_bank_entries} qbe ON qr.questionbankentryid = qbe.id AND qr.component = 'mod_studentquiz' AND qr.questionarea = 'studentquiz_question' + AND qr.usingcontextid = :contextid2 JOIN {question_versions} qv ON qv.questionbankentryid = qr.questionbankentryid AND qv.version = ( SELECT MAX(version) FROM {question_versions} @@ -467,6 +482,8 @@ public static function get_question_stats($cmid, $groupid) { $params = [ 'cmid1' => $cmid, 'cmid2' => $cmid, + 'contextid1' => $contextid, + 'contextid2' => $contextid, 'status1' => question_version_status::QUESTION_STATUS_HIDDEN, 'status2' => question_version_status::QUESTION_STATUS_HIDDEN, ]; diff --git a/reportlib.php b/reportlib.php index b5d135b8..85c2d3b6 100755 --- a/reportlib.php +++ b/reportlib.php @@ -106,8 +106,8 @@ public function get_enrolled_users() { */ public function get_studentquiz_stats() { if (empty($this->studentquizstats)) { - $this->studentquizstats = statistics_calculator::get_community_stats($this->get_cm_id(), $this->groupid); - $this->questionstats = statistics_calculator::get_question_stats($this->get_cm_id(), $this->groupid); + $this->studentquizstats = statistics_calculator::get_community_stats($this->get_cm_id(), $this->context->id, $this->groupid); + $this->questionstats = statistics_calculator::get_question_stats($this->get_cm_id(), $this->context->id, $this->groupid); $this->studentquizstats->questions_available = $this->questionstats->questions_available; $this->studentquizstats->questions_average_rating = $this->questionstats->average_rating; $this->studentquizstats->questions_questions_approved = $this->questionstats->questions_approved; @@ -128,7 +128,7 @@ public function get_studentquiz_stats() { */ public function get_user_stats() { if (empty($this->userrankingstats)) { - $this->userrankingstats = statistics_calculator::get_user_stats($this->get_cm_id(), $this->groupid, + $this->userrankingstats = statistics_calculator::get_user_stats($this->get_cm_id(), $this->context->id, $this->groupid, $this->get_quantifiers(), $this->get_user_id()); return $this->userrankingstats; } else { @@ -399,7 +399,7 @@ public function get_studentquiz_id() { public function get_user_ranking_table($limitfrom = 0, $limitnum = 0) { $excluderoles = $this->get_roles_to_exclude(); - return statistics_calculator::get_user_ranking_table($this->get_cm_id(), $this->groupid, $this->get_quantifiers(), + return statistics_calculator::get_user_ranking_table($this->get_cm_id(), $this->context->id, $this->groupid, $this->get_quantifiers(), $excluderoles, $limitfrom, $limitnum); } diff --git a/tests/report_test.php b/tests/report_test.php index 0384940d..c7b6d569 100644 --- a/tests/report_test.php +++ b/tests/report_test.php @@ -214,7 +214,7 @@ public function test_mod_studentquiz_community_stats() { * @covers \mod_studentquiz\statistics_calculator::get_user_stats */ public function test_mod_studentquiz_user_stats() { - $userstats = statistics_calculator::get_user_stats($this->cm->id, 0, $this->report->get_quantifiers(), $this->users[0]->id); + $userstats = statistics_calculator::get_user_stats($this->cm->id, $this->context->id, 0, $this->report->get_quantifiers(), $this->users[0]->id); $this->assertEquals(0, $userstats->questions_created); }