Merge branch 'MDL-43395-master' of git://github.com/danpoltawski/moodle
This commit is contained in:
@@ -924,6 +924,54 @@ abstract class moodle_database {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Ensures that limit params are numeric and positive integers, to be passed to the database.
|
||||
* We explicitly treat null, '' and -1 as 0 in order to provide compatibility with how limit
|
||||
* values have been passed historically.
|
||||
*
|
||||
* @param int $limitfrom Where to start results from
|
||||
* @param int $limitnum How many results to return
|
||||
* @return array Normalised limit params in array($limitfrom, $limitnum)
|
||||
*/
|
||||
protected function normalise_limit_from_num($limitfrom, $limitnum) {
|
||||
global $CFG;
|
||||
|
||||
// We explicilty treat these cases as 0.
|
||||
if ($limitfrom === null || $limitfrom === '' || $limitfrom === -1) {
|
||||
$limitfrom = 0;
|
||||
}
|
||||
if ($limitnum === null || $limitnum === '' || $limitnum === -1) {
|
||||
$limitnum = 0;
|
||||
}
|
||||
|
||||
if ($CFG->debugdeveloper) {
|
||||
if (!is_numeric($limitfrom)) {
|
||||
$strvalue = var_export($limitfrom, true);
|
||||
debugging("Non-numeric limitfrom parameter detected: $strvalue, did you pass the correct arguments?",
|
||||
DEBUG_DEVELOPER);
|
||||
} else if ($limitfrom < 0) {
|
||||
debugging("Negative limitfrom parameter detected: $limitfrom, did you pass the correct arguments?",
|
||||
DEBUG_DEVELOPER);
|
||||
}
|
||||
|
||||
if (!is_numeric($limitnum)) {
|
||||
$strvalue = var_export($limitnum, true);
|
||||
debugging("Non-numeric limitnum parameter detected: $strvalue, did you pass the correct arguments?",
|
||||
DEBUG_DEVELOPER);
|
||||
} else if ($limitnum < 0) {
|
||||
debugging("Negative limitnum parameter detected: $limitnum, did you pass the correct arguments?",
|
||||
DEBUG_DEVELOPER);
|
||||
}
|
||||
}
|
||||
|
||||
$limitfrom = (int)$limitfrom;
|
||||
$limitnum = (int)$limitnum;
|
||||
$limitfrom = max(0, $limitfrom);
|
||||
$limitnum = max(0, $limitnum);
|
||||
|
||||
return array($limitfrom, $limitnum);
|
||||
}
|
||||
|
||||
/**
|
||||
* Return tables in database WITHOUT current prefix.
|
||||
* @param bool $usecache if true, returns list of cached tables.
|
||||
|
||||
@@ -693,10 +693,9 @@ class mssql_native_moodle_database extends moodle_database {
|
||||
* @throws dml_exception A DML specific exception is thrown for any errors.
|
||||
*/
|
||||
public function get_recordset_sql($sql, array $params=null, $limitfrom=0, $limitnum=0) {
|
||||
$limitfrom = (int)$limitfrom;
|
||||
$limitnum = (int)$limitnum;
|
||||
$limitfrom = ($limitfrom < 0) ? 0 : $limitfrom;
|
||||
$limitnum = ($limitnum < 0) ? 0 : $limitnum;
|
||||
|
||||
list($limitfrom, $limitnum) = $this->normalise_limit_from_num($limitfrom, $limitnum);
|
||||
|
||||
if ($limitfrom or $limitnum) {
|
||||
if ($limitnum >= 1) { // Only apply TOP clause if we have any limitnum (limitfrom offset is handled later)
|
||||
$fetch = $limitfrom + $limitnum;
|
||||
|
||||
@@ -912,10 +912,8 @@ class mysqli_native_moodle_database extends moodle_database {
|
||||
* @throws dml_exception A DML specific exception is thrown for any errors.
|
||||
*/
|
||||
public function get_recordset_sql($sql, array $params=null, $limitfrom=0, $limitnum=0) {
|
||||
$limitfrom = (int)$limitfrom;
|
||||
$limitnum = (int)$limitnum;
|
||||
$limitfrom = ($limitfrom < 0) ? 0 : $limitfrom;
|
||||
$limitnum = ($limitnum < 0) ? 0 : $limitnum;
|
||||
|
||||
list($limitfrom, $limitnum) = $this->normalise_limit_from_num($limitfrom, $limitnum);
|
||||
|
||||
if ($limitfrom or $limitnum) {
|
||||
if ($limitnum < 1) {
|
||||
@@ -976,10 +974,8 @@ class mysqli_native_moodle_database extends moodle_database {
|
||||
* @throws dml_exception A DML specific exception is thrown for any errors.
|
||||
*/
|
||||
public function get_records_sql($sql, array $params=null, $limitfrom=0, $limitnum=0) {
|
||||
$limitfrom = (int)$limitfrom;
|
||||
$limitnum = (int)$limitnum;
|
||||
$limitfrom = ($limitfrom < 0) ? 0 : $limitfrom;
|
||||
$limitnum = ($limitnum < 0) ? 0 : $limitnum;
|
||||
|
||||
list($limitfrom, $limitnum) = $this->normalise_limit_from_num($limitfrom, $limitnum);
|
||||
|
||||
if ($limitfrom or $limitnum) {
|
||||
if ($limitnum < 1) {
|
||||
|
||||
@@ -712,11 +712,7 @@ class oci_native_moodle_database extends moodle_database {
|
||||
*/
|
||||
private function get_limit_sql($sql, array $params = null, $limitfrom=0, $limitnum=0) {
|
||||
|
||||
$limitfrom = (int)$limitfrom;
|
||||
$limitnum = (int)$limitnum;
|
||||
$limitfrom = ($limitfrom < 0) ? 0 : $limitfrom;
|
||||
$limitnum = ($limitnum < 0) ? 0 : $limitnum;
|
||||
|
||||
list($limitfrom, $limitnum) = $this->normalise_limit_from_num($limitfrom, $limitnum);
|
||||
// TODO: Add the /*+ FIRST_ROWS */ hint if there isn't another hint
|
||||
|
||||
if ($limitfrom and $limitnum) {
|
||||
|
||||
@@ -681,10 +681,9 @@ class pgsql_native_moodle_database extends moodle_database {
|
||||
* @throws dml_exception A DML specific exception is thrown for any errors.
|
||||
*/
|
||||
public function get_recordset_sql($sql, array $params=null, $limitfrom=0, $limitnum=0) {
|
||||
$limitfrom = (int)$limitfrom;
|
||||
$limitnum = (int)$limitnum;
|
||||
$limitfrom = ($limitfrom < 0) ? 0 : $limitfrom;
|
||||
$limitnum = ($limitnum < 0) ? 0 : $limitnum;
|
||||
|
||||
list($limitfrom, $limitnum) = $this->normalise_limit_from_num($limitfrom, $limitnum);
|
||||
|
||||
if ($limitfrom or $limitnum) {
|
||||
if ($limitnum < 1) {
|
||||
$limitnum = "ALL";
|
||||
@@ -724,10 +723,9 @@ class pgsql_native_moodle_database extends moodle_database {
|
||||
* @throws dml_exception A DML specific exception is thrown for any errors.
|
||||
*/
|
||||
public function get_records_sql($sql, array $params=null, $limitfrom=0, $limitnum=0) {
|
||||
$limitfrom = (int)$limitfrom;
|
||||
$limitnum = (int)$limitnum;
|
||||
$limitfrom = ($limitfrom < 0) ? 0 : $limitfrom;
|
||||
$limitnum = ($limitnum < 0) ? 0 : $limitnum;
|
||||
|
||||
list($limitfrom, $limitnum) = $this->normalise_limit_from_num($limitfrom, $limitnum);
|
||||
|
||||
if ($limitfrom or $limitnum) {
|
||||
if ($limitnum < 1) {
|
||||
$limitnum = "ALL";
|
||||
|
||||
@@ -762,10 +762,8 @@ class sqlsrv_native_moodle_database extends moodle_database {
|
||||
* @throws dml_exception A DML specific exception is thrown for any errors.
|
||||
*/
|
||||
public function get_recordset_sql($sql, array $params = null, $limitfrom = 0, $limitnum = 0) {
|
||||
$limitfrom = (int)$limitfrom;
|
||||
$limitnum = (int)$limitnum;
|
||||
$limitfrom = max(0, $limitfrom);
|
||||
$limitnum = max(0, $limitnum);
|
||||
|
||||
list($limitfrom, $limitnum) = $this->normalise_limit_from_num($limitfrom, $limitnum);
|
||||
|
||||
if ($limitfrom or $limitnum) {
|
||||
if ($limitnum >= 1) { // Only apply TOP clause if we have any limitnum (limitfrom offset is handled later)
|
||||
|
||||
@@ -4997,6 +4997,74 @@ class core_dml_testcase extends database_driver_testcase {
|
||||
$this->assertEquals(2, reset($records)->count);
|
||||
$this->assertEquals(2, end($records)->count);
|
||||
}
|
||||
|
||||
/**
|
||||
* Test debugging messages about invalid limit number values.
|
||||
*/
|
||||
public function test_invalid_limits_debugging() {
|
||||
$DB = $this->tdb;
|
||||
$dbman = $DB->get_manager();
|
||||
|
||||
// Setup test data.
|
||||
$table = $this->get_test_table();
|
||||
$tablename = $table->getName();
|
||||
$table->add_field('id', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, XMLDB_SEQUENCE, null);
|
||||
$table->add_field('course', XMLDB_TYPE_INTEGER, '10', null, XMLDB_NOTNULL, null, '0');
|
||||
$table->add_key('primary', XMLDB_KEY_PRIMARY, array('id'));
|
||||
$dbman->create_table($table);
|
||||
$DB->insert_record($tablename, array('course' => '1'));
|
||||
|
||||
// Verify that get_records_sql throws debug notices with invalid limit params.
|
||||
$DB->get_records_sql("SELECT * FROM {{$tablename}}", null, 'invalid');
|
||||
$this->assertDebuggingCalled("Non-numeric limitfrom parameter detected: 'invalid', did you pass the correct arguments?");
|
||||
|
||||
$DB->get_records_sql("SELECT * FROM {{$tablename}}", null, 1, 'invalid');
|
||||
$this->assertDebuggingCalled("Non-numeric limitnum parameter detected: 'invalid', did you pass the correct arguments?");
|
||||
|
||||
// Verify that get_recordset_sql throws debug notices with invalid limit params.
|
||||
$rs = $DB->get_recordset_sql("SELECT * FROM {{$tablename}}", null, 'invalid');
|
||||
$this->assertDebuggingCalled("Non-numeric limitfrom parameter detected: 'invalid', did you pass the correct arguments?");
|
||||
$rs->close();
|
||||
|
||||
$rs = $DB->get_recordset_sql("SELECT * FROM {{$tablename}}", null, 1, 'invalid');
|
||||
$this->assertDebuggingCalled("Non-numeric limitnum parameter detected: 'invalid', did you pass the correct arguments?");
|
||||
$rs->close();
|
||||
|
||||
// Verify that some edge cases do no create debugging messages.
|
||||
// String form of integer values.
|
||||
$DB->get_records_sql("SELECT * FROM {{$tablename}}", null, '1');
|
||||
$this->assertDebuggingNotCalled();
|
||||
$DB->get_records_sql("SELECT * FROM {{$tablename}}", null, 1, '2');
|
||||
$this->assertDebuggingNotCalled();
|
||||
// Empty strings.
|
||||
$DB->get_records_sql("SELECT * FROM {{$tablename}}", null, '');
|
||||
$this->assertDebuggingNotCalled();
|
||||
$DB->get_records_sql("SELECT * FROM {{$tablename}}", null, 1, '');
|
||||
$this->assertDebuggingNotCalled();
|
||||
// Null values.
|
||||
$DB->get_records_sql("SELECT * FROM {{$tablename}}", null, null);
|
||||
$this->assertDebuggingNotCalled();
|
||||
$DB->get_records_sql("SELECT * FROM {{$tablename}}", null, 1, null);
|
||||
$this->assertDebuggingNotCalled();
|
||||
|
||||
// Verify that empty arrays DO create debugging mesages.
|
||||
$DB->get_records_sql("SELECT * FROM {{$tablename}}", null, array());
|
||||
$this->assertDebuggingCalled("Non-numeric limitfrom parameter detected: array (\n), did you pass the correct arguments?");
|
||||
$DB->get_records_sql("SELECT * FROM {{$tablename}}", null, 1, array());
|
||||
$this->assertDebuggingCalled("Non-numeric limitnum parameter detected: array (\n), did you pass the correct arguments?");
|
||||
|
||||
// Verify Negative number handling:
|
||||
// -1 is explicitly treated as 0 for historical reasons.
|
||||
$DB->get_records_sql("SELECT * FROM {{$tablename}}", null, -1);
|
||||
$this->assertDebuggingNotCalled();
|
||||
$DB->get_records_sql("SELECT * FROM {{$tablename}}", null, 1, -1);
|
||||
$this->assertDebuggingNotCalled();
|
||||
// Any other negative values should throw debugging messages.
|
||||
$DB->get_records_sql("SELECT * FROM {{$tablename}}", null, -2);
|
||||
$this->assertDebuggingCalled("Negative limitfrom parameter detected: -2, did you pass the correct arguments?");
|
||||
$DB->get_records_sql("SELECT * FROM {{$tablename}}", null, 1, -2);
|
||||
$this->assertDebuggingCalled("Negative limitnum parameter detected: -2, did you pass the correct arguments?");
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
Reference in New Issue
Block a user