diff --git a/backup/controller/tests/controller_test.php b/backup/controller/tests/controller_test.php index d5142f15376..c34ff288cac 100644 --- a/backup/controller/tests/controller_test.php +++ b/backup/controller/tests/controller_test.php @@ -149,6 +149,32 @@ class core_backup_controller_testcase extends advanced_testcase { } $this->assertTrue($alltrue); } + + /** + * Test restore of deadlock causing backup. + */ + public function test_restore_of_deadlock_causing_backup() { + global $USER, $CFG; + $this->preventResetByRollback(); + + $foldername = 'deadlock'; + $fp = get_file_packer('application/vnd.moodle.backup'); + $tempdir = $CFG->dataroot . '/temp/backup/' . $foldername; + $files = $fp->extract_to_pathname($CFG->dirroot . '/backup/controller/tests/fixtures/deadlock.mbz', $tempdir); + + $this->setAdminUser(); + $controller = new restore_controller( + 'deadlock', + $this->courseid, + backup::INTERACTIVE_NO, + backup::MODE_GENERAL, + $USER->id, + backup::TARGET_NEW_COURSE + ); + $this->assertTrue($controller->execute_precheck()); + $controller->execute_plan(); + $controller->destroy(); + } } diff --git a/backup/controller/tests/fixtures/deadlock.mbz b/backup/controller/tests/fixtures/deadlock.mbz new file mode 100644 index 00000000000..f45a956a161 Binary files /dev/null and b/backup/controller/tests/fixtures/deadlock.mbz differ diff --git a/lib/dml/sqlsrv_native_moodle_database.php b/lib/dml/sqlsrv_native_moodle_database.php index b80191c5161..08f27357748 100644 --- a/lib/dml/sqlsrv_native_moodle_database.php +++ b/lib/dml/sqlsrv_native_moodle_database.php @@ -50,6 +50,31 @@ class sqlsrv_native_moodle_database extends moodle_database { /** @var array list of open recordsets */ protected $recordsets = array(); + /** @var array list of reserve words in MSSQL / Transact from http://msdn2.microsoft.com/en-us/library/ms189822.aspx */ + protected $reservewords = [ + "add", "all", "alter", "and", "any", "as", "asc", "authorization", "avg", "backup", "begin", "between", "break", + "browse", "bulk", "by", "cascade", "case", "check", "checkpoint", "close", "clustered", "coalesce", "collate", "column", + "commit", "committed", "compute", "confirm", "constraint", "contains", "containstable", "continue", "controlrow", + "convert", "count", "create", "cross", "current", "current_date", "current_time", "current_timestamp", "current_user", + "cursor", "database", "dbcc", "deallocate", "declare", "default", "delete", "deny", "desc", "disk", "distinct", + "distributed", "double", "drop", "dummy", "dump", "else", "end", "errlvl", "errorexit", "escape", "except", "exec", + "execute", "exists", "exit", "external", "fetch", "file", "fillfactor", "floppy", "for", "foreign", "freetext", + "freetexttable", "from", "full", "function", "goto", "grant", "group", "having", "holdlock", "identity", + "identity_insert", "identitycol", "if", "in", "index", "inner", "insert", "intersect", "into", "is", "isolation", + "join", "key", "kill", "left", "level", "like", "lineno", "load", "max", "merge", "min", "mirrorexit", "national", + "nocheck", "nonclustered", "not", "null", "nullif", "of", "off", "offsets", "on", "once", "only", "open", + "opendatasource", "openquery", "openrowset", "openxml", "option", "or", "order", "outer", "over", "percent", "perm", + "permanent", "pipe", "pivot", "plan", "precision", "prepare", "primary", "print", "privileges", "proc", "procedure", + "processexit", "public", "raiserror", "read", "readtext", "reconfigure", "references", "repeatable", "replication", + "restore", "restrict", "return", "revert", "revoke", "right", "rollback", "rowcount", "rowguidcol", "rule", "save", + "schema", "securityaudit", "select", "semantickeyphrasetable", "semanticsimilaritydetailstable", + "semanticsimilaritytable", "serializable", "session_user", "set", "setuser", "shutdown", "some", "statistics", "sum", + "system_user", "table", "tablesample", "tape", "temp", "temporary", "textsize", "then", "to", "top", "tran", + "transaction", "trigger", "truncate", "try_convert", "tsequal", "uncommitted", "union", "unique", "unpivot", "update", + "updatetext", "use", "user", "values", "varying", "view", "waitfor", "when", "where", "while", "with", "within group", + "work", "writetext" + ]; + /** * Constructor - instantiates the database, specifying if it's external (connect to other systems) or no (Moodle DB) * note this has effect to decide if prefix checks must be performed or no @@ -864,6 +889,10 @@ class sqlsrv_native_moodle_database extends moodle_database { } } } + + // Add WITH (NOLOCK) to any temp tables. + $sql = $this->add_no_lock_to_temp_tables($sql); + $result = $this->do_query($sql, $params, SQL_QUERY_SELECT, false, $needscrollable); if ($needscrollable) { // Skip $limitfrom records. @@ -872,6 +901,34 @@ class sqlsrv_native_moodle_database extends moodle_database { return $this->create_recordset($result); } + /** + * Use NOLOCK on any temp tables. Since it's a temp table and uncommitted reads are low risk anyway. + * + * @param string $sql the SQL select query to execute. + * @return string The SQL, with WITH (NOLOCK) added to all temp tables + */ + protected function add_no_lock_to_temp_tables($sql) { + return preg_replace_callback('/(\{([a-z][a-z0-9_]*)\})(\s+(\w+))?/', function($matches) { + $table = $matches[1]; // With the braces, so we can put it back in the query. + $name = $matches[2]; // Without the braces, so we can check if it's a temptable. + $tail = isset($matches[3]) ? $matches[3] : ''; // Catch the next word afterwards so that we can check if it's an alias. + $replacement = $matches[0]; // The table and the word following it, so we can replace it back if no changes are needed. + + if ($this->temptables && $this->temptables->is_temptable($name)) { + if (!empty($tail)) { + if (in_array(strtolower(trim($tail)), $this->reservewords)) { + // If the table is followed by a reserve word, it's not an alias so put the WITH (NOLOCK) in between. + return $table . ' WITH (NOLOCK)' . $tail; + } + } + // If the table is not followed by a reserve word, put the WITH (NOLOCK) after the whole match. + return $replacement . ' WITH (NOLOCK)'; + } else { + return $replacement; + } + }, $sql); + } + /** * Create a record set and initialize with first row * diff --git a/lib/dml/tests/sqlsrv_native_moodle_database_test.php b/lib/dml/tests/sqlsrv_native_moodle_database_test.php new file mode 100644 index 00000000000..65cd4706e7a --- /dev/null +++ b/lib/dml/tests/sqlsrv_native_moodle_database_test.php @@ -0,0 +1,168 @@ +. + +/** + * Test sqlsrv dml support. + * + * @package core + * @category dml + * @copyright 2017 John Okely + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +defined('MOODLE_INTERNAL') || die(); + +global $CFG; +require_once($CFG->dirroot.'/lib/dml/sqlsrv_native_moodle_database.php'); + +/** + * Test case for sqlsrv dml support. + * + * @package core + * @category dml + * @copyright 2017 John Okely + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class sqlsrv_native_moodle_database_testcase extends advanced_testcase { + + public function setUp() { + parent::setUp(); + $this->resetAfterTest(); + } + + /** + * Dataprovider for test_add_no_lock_to_temp_tables + * @return array Data for test_add_no_lock_to_temp_tables + */ + public function add_no_lock_to_temp_tables_provider() { + return [ + "Basic temp table, nothing following" => [ + 'input' => 'SELECT * FROM {table_temp}', + 'expected' => 'SELECT * FROM {table_temp} WITH (NOLOCK)' + ], + "Basic temp table, with capitalised alias" => [ + 'input' => 'SELECT * FROM {table_temp} MYTABLE', + 'expected' => 'SELECT * FROM {table_temp} MYTABLE WITH (NOLOCK)' + ], + "Temp table with alias, and another non-temp table" => [ + 'input' => 'SELECT * FROM {table_temp} x WHERE y in (SELECT y from {table2})', + 'expected' => 'SELECT * FROM {table_temp} x WITH (NOLOCK) WHERE y in (SELECT y from {table2})' + ], + "Temp table with reserve word following, no alias" => [ + 'input' => 'SELECT DISTINCT * FROM {table_temp} WHERE y in (SELECT y from {table2} nottemp)', + 'expected' => 'SELECT DISTINCT * FROM {table_temp} WITH (NOLOCK) WHERE y in (SELECT y from {table2} nottemp)' + ], + "Temp table with reserve word, lower case" => [ + 'input' => 'SELECT DISTINCT * FROM {table_temp} where y in (SELECT y from {table2} nottemp)', + 'expected' => 'SELECT DISTINCT * FROM {table_temp} WITH (NOLOCK) where y in (SELECT y from {table2} nottemp)' + ], + "Another reserve word test" => [ + 'input' => 'SELECT DISTINCT * FROM {table_temp} PIVOT y in (SELECT y from {table2} nottemp)', + 'expected' => 'SELECT DISTINCT * FROM {table_temp} WITH (NOLOCK) PIVOT y in (SELECT y from {table2} nottemp)' + ], + "Another reserve word test should fail" => [ + 'input' => 'SELECT DISTINCT * FROM {table_temp} PIVOT y in (SELECT y from {table2} nottemp)', + 'expected' => 'SELECT DISTINCT * FROM {table_temp} WITH (NOLOCK) PIVOT y in (SELECT y from {table2} nottemp)' + ], + "Temp table with an alias starting with a keyword" => [ + 'input' => 'SELECT * FROM {table_temp} asx', + 'expected' => 'SELECT * FROM {table_temp} asx WITH (NOLOCK)' + ], + "Keep alias with underscore" => [ + 'input' => 'SELECT * FROM {table_temp} alias_for_table', + 'expected' => 'SELECT * FROM {table_temp} alias_for_table WITH (NOLOCK)' + ], + "Alias with number" => [ + 'input' => 'SELECT * FROM {table_temp} a5 WHERE y', + 'expected' => 'SELECT * FROM {table_temp} a5 WITH (NOLOCK) WHERE y' + ], + "Alias with number and underscore" => [ + 'input' => 'SELECT * FROM {table_temp} a_5 WHERE y', + 'expected' => 'SELECT * FROM {table_temp} a_5 WITH (NOLOCK) WHERE y' + ], + "Temp table in subquery" => [ + 'input' => 'select * FROM (SELECT DISTINCT * FROM {table_temp})', + 'expected' => 'select * FROM (SELECT DISTINCT * FROM {table_temp} WITH (NOLOCK))' + ], + "Temp table in subquery, with following commands" => [ + 'input' => 'select * FROM (SELECT DISTINCT * FROM {table_temp} ) WHERE y', + 'expected' => 'select * FROM (SELECT DISTINCT * FROM {table_temp} WITH (NOLOCK) ) WHERE y' + ], + "Temp table in subquery, with alias" => [ + 'input' => 'select * FROM (SELECT DISTINCT * FROM {table_temp} x) WHERE y', + 'expected' => 'select * FROM (SELECT DISTINCT * FROM {table_temp} x WITH (NOLOCK)) WHERE y' + ], + ]; + } + + /** + * Test add_no_lock_to_temp_tables + * + * @param string $input The input SQL query + * @param string $expected The expected resultant query + * @dataProvider add_no_lock_to_temp_tables_provider + */ + public function test_add_no_lock_to_temp_tables($input, $expected) { + $sqlsrv = new sqlsrv_native_moodle_database(); + + $reflector = new ReflectionObject($sqlsrv); + + $method = $reflector->getMethod('add_no_lock_to_temp_tables'); + $method->setAccessible(true); + + $temptablesproperty = $reflector->getProperty('temptables'); + $temptablesproperty->setAccessible(true); + $temptables = new temptables_tester(); + + $temptablesproperty->setValue($sqlsrv, $temptables); + + $result = $method->invoke($sqlsrv, $input); + + $temptablesproperty->setValue($sqlsrv, null); + $this->assertEquals($expected, $result); + } +} + +/** + * Test class for testing temptables + * + * @copyright 2017 John Okely + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class temptables_tester { + /** + * Returns if one table, based in the information present in the store, is a temp table + * + * For easy testing, anything with the word 'temp' in it is considered temporary. + * + * @param string $tablename name without prefix of the table we are asking about + * @return bool true if the table is a temp table (based in the store info), false if not + */ + public function is_temptable($tablename) { + if (strpos($tablename, 'temp') === false) { + return false; + } else { + return true; + } + } + /** + * Dispose the temptables + * + * @return void + */ + public function dispose() { + } +}