MDL-32616 question engine: set_max_mark_in_attempts MySQL perf
authorTim Hunt <T.J.Hunt@open.ac.uk>
Thu, 25 Sep 2014 11:38:16 +0000 (12:38 +0100)
committerTim Hunt <T.J.Hunt@open.ac.uk>
Mon, 20 Oct 2014 11:06:39 +0000 (12:06 +0100)
The MySQL (& Maria DB) query 'optimizer' was failing to handle this
query, so for that DB only, we give it an equivalent query that it can
get right.

With unit test.

question/engine/datalib.php
question/engine/tests/datalib_test.php [new file with mode: 0644]

index 61910e8..7e36703 100644 (file)
@@ -1076,6 +1076,20 @@ ORDER BY
      * @param number $newmaxmark the new max mark to set.
      */
     public function set_max_mark_in_attempts(qubaid_condition $qubaids, $slot, $newmaxmark) {
+        if ($this->db->get_dbfamily() == 'mysql') {
+            // MySQL's query optimiser completely fails to cope with the
+            // set_field_select call below, so we have to give it a clue. See MDL-32616.
+            // TODO MDL-29589 encapsulate this MySQL-specific code with a $DB method.
+            $this->db->execute("
+                    UPDATE " . $qubaids->from_question_attempts('qa') . "
+                       SET qa.maxmark = :newmaxmark
+                     WHERE " . $qubaids->where() . "
+                       AND slot = :slot
+                    ", $qubaids->from_where_params() + array('newmaxmark' => $newmaxmark, 'slot' => $slot));
+            return;
+        }
+
+        // Normal databases.
         $this->db->set_field_select('question_attempts', 'maxmark', $newmaxmark,
                 "questionusageid {$qubaids->usage_id_in()} AND slot = :slot",
                 $qubaids->usage_id_in_params() + array('slot' => $slot));
diff --git a/question/engine/tests/datalib_test.php b/question/engine/tests/datalib_test.php
new file mode 100644 (file)
index 0000000..9279626
--- /dev/null
@@ -0,0 +1,129 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * Unit tests for parts of {@link question_engine_data_mapper}.
+ *
+ * @package   core_question
+ * @category  test
+ * @copyright 2014 The Open University
+ * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+
+defined('MOODLE_INTERNAL') || die();
+
+global $CFG;
+require_once(dirname(__FILE__) . '/../lib.php');
+require_once(dirname(__FILE__) . '/helpers.php');
+
+
+/**
+ * Unit tests for parts of {@link question_engine_data_mapper}.
+ *
+ * Note that many of the methods used when attempting questions, like
+ * load_questions_usage_by_activity, insert_question_*, delete_steps are
+ * tested elsewhere, e.g. by {@link question_usage_autosave_test}. We do not
+ * re-test them here.
+ *
+ * @copyright 2014 The Open University
+ * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class question_engine_data_mapper_testcase extends qbehaviour_walkthrough_test_base {
+
+    /**
+     * We create two usages, each with two questions, a short-answer marked
+     * out of 5, and and essay marked out of 10. We just start these attempts.
+     *
+     * Then we change the max mark for the short-answer question in one of the
+     * usages to 20, using a qubaid_list, and verify.
+     *
+     * Then we change the max mark for the essay question in the other
+     * usage to 2, using a qubaid_join, and verify.
+     */
+    public function test_set_max_mark_in_attempts() {
+
+        // Set up some things the tests will need.
+        $this->resetAfterTest();
+        $dm = new question_engine_data_mapper();
+
+        // Create the questions.
+        $generator = $this->getDataGenerator()->get_plugin_generator('core_question');
+        $cat = $generator->create_question_category();
+        $sa = $generator->create_question('shortanswer', null,
+                array('category' => $cat->id));
+        $essay = $generator->create_question('essay', null,
+                array('category' => $cat->id));
+
+        // Create the first usage.
+        $q = question_bank::load_question($sa->id);
+        $this->start_attempt_at_question($q, 'interactive', 5);
+
+        $q = question_bank::load_question($essay->id);
+        $this->start_attempt_at_question($q, 'interactive', 10);
+
+        $this->finish();
+        $this->save_quba();
+        $usage1id = $this->quba->get_id();
+
+        // Create the second usage.
+        $this->quba = question_engine::make_questions_usage_by_activity('unit_test',
+                context_system::instance());
+
+        $q = question_bank::load_question($sa->id);
+        $this->start_attempt_at_question($q, 'interactive', 5);
+        $this->process_submission(array('answer' => 'fish'));
+
+        $q = question_bank::load_question($essay->id);
+        $this->start_attempt_at_question($q, 'interactive', 10);
+
+        $this->finish();
+        $this->save_quba();
+        $usage2id = $this->quba->get_id();
+
+        // Test set_max_mark_in_attempts with a qubaid_list.
+        $usagestoupdate = new qubaid_list(array($usage1id));
+        $dm->set_max_mark_in_attempts($usagestoupdate, 1, 20.0);
+        $quba1 = question_engine::load_questions_usage_by_activity($usage1id);
+        $quba2 = question_engine::load_questions_usage_by_activity($usage2id);
+        $this->assertEquals(20, $quba1->get_question_max_mark(1));
+        $this->assertEquals(10, $quba1->get_question_max_mark(2));
+        $this->assertEquals( 5, $quba2->get_question_max_mark(1));
+        $this->assertEquals(10, $quba2->get_question_max_mark(2));
+
+        // Test set_max_mark_in_attempts with a qubaid_join.
+        $usagestoupdate = new qubaid_join('{question_usages} qu', 'qu.id',
+                'qu.id = :usageid', array('usageid' => $usage2id));
+        $dm->set_max_mark_in_attempts($usagestoupdate, 2, 2.0);
+        $quba1 = question_engine::load_questions_usage_by_activity($usage1id);
+        $quba2 = question_engine::load_questions_usage_by_activity($usage2id);
+        $this->assertEquals(20, $quba1->get_question_max_mark(1));
+        $this->assertEquals(10, $quba1->get_question_max_mark(2));
+        $this->assertEquals( 5, $quba2->get_question_max_mark(1));
+        $this->assertEquals( 2, $quba2->get_question_max_mark(2));
+
+        // Test the nothing to do case.
+        $usagestoupdate = new qubaid_join('{question_usages} qu', 'qu.id',
+                'qu.id = :usageid', array('usageid' => -1));
+        $dm->set_max_mark_in_attempts($usagestoupdate, 2, 2.0);
+        $quba1 = question_engine::load_questions_usage_by_activity($usage1id);
+        $quba2 = question_engine::load_questions_usage_by_activity($usage2id);
+        $this->assertEquals(20, $quba1->get_question_max_mark(1));
+        $this->assertEquals(10, $quba1->get_question_max_mark(2));
+        $this->assertEquals( 5, $quba2->get_question_max_mark(1));
+        $this->assertEquals( 2, $quba2->get_question_max_mark(2));
+    }
+}