From bbf727f175300e449a7defec51a9054ef8a433d9 Mon Sep 17 00:00:00 2001
From: Frederic Massart <fred@moodle.com>
Date: Thu, 12 May 2016 14:39:29 +0800
Subject: [PATCH] MDL-54542 competency: Prevent long query aliases in
 get_sql_fields()

Otherwise we could hit ORA-00972 because they exceed 30 characters.
---
 competency/classes/api.php           | 16 +++++++--------
 competency/classes/persistent.php    | 10 +++++++++-
 competency/tests/persistent_test.php | 29 ++++++++++++++++++++++++++++
 3 files changed, 46 insertions(+), 9 deletions(-)

diff --git a/competency/classes/api.php b/competency/classes/api.php
index a70ac33de01..f58a59acd37 100644
--- a/competency/classes/api.php
+++ b/competency/classes/api.php
@@ -1332,8 +1332,8 @@ class api {
         }
 
         $capability = 'moodle/competency:usercompetencyreview';
-        $ucfields = user_competency::get_sql_fields('uc');
-        $compfields = competency::get_sql_fields('c');
+        $ucfields = user_competency::get_sql_fields('uc', 'uc_');
+        $compfields = competency::get_sql_fields('c', 'c_');
         $usercols = array('id') + get_user_fieldnames();
         $userfields = array();
         foreach ($usercols as $field) {
@@ -1376,8 +1376,8 @@ class api {
         $records = $DB->get_recordset_sql($getsql, $params, $skip, $limit);
         foreach ($records as $record) {
             $objects = (object) array(
-                'usercompetency' => new user_competency(0, user_competency::extract_record($record)),
-                'competency' => new competency(0, competency::extract_record($record)),
+                'usercompetency' => new user_competency(0, user_competency::extract_record($record, 'uc_')),
+                'competency' => new competency(0, competency::extract_record($record, 'c_')),
                 'user' => persistent::extract_record($record, 'usr_'),
             );
             $competencies[] = $objects;
@@ -2369,8 +2369,8 @@ class api {
             $userid = $USER->id;
         }
 
-        $planfields = plan::get_sql_fields('p');
-        $tplfields = template::get_sql_fields('t');
+        $planfields = plan::get_sql_fields('p', 'plan_');
+        $tplfields = template::get_sql_fields('t', 'tpl_');
         $usercols = array('id') + get_user_fieldnames();
         $userfields = array();
         foreach ($usercols as $field) {
@@ -2412,11 +2412,11 @@ class api {
         $plans = array();
         $records = $DB->get_recordset_sql($select . $sql, $params, $skip, $limit);
         foreach ($records as $record) {
-            $plan = new plan(0, plan::extract_record($record));
+            $plan = new plan(0, plan::extract_record($record, 'plan_'));
             $template = null;
 
             if ($plan->is_based_on_template()) {
-                $template = new template(0, template::extract_record($record));
+                $template = new template(0, template::extract_record($record, 'tpl_'));
             }
 
             $plans[] = (object) array(
diff --git a/competency/classes/persistent.php b/competency/classes/persistent.php
index 7db1c4858b9..fe94961f17f 100644
--- a/competency/classes/persistent.php
+++ b/competency/classes/persistent.php
@@ -785,6 +785,7 @@ abstract class persistent {
      * @return string The SQL fragment.
      */
     public static function get_sql_fields($alias, $prefix = null) {
+        global $CFG;
         $fields = array();
 
         if ($prefix === null) {
@@ -798,7 +799,14 @@ abstract class persistent {
         $properties = array('id' => $id) + $properties;
 
         foreach ($properties as $property => $definition) {
-            $fields[] = $alias . '.' . $property . ' AS ' . $prefix . $property;
+            $as = $prefix . $property;
+            $fields[] = $alias . '.' . $property . ' AS ' . $as;
+
+            // Warn developers that the query will not always work.
+            if ($CFG->debugdeveloper && strlen($as) > 30) {
+                throw new coding_exception("The alias '$as' for column '$alias.$property' exceeds 30 characters" .
+                    " and will therefore not work across all supported databases.");
+            }
         }
 
         return implode(', ', $fields);
diff --git a/competency/tests/persistent_test.php b/competency/tests/persistent_test.php
index c37f06d9c83..b9dc0fda24b 100644
--- a/competency/tests/persistent_test.php
+++ b/competency/tests/persistent_test.php
@@ -392,6 +392,35 @@ class core_competency_persistent_testcase extends advanced_testcase {
         $this->assertFalse(core_competency_testable_persistent::record_exists($id));
     }
 
+    public function test_get_sql_fields() {
+        $expected = '' .
+            'c.id AS comp_id, ' .
+            'c.shortname AS comp_shortname, ' .
+            'c.idnumber AS comp_idnumber, ' .
+            'c.description AS comp_description, ' .
+            'c.descriptionformat AS comp_descriptionformat, ' .
+            'c.parentid AS comp_parentid, ' .
+            'c.path AS comp_path, ' .
+            'c.sortorder AS comp_sortorder, ' .
+            'c.competencyframeworkid AS comp_competencyframeworkid, ' .
+            'c.ruletype AS comp_ruletype, ' .
+            'c.ruleconfig AS comp_ruleconfig, ' .
+            'c.ruleoutcome AS comp_ruleoutcome, ' .
+            'c.scaleid AS comp_scaleid, ' .
+            'c.scaleconfiguration AS comp_scaleconfiguration, ' .
+            'c.timecreated AS comp_timecreated, ' .
+            'c.timemodified AS comp_timemodified, ' .
+            'c.usermodified AS comp_usermodified';
+        $this->assertEquals($expected, core_competency_testable_persistent::get_sql_fields('c', 'comp_'));
+    }
+
+    /**
+     * @expectedException               coding_exception
+     * @expectedExceptionMessageRegExp  /The alias .+ exceeds 30 characters/
+     */
+    public function test_get_sql_fields_too_long() {
+        core_competency_testable_persistent::get_sql_fields('c');
+    }
 }
 
 /**