mirror_php/moodle
1
0
Fork 0
mirror of https://github.com/moodle/moodle.git synced 2025-04-24 09:55:33 +02:00
Code Issues Projects Releases Wiki Activity

Merge branch 'MDL-78030-main-improvement' of https://github.com/meirzamoodle/moodle

Browse Source
This commit is contained in:
Shamim Rezaie 2024-07-31 17:38:29 +10:00
commit 5aee595206
11 changed files with 270 additions and 31 deletions

7
.upgradenotes/MDL-78030-2024071101575428.yml Normal file

@ -0,0 +1,7 @@
issueNumber: MDL-78030
notes:
core_user:
- message: >-
The participants_search::get_total_participants_count() is no longer
used since the total count can be obtained from ::get_participants()
type: deprecated

18
.upgradenotes/MDL-78030-2024071102213324.yml Normal file

@ -0,0 +1,18 @@
issueNumber: MDL-78030
notes:
core:
- message: |
Two new functions have been introduced in the \moodle_database class:
- `get_counted_records_sql()`
- `get_counted_recordset_sql()`
These methods are compatible with all databases.
They will check the current running database engine and apply the COUNT window function if it is supported,
otherwise, they will use the usual COUNT function.
The COUNT window function optimization is applied to the following databases:
- PostgreSQL
- MariaDB
- Oracle
MySQL and SQL Server do not use this optimization due to insignificant performance differences before and
after the improvement.
type: improved

14
admin/tool/customlang/locallib.php

@ -506,7 +506,6 @@ class tool_customlang_translator implements renderable {
list($insql, $inparams) = $DB->get_in_or_equal($filter->component, SQL_PARAMS_NAMED);
$csql = "SELECT COUNT(*)";
$fsql = "SELECT s.*, c.name AS component";
$sql = " FROM {tool_customlang_components} c
JOIN {tool_customlang} s ON s.componentid = c.id
@ -545,9 +544,16 @@ class tool_customlang_translator implements renderable {
$params['link'] = '%\_link';
}
$osql = " ORDER BY c.name, s.stringid";
$osql = "component, stringid";
$this->numofrows = $DB->count_records_sql($csql.$sql, $params);
$this->strings = $DB->get_records_sql($fsql.$sql.$osql, $params, ($this->currentpage) * self::PERPAGE, self::PERPAGE);
$this->strings = $DB->get_counted_records_sql(
sql: $fsql.$sql,
fullcountcolumn: 'fullcount',
sort: $osql,
params: $params,
limitfrom: ($this->currentpage) * self::PERPAGE,
limitnum: self::PERPAGE,
);
$this->numofrows = reset($this->strings)->fullcount ?? 0;
}
}

9
lib/dml/mariadb_native_moodle_database.php

@ -113,4 +113,13 @@ class mariadb_native_moodle_database extends mysqli_native_moodle_database {
}
return false;
}
/**
* MariaDB supports the COUNT() window function and provides a performance improvement.
*
* @return bool
*/
public function is_count_window_function_supported(): bool {
return true;
}
}

93
lib/dml/moodle_database.php

