diff --git a/reportbuilder/classes/local/helpers/database.php b/reportbuilder/classes/local/helpers/database.php
index de250176270..a19694c12b7 100644
--- a/reportbuilder/classes/local/helpers/database.php
+++ b/reportbuilder/classes/local/helpers/database.php
@@ -101,6 +101,29 @@ class database {
return true;
}
+ /**
+ * Replace parameter names within given SQL expression, allowing caller to specify callback to handle their replacement
+ * primarily to ensure uniqueness when the expression is to be used as part of a larger query
+ *
+ * @param string $sql
+ * @param array $params
+ * @param callable $callback Method that takes a single string parameter, and returns another string
+ * @return string
+ */
+ public static function sql_replace_parameter_names(string $sql, array $params, callable $callback): string {
+ foreach ($params as $param) {
+
+ // Pattern to look for param within the SQL.
+ $pattern = '/:(?' . preg_quote($param) . ')\b/';
+
+ $sql = preg_replace_callback($pattern, function(array $matches) use ($callback): string {
+ return ':' . $callback($matches['param']);
+ }, $sql);
+ }
+
+ return $sql;
+ }
+
/**
* Generate SQL expression for sorting group concatenated fields
*
diff --git a/reportbuilder/classes/local/report/column.php b/reportbuilder/classes/local/report/column.php
index 5db3f8dd2c4..b5a344e5aaf 100644
--- a/reportbuilder/classes/local/report/column.php
+++ b/reportbuilder/classes/local/report/column.php
@@ -367,12 +367,12 @@ final class column {
$fields = [];
foreach ($this->fields as $alias => $sql) {
- // Ensure params within SQL are prefixed with column index.
- foreach ($this->params as $name => $value) {
- $sql = preg_replace_callback('/:(?' . preg_quote($name, '\b/') . ')/', function(array $matches): string {
- return ':' . $this->unique_param_name($matches['param']);
- }, $sql);
- }
+
+ // Ensure parameter names within SQL are prefixed with column index.
+ $params = array_keys($this->params);
+ $sql = database::sql_replace_parameter_names($sql, $params, function(string $param): string {
+ return $this->unique_param_name($param);
+ });
$fields[$alias] = [
'sql' => $sql,
diff --git a/reportbuilder/tests/local/helpers/database_test.php b/reportbuilder/tests/local/helpers/database_test.php
index 5dc496b5d51..22df8614a49 100644
--- a/reportbuilder/tests/local/helpers/database_test.php
+++ b/reportbuilder/tests/local/helpers/database_test.php
@@ -139,4 +139,31 @@ class database_test extends advanced_testcase {
$record = $DB->get_record_sql($sql, $params);
$this->assertEquals($admin->id, $record->{$userfieldalias});
}
+
+ /**
+ * Test replacement of parameter names within SQL statements
+ */
+ public function test_sql_replace_parameter_names(): void {
+ global $DB;
+
+ // Predefine parameter names, to ensure they don't overwrite each other.
+ [$param0, $param1, $param10] = ['rbparam0', 'rbparam1', 'rbparam10'];
+
+ $sql = "SELECT :{$param0} AS field0, :{$param1} AS field1, :{$param10} AS field10" . $DB->sql_null_from_clause();
+ $sql = database::sql_replace_parameter_names($sql, [$param0, $param1, $param10], static function(string $param): string {
+ return "prefix_{$param}";
+ });
+
+ $record = $DB->get_record_sql($sql, [
+ "prefix_{$param0}" => 'Zero',
+ "prefix_{$param1}" => 'One',
+ "prefix_{$param10}" => 'Ten',
+ ]);
+
+ $this->assertEquals((object) [
+ 'field0' => 'Zero',
+ 'field1' => 'One',
+ 'field10' => 'Ten',
+ ], $record);
+ }
}
diff --git a/reportbuilder/tests/local/report/column_test.php b/reportbuilder/tests/local/report/column_test.php
index 721b1a7b3b8..35ced84f97a 100644
--- a/reportbuilder/tests/local/report/column_test.php
+++ b/reportbuilder/tests/local/report/column_test.php
@@ -173,19 +173,31 @@ class column_test extends advanced_testcase {
* Test adding params to field, and retrieving them
*/
public function test_add_field_with_params(): void {
- $param = database::generate_param_name();
+ [$param0, $param1] = database::generate_param_names(2);
$column = $this->create_column('test')
->set_index(1)
- ->add_field(":{$param}", 'foo', [$param => 'bar']);
+ ->add_field(":{$param0}", 'foo', [$param0 => 'foo'])
+ ->add_field(":{$param1}", 'bar', [$param1 => 'bar']);
// Select will look like the following: "p_rbparam", where index is the column index and counter is
// a static value of the report helper class.
- $select = $column->get_fields();
- preg_match('/:(?p1_rbparam[\d]+) AS c1_foo/', $select[0], $matches);
+ $fields = $column->get_fields();
+ $this->assertCount(2, $fields);
+ preg_match('/:(?p1_rbparam[\d]+) AS c1_foo/', $fields[0], $matches);
$this->assertArrayHasKey('paramname', $matches);
- $this->assertEquals([$matches['paramname'] => 'bar'], $column->get_params());
+ $fieldparam0 = $matches['paramname'];
+
+ preg_match('/:(?p1_rbparam[\d]+) AS c1_bar/', $fields[1], $matches);
+ $this->assertArrayHasKey('paramname', $matches);
+ $fieldparam1 = $matches['paramname'];
+
+ // Ensure column parameters have been renamed appropriately.
+ $this->assertEquals([
+ $fieldparam0 => 'foo',
+ $fieldparam1 => 'bar',
+ ], $column->get_params());
}
/**
diff --git a/reportbuilder/upgrade.txt b/reportbuilder/upgrade.txt
index 110cd00adad..c6fabcb1cff 100644
--- a/reportbuilder/upgrade.txt
+++ b/reportbuilder/upgrade.txt
@@ -1,6 +1,11 @@
This file describes API changes in /reportbuilder/*
Information provided here is intended especially for developers.
+=== 4.1.3 ===
+
+* New database helper method `sql_replace_parameter_names` to help ensure uniqueness of parameters within an expression (where
+ that expression can be used multiple times as part of a larger query)
+
=== 4.1.2 ===
* The schedule helper `create_schedule` method accepts a `$timenow` parameter to use for comparisons against current date