Skip to content

Commit

Permalink
Improve performance of statistics queries
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@aolley
aolley committed Jul 15, 2024
1 parent 3c2ae9b commit 83655bd
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 13 deletions.
33 changes: 25 additions & 8 deletions classes/statistics_calculator.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,'
Expand All @@ -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;
Expand All @@ -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);
Expand All @@ -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;

Expand All @@ -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);
Expand All @@ -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,
Expand All @@ -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)
Expand All @@ -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}
Expand All @@ -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,
];
Expand Down
8 changes: 4 additions & 4 deletions reportlib.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand Down Expand Up @@ -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);
}

Expand Down
2 changes: 1 addition & 1 deletion tests/report_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down

0 comments on commit 83655bd

Please sign in to comment.