@ -2925,4 +2925,97 @@ abstract class moodle_database {
// No support unless specified.
return false;
}
/**
* Whether the database is able to support the COUNT() window function and provides a performance improvement.
*
* @return bool
*/
public function is_count_window_function_supported(): bool {
// No support unless specified.
return false;
}
/**
* Retrieve records with a select query and count the total number of records.
*
* @param string $sql The query string.
* @param string $fullcountcolumn The column name used for counting total records.
* @param string $sort (Optional) Sorting criteria for the records.
* The reason to separate $sort from $sql are:
* 1. The $sort needs to be placed outside the full count subquery
* in order to function properly in MariaDB.
* 2. For unsupported databases, it is not allowed to run a query to get the total with the $sort.
* Please refer to the {@see ::generate_fullcount_sql()} for details.
* @param array|null $params (Optional) Parameters to bind with the query.
* @param int $limitfrom (Optional) Offset for pagination.
* @param int $limitnum (Optional) Limit for pagination.
* @return array Fetched records.
*/
public function get_counted_records_sql(
string $sql,
string $fullcountcolumn,
string $sort = '',
?array $params = null,
int $limitfrom = 0,
int $limitnum = 0,
): array {
$fullcountsql = $this->generate_fullcount_sql($sql, $params, $fullcountcolumn);
if ($sort) {
$fullcountsql .= " ORDER BY " . $sort;
}
return $this->get_records_sql($fullcountsql, $params, $limitfrom, $limitnum);
}
/**
* Retrieve a recordset with a select query and count the total number of records.
*
* @param string $sql The query string.
* @param string $fullcountcolumn The column name used for counting total records.
* @param string $sort (Optional) Sorting criteria for the records.
* The reason to separate $sort from $sql are:
* 1. The $sort needs to be placed outside the full count subquery
* in order to function properly in MariaDB.
* 2. For unsupported databases, it is not allowed to run a query to get the total with the $sort.
* Please refer to the {@see ::generate_fullcount_sql()} for details.
* @param array|null $params (Optional) Parameters to bind with the query.
* @param int $limitfrom (Optional) Offset for pagination.
* @param int $limitnum (Optional) Limit for pagination.
* @return moodle_recordset A moodle_recordset instance..
*/
public function get_counted_recordset_sql(
string $sql,
string $fullcountcolumn,
string $sort = '',
?array $params = null,
int $limitfrom = 0,
int $limitnum = 0,
): moodle_recordset {
$fullcountsql = $this->generate_fullcount_sql($sql, $params, $fullcountcolumn);
if ($sort) {
$fullcountsql .= " ORDER BY " . $sort;
}
return $this->get_recordset_sql($fullcountsql, $params, $limitfrom, $limitnum);
}
/**
* Helper function to generate window COUNT() aggregate function to the SQL query.
*
* @param string $sql The SQL select query to execute.
* @param array|null $params array of sql parameters
* @param string $fullcountcolumn An alias column name for the window function results.
* @return string The generated query.
*/
private function generate_fullcount_sql(
string $sql,
?array $params,
string $fullcountcolumn,
): string {
$fullcountvalue = "COUNT(1) OVER()";
if (!$this->is_count_window_function_supported()) {
$sqlcount = "SELECT COUNT(1) FROM ($sql) results";
$fullcountvalue = $this->count_records_sql($sqlcount, $params);
}
return "SELECT results.*, $fullcountvalue AS $fullcountcolumn FROM ($sql) results";
}
}

9
lib/dml/oci_native_moodle_database.php

@ -1874,4 +1874,13 @@ class oci_native_moodle_database extends moodle_database {
$this->commit_status = OCI_COMMIT_ON_SUCCESS;
$this->query_end($result);
}
/**
* Oracle supports the COUNT() window function and provides a performance improvement.
*
* @return bool
*/
public function is_count_window_function_supported(): bool {
return true;
}
}

9
lib/dml/pgsql_native_moodle_database.php

@ -1722,4 +1722,13 @@ class pgsql_native_moodle_database extends moodle_database {
public function is_fulltext_search_supported() {
return true;
}
/**
* Postgresql supports the COUNT() window function and provides a performance improvement.
*
* @return bool
*/
public function is_count_window_function_supported(): bool {
return true;
}
}

56
lib/dml/tests/dml_test.php

