MDL-35155 database: better sql_substr() impl. for mssql/sqlsrv + unit…

… tests

MSSQL's substring() implementation is somehow silly/strict and unable to
perform implicit casts to integer both for the start and length parameters.

This hits Moodle badly because of another problems (MDL-23997) we decided
to cast to string all bound placeholders long ago.

So this commit just enforces the cast of the start and length parameters to
integer. And includes unit tests for using placeholders on all positions in
the sql_substr() method.

lib/dml/mssql_native_moodle_database.php

@@ -1257,9 +1257,9 @@ public function sql_substr($expr, $start, $length=false) {
 s only returning name of SQL substring function, it now requires all parameters.');
         }
         if ($length === false) {
-            return "SUBSTRING($expr, $start, LEN($expr))";
+            return "SUBSTRING($expr, " . $this->sql_cast_char2int($start) . ", 2^31-1)";
         } else {
-            return "SUBSTRING($expr, $start, $length)";
+            return "SUBSTRING($expr, " . $this->sql_cast_char2int($start) . ", " . $this->sql_cast_char2int($length) . ")";
         }
     }
 

lib/dml/sqlsrv_native_moodle_database.php

@@ -1324,9 +1324,9 @@ public function sql_substr($expr, $start, $length = false) {
         }
 
         if ($length === false) {
-            return "SUBSTRING($expr, $start, LEN($expr))";
+            return "SUBSTRING($expr, " . $this->sql_cast_char2int($start) . ", 2^31-1)";
         } else {
-            return "SUBSTRING($expr, $start, $length)";
+            return "SUBSTRING($expr, " . $this->sql_cast_char2int($start) . ", " . $this->sql_cast_char2int($length) . ")";
         }
     }
 

lib/dml/tests/dml_test.php

@@ -4027,9 +4027,30 @@ public function test_sql_substring() {
             $this->assertInstanceOf('coding_exception', $e);
         }
 
-        $sql = "SELECT id, ".$DB->sql_substr("name", ":param1 + 1")." AS name FROM {{$tablename}}";
-        $record = $DB->get_record_sql($sql, array('param1' => 4));
-        $this->assertEquals(substr($string, 5-1), $record->name);
+        // Cover the function using placeholders in all positions.
+        $start = 4;
+        $length = 2;
+        // 1st param (target).
+        $sql = "SELECT id, ".$DB->sql_substr(":param1", $start)." AS name FROM {{$tablename}}";
+        $record = $DB->get_record_sql($sql, array('param1' => $string));
+        $this->assertEquals(substr($string, $start - 1), $record->name); // PHP's substr is 0-based.
+        // 2nd param (start).
+        $sql = "SELECT id, ".$DB->sql_substr("name", ":param1")." AS name FROM {{$tablename}}";
+        $record = $DB->get_record_sql($sql, array('param1' => $start));
+        $this->assertEquals(substr($string, $start - 1), $record->name); // PHP's substr is 0-based.
+        // 3rd param (length).
+        $sql = "SELECT id, ".$DB->sql_substr("name", $start, ":param1")." AS name FROM {{$tablename}}";
+        $record = $DB->get_record_sql($sql, array('param1' => $length));
+        $this->assertEquals(substr($string, $start - 1,  $length), $record->name); // PHP's substr is 0-based.
+        // All together.
+        $sql = "SELECT id, ".$DB->sql_substr(":param1", ":param2", ":param3")." AS name FROM {{$tablename}}";
+        $record = $DB->get_record_sql($sql, array('param1' => $string, 'param2' => $start, 'param3' => $length));
+        $this->assertEquals(substr($string, $start - 1,  $length), $record->name); // PHP's substr is 0-based.
+
+        // Try also with some expression passed.
+        $sql = "SELECT id, ".$DB->sql_substr("name", "(:param1 + 1) - 1")." AS name FROM {{$tablename}}";
+        $record = $DB->get_record_sql($sql, array('param1' => $start));
+        $this->assertEquals(substr($string, $start - 1), $record->name); // PHP's substr is 0-based.
     }
 
     public function test_sql_length() {