mirror of
https://github.com/moodle/moodle.git
synced 2025-03-17 14:10:08 +01:00
Merge branch 'm38_MDL-65811_Fix_Incorrect_ORDER_BY_Search' of https://github.com/scara/moodle
This commit is contained in:
commit
46f1284768
@ -841,6 +841,30 @@ class sqlsrv_native_moodle_database extends moodle_database {
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether the given SQL statement has the ORDER BY clause in the main query.
|
||||
*
|
||||
* @param string $sql the SQL statement
|
||||
* @return bool true if the main query has the ORDER BY clause; otherwise, false.
|
||||
*/
|
||||
protected static function has_query_order_by(string $sql) {
|
||||
$sqltoupper = strtoupper($sql);
|
||||
// Fail fast if there is no ORDER BY clause in the original query.
|
||||
if (strpos($sqltoupper, 'ORDER BY') === false) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Search for an ORDER BY clause in the main query, not in any subquery (not always allowed in MSSQL)
|
||||
// or in clauses like OVER with a window function e.g. ROW_NUMBER() OVER (ORDER BY ...) or RANK() OVER (ORDER BY ...):
|
||||
// use PHP PCRE recursive patterns to remove everything found within round brackets.
|
||||
$mainquery = preg_replace('/\(((?>[^()]+)|(?R))*\)/', '()', $sqltoupper);
|
||||
if (strpos($mainquery, 'ORDER BY') !== false) {
|
||||
return true;
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get a number of records as a moodle_recordset using a SQL statement.
|
||||
*
|
||||
@ -876,9 +900,9 @@ class sqlsrv_native_moodle_database extends moodle_database {
|
||||
} else {
|
||||
$needscrollable = false; // Using supported fetch/offset, no need to scroll anymore.
|
||||
$sql = (substr($sql, -1) === ';') ? substr($sql, 0, -1) : $sql;
|
||||
// We need order by to use FETCH/OFFSET.
|
||||
// We need ORDER BY to use FETCH/OFFSET.
|
||||
// Ordering by first column shouldn't break anything if there was no order in the first place.
|
||||
if (!strpos(strtoupper($sql), "ORDER BY")) {
|
||||
if (!self::has_query_order_by($sql)) {
|
||||
$sql .= " ORDER BY 1";
|
||||
}
|
||||
|
||||
|
@ -134,6 +134,124 @@ class sqlsrv_native_moodle_database_testcase extends advanced_testcase {
|
||||
$temptablesproperty->setValue($sqlsrv, null);
|
||||
$this->assertEquals($expected, $result);
|
||||
}
|
||||
|
||||
/**
|
||||
* Data provider for test_has_query_order_by
|
||||
*
|
||||
* @return array data for test_has_query_order_by
|
||||
*/
|
||||
public function has_query_order_by_provider() {
|
||||
// Fixtures taken from https://docs.moodle.org/en/ad-hoc_contributed_reports.
|
||||
|
||||
return [
|
||||
'User with language => FALSE' => [
|
||||
'sql' => <<<EOT
|
||||
SELECT username, lang
|
||||
FROM prefix_user
|
||||
EOT
|
||||
,
|
||||
'expectedmainquery' => <<<EOT
|
||||
SELECT username, lang
|
||||
FROM prefix_user
|
||||
EOT
|
||||
,
|
||||
'expectedresult' => false
|
||||
],
|
||||
'List Users with extra info (email) in current course => FALSE' => [
|
||||
'sql' => <<<EOT
|
||||
SELECT u.firstname, u.lastname, u.email
|
||||
FROM prefix_role_assignments AS ra
|
||||
JOIN prefix_context AS context ON context.id = ra.contextid AND context.contextlevel = 50
|
||||
JOIN prefix_course AS c ON c.id = context.instanceid AND c.id = %%COURSEID%%
|
||||
JOIN prefix_user AS u ON u.id = ra.userid
|
||||
EOT
|
||||
,
|
||||
'expectedmainquery' => <<<EOT
|
||||
SELECT u.firstname, u.lastname, u.email
|
||||
FROM prefix_role_assignments AS ra
|
||||
JOIN prefix_context AS context ON context.id = ra.contextid AND context.contextlevel = 50
|
||||
JOIN prefix_course AS c ON c.id = context.instanceid AND c.id = %%COURSEID%%
|
||||
JOIN prefix_user AS u ON u.id = ra.userid
|
||||
EOT
|
||||
,
|
||||
'expectedresult' => false
|
||||
],
|
||||
'ROW_NUMBER() OVER (ORDER BY ...) => FALSE (https://github.com/jleyva/moodle-block_configurablereports/issues/120)' => [
|
||||
'sql' => <<<EOT
|
||||
SELECT COUNT(*) AS 'Users who have logged in today'
|
||||
FROM (
|
||||
SELECT ROW_NUMBER() OVER(ORDER BY lastaccess DESC) AS Row
|
||||
FROM mdl_user
|
||||
WHERE lastaccess > DATEDIFF(s, '1970-01-01 02:00:00', (SELECT Convert(DateTime, DATEDIFF(DAY, 0, GETDATE()))))
|
||||
) AS Logins
|
||||
EOT
|
||||
,
|
||||
'expectedmainquery' => <<<EOT
|
||||
SELECT COUNT() AS 'Users who have logged in today'
|
||||
FROM () AS Logins
|
||||
EOT
|
||||
,
|
||||
'expectedresult' => false
|
||||
],
|
||||
'CONTRIB-7725 workaround) => TRUE' => [
|
||||
'sql' => <<<EOT
|
||||
SELECT COUNT(*) AS 'Users who have logged in today'
|
||||
FROM (
|
||||
SELECT ROW_NUMBER() OVER(ORDER BY lastaccess DESC) AS Row
|
||||
FROM mdl_user
|
||||
WHERE lastaccess > DATEDIFF(s, '1970-01-01 02:00:00', (SELECT Convert(DateTime, DATEDIFF(DAY, 0, GETDATE()))))
|
||||
) AS Logins ORDER BY 1
|
||||
EOT
|
||||
,
|
||||
'expectedmainquery' => <<<EOT
|
||||
SELECT COUNT() AS 'Users who have logged in today'
|
||||
FROM () AS Logins ORDER BY 1
|
||||
EOT
|
||||
,
|
||||
'expectedresult' => true
|
||||
],
|
||||
'Enrolment count in each Course => TRUE' => [
|
||||
'sql' => <<<EOT
|
||||
SELECT c.fullname, COUNT(ue.id) AS Enroled
|
||||
FROM prefix_course AS c
|
||||
JOIN prefix_enrol AS en ON en.courseid = c.id
|
||||
JOIN prefix_user_enrolments AS ue ON ue.enrolid = en.id
|
||||
GROUP BY c.id
|
||||
ORDER BY c.fullname
|
||||
EOT
|
||||
,
|
||||
'expectedmainquery' => <<<EOT
|
||||
SELECT c.fullname, COUNT() AS Enroled
|
||||
FROM prefix_course AS c
|
||||
JOIN prefix_enrol AS en ON en.courseid = c.id
|
||||
JOIN prefix_user_enrolments AS ue ON ue.enrolid = en.id
|
||||
GROUP BY c.id
|
||||
ORDER BY c.fullname
|
||||
EOT
|
||||
,
|
||||
'expectedresult' => true
|
||||
],
|
||||
];
|
||||
}
|
||||
|
||||
/**
|
||||
* Test has_query_order_by
|
||||
*
|
||||
* @dataProvider has_query_order_by_provider
|
||||
* @param string $sql the query
|
||||
* @param string $expectedmainquery the expected main query
|
||||
* @param bool $expectedresult the expected result
|
||||
*/
|
||||
public function test_has_query_order_by(string $sql, string $expectedmainquery, bool $expectedresult) {
|
||||
$mainquery = preg_replace('/\(((?>[^()]+)|(?R))*\)/', '()', $sql);
|
||||
$this->assertSame($expectedmainquery, $mainquery);
|
||||
|
||||
// The has_query_order_by static method is protected. Use Reflection to call the method.
|
||||
$method = new ReflectionMethod('sqlsrv_native_moodle_database', 'has_query_order_by');
|
||||
$method->setAccessible(true);
|
||||
$result = $method->invoke(null, $sql);
|
||||
$this->assertSame($expectedresult, $result);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
|
Loading…
x
Reference in New Issue
Block a user