@ -6432,6 +6432,62 @@ EOD;
"Found invalid DB server version format when reading version from DB: '{$version}' ({$description}).");
$db2->dispose();
}
/**
* Test the COUNT() window function with the actual DB Server.
*
* @covers \moodle_database::get_counted_recordset_sql()
* @covers \moodle_database::get_counted_records_sql()
* @covers \moodle_database::generate_fullcount_sql()
* @return void
*/
public function test_count_window_function(): void {
$DB = $this->tdb;
$dbman = $DB->get_manager();
$table = $this->get_test_table();
$tablename = $table->getName();
$table->add_field('id', XMLDB_TYPE_INTEGER, '1', null, XMLDB_NOTNULL, XMLDB_SEQUENCE, null);
$table->add_field('course', XMLDB_TYPE_INTEGER, '1', null, XMLDB_NOTNULL, null, '0');
$table->add_key('primary', XMLDB_KEY_PRIMARY, ['id']);
$dbman->create_table($table);
for ($i = 1; $i <= 5; $i++) {
$DB->insert_record($tablename, ['course' => $i], false);
}
// Test with the get_recordset_select().
$rs = $DB->get_counted_recordset_sql(
sql: "SELECT * FROM {{$tablename}}",
fullcountcolumn: 'fullcount',
sort: "course DESC",
limitfrom: 1,
limitnum: 3,
);
// Check whether the fullcount column returns the correct number.
$this->assertEquals(5, $rs->current()->fullcount);
// Check whether the `limitfrom` works properly.
$this->assertEquals(4, $rs->current()->course);
// Check whether the 'limitnum' works properly.
$this->assertEquals(3, iterator_count($rs));
// Test with the get_records_select().
$rs = $DB->get_counted_records_sql(
sql: "SELECT * FROM {{$tablename}}",
fullcountcolumn: 'fullcount',
sort: "course DESC",
limitfrom: 3,
limitnum: 2,
);
$resetrs = reset($rs);
// Check whether the fullcount column returns the correct number.
$this->assertEquals(5, $resetrs->fullcount);
// Check whether the 'limitfrom' works properly.
$this->assertEquals(2, $resetrs->course);
// Check whether the 'limitnum' works properly.
$this->assertEquals(2, count($rs));
}
}
/**

12
user/classes/table/participants.php

@ -404,19 +404,17 @@ class participants extends \table_sql implements dynamic_table {
* @param bool $useinitialsbar do you want to use the initials bar.
*/
public function query_db($pagesize, $useinitialsbar = true) {
global $DB;
list($twhere, $tparams) = $this->get_sql_where();
$psearch = new participants_search($this->course, $this->context, $this->filterset);
$total = $psearch->get_total_participants_count($twhere, $tparams);
$this->pagesize($pagesize, $total);
$sort = $this->get_sql_sort();
if ($sort) {
$sort = 'ORDER BY ' . $sort;
}
$this->use_pages = true;
$rawdata = $psearch->get_participants($twhere, $tparams, $sort, $this->get_page_start(), $this->get_page_size());
$total = $rawdata->current()->fullcount ?? 0;
$this->pagesize($pagesize, $total);
$this->rawdata = [];
foreach ($rawdata as $user) {

37
user/classes/table/participants_search.php

@ -105,26 +105,43 @@ class participants_search {
'params' => $params,
] = $this->get_participants_sql($additionalwhere, $additionalparams);
$sql = "{$outerselect}
FROM ({$innerselect}
FROM {$innerjoins}
{$innerwhere}
) {$subqueryalias}
{$outerjoins}
{$outerwhere}
{$sort}";
return $DB->get_recordset_sql($sql, $params, $limitfrom, $limitnum);
$select = "{$outerselect}
FROM ({$innerselect}
FROM {$innerjoins}
{$innerwhere}
) {$subqueryalias}
{$outerjoins}
{$outerwhere}";
return $DB->get_counted_recordset_sql(
sql: $select,
fullcountcolumn: 'fullcount',
sort: $sort,
params: $params,
limitfrom: $limitfrom,
limitnum: $limitnum,
);
}
/**
* Returns the total number of participants for a given course.
*
* @deprecated Moodle 4.5 MDL-78030 - No longer used since the total count can be obtained from {@see ::get_participants()}.
* @todo Final deprecation on Moodle 6.0 MDL-82441.
*
* @param string $additionalwhere Any additional SQL to add to where.
* @param array $additionalparams The additional params used by $additionalwhere.
* @return int
*/
#[\core\attribute\deprecated(
'participants_search::get_participants()',
since: '4.5',
mdl: 'MDL-78030',
reason: 'No longer used since the total count can be obtained from {@see ::get_participants()}',
)]
public function get_total_participants_count(string $additionalwhere = '', array $additionalparams = []): int {
\core\deprecation::emit_deprecation_if_present([$this, __FUNCTION__]);
global $DB;
[

37
user/tests/table/participants_search_test.php

@ -169,9 +169,11 @@ final class participants_search_test extends advanced_testcase {
$rs = $search->get_participants();
$this->assertInstanceOf(moodle_recordset::class, $rs);
$records = $this->convert_recordset_to_array($rs);
$resetrecords = reset($records);
$totalparticipants = $resetrecords->fullcount ?? 0;
$this->assertCount($count, $records);
$this->assertEquals($count, $search->get_total_participants_count());
$this->assertEquals($count, $totalparticipants);
foreach ($expectedusers as $expecteduser) {
$this->assertArrayHasKey($users[$expecteduser]->id, $records);
@ -779,9 +781,10 @@ final class participants_search_test extends advanced_testcase {
// Run the search, assert count matches the number of expected users.
$search = new participants_search($course, context_course::instance($course->id), $filterset);
$this->assertEquals(count($expectedusers), $search->get_total_participants_count());
$rs = $search->get_participants();
$totalparticipants = $rs->current()->fullcount ?? 0;
$this->assertEquals(count($expectedusers), $totalparticipants);
$this->assertInstanceOf(moodle_recordset::class, $rs);
// Assert that each expected user is within the participant records.
@ -977,9 +980,11 @@ final class participants_search_test extends advanced_testcase {
$rs = $search->get_participants();
$this->assertInstanceOf(moodle_recordset::class, $rs);
$records = $this->convert_recordset_to_array($rs);
$resetrecords = reset($records);
$totalparticipants = $resetrecords->fullcount ?? 0;
$this->assertCount($count, $records);
$this->assertEquals($count, $search->get_total_participants_count());
$this->assertEquals($count, $totalparticipants);
foreach ($expectedusers as $expecteduser) {
$this->assertArrayHasKey($users[$expecteduser]->id, $records);
@ -1523,9 +1528,11 @@ final class participants_search_test extends advanced_testcase {
$rs = $search->get_participants();
$this->assertInstanceOf(moodle_recordset::class, $rs);
$records = $this->convert_recordset_to_array($rs);
$resetrecords = reset($records);
$totalparticipants = $resetrecords->fullcount ?? 0;
$this->assertCount($count, $records);
$this->assertEquals($count, $search->get_total_participants_count());
$this->assertEquals($count, $totalparticipants);
foreach ($expectedusers as $expecteduser) {
$this->assertArrayHasKey($users[$expecteduser]->id, $records);
@ -1778,9 +1785,11 @@ final class participants_search_test extends advanced_testcase {
$rs = $search->get_participants();
$this->assertInstanceOf(moodle_recordset::class, $rs);
$records = $this->convert_recordset_to_array($rs);
$resetrecords = reset($records);
$totalparticipants = $resetrecords->fullcount ?? 0;
$this->assertCount($count, $records);
$this->assertEquals($count, $search->get_total_participants_count());
$this->assertEquals($count, $totalparticipants);
foreach ($expectedusers as $expecteduser) {
$this->assertArrayHasKey($users[$expecteduser]->id, $records);
@ -2002,9 +2011,11 @@ final class participants_search_test extends advanced_testcase {
$rs = $search->get_participants();
$this->assertInstanceOf(moodle_recordset::class, $rs);
$records = $this->convert_recordset_to_array($rs);
$resetrecords = reset($records);
$totalparticipants = $resetrecords->fullcount ?? 0;
$this->assertCount($count, $records);
$this->assertEquals($count, $search->get_total_participants_count());
$this->assertEquals($count, $totalparticipants);
foreach ($expectedusers as $expecteduser) {
$this->assertArrayHasKey($users[$expecteduser]->id, $records);
@ -2340,9 +2351,11 @@ final class participants_search_test extends advanced_testcase {
$rs = $search->get_participants();
$this->assertInstanceOf(moodle_recordset::class, $rs);
$records = $this->convert_recordset_to_array($rs);
$resetrecords = reset($records);
$totalparticipants = $resetrecords->fullcount ?? 0;
$this->assertCount($count, $records);
$this->assertEquals($count, $search->get_total_participants_count());
$this->assertEquals($count, $totalparticipants);
foreach ($expectedusers as $expecteduser) {
$this->assertArrayHasKey($users[$expecteduser]->id, $records);
@ -2701,9 +2714,11 @@ final class participants_search_test extends advanced_testcase {
$rs = $search->get_participants();
$this->assertInstanceOf(moodle_recordset::class, $rs);
$records = $this->convert_recordset_to_array($rs);
$resetrecords = reset($records);
$totalparticipants = $resetrecords->fullcount ?? 0;
$this->assertCount($count, $records);
$this->assertEquals($count, $search->get_total_participants_count());
$this->assertEquals($count, $totalparticipants);
foreach ($expectedusers as $expecteduser) {
$this->assertArrayHasKey($users[$expecteduser]->id, $records);
@ -3106,9 +3121,11 @@ final class participants_search_test extends advanced_testcase {
$rs = $search->get_participants();
$this->assertInstanceOf(moodle_recordset::class, $rs);
$records = $this->convert_recordset_to_array($rs);
$resetrecords = reset($records);
$totalparticipants = $resetrecords->fullcount ?? 0;
$this->assertCount($count, $records);
$this->assertEquals($count, $search->get_total_participants_count());
$this->assertEquals($count, $totalparticipants);
foreach ($expectedusers as $expecteduser) {
$this->assertArrayHasKey($users[$expecteduser]->id, $records);