From 63224dee44a31313c4c6017ce87b5070e7f5b40c Mon Sep 17 00:00:00 2001
From: Petr Skoda <commits@skodak.org>
Date: Sun, 10 Jun 2012 10:26:30 +0200
Subject: [PATCH] MDL-33568 improve DB->count_records*()

Now always returns integer and invalid queries are detected.
---
 lib/dml/moodle_database.php |  8 ++++----
 lib/dml/tests/dml_test.php  | 34 +++++++++++++++++++++++++---------
 2 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/lib/dml/moodle_database.php b/lib/dml/moodle_database.php
index 48a992c73eb..bec6ca13bae 100644
--- a/lib/dml/moodle_database.php
+++ b/lib/dml/moodle_database.php
@@ -1578,11 +1578,11 @@ abstract class moodle_database {
      * @throws dml_exception A DML specific exception is thrown for any errors.
      */
     public function count_records_sql($sql, array $params=null) {
-        if ($count = $this->get_field_sql($sql, $params)) {
-            return $count;
-        } else {
-            return 0;
+        $count = $this->get_field_sql($sql, $params);
+        if ($count === false or !is_number($count) or $count < 0) {
+            throw new coding_exception("count_records_sql() expects the first field to contain non-negative number from COUNT(), '$count' found instead.");
         }
+        return (int)$count;
     }
 
     /**
diff --git a/lib/dml/tests/dml_test.php b/lib/dml/tests/dml_test.php
index a5ad8c25010..0f15ff5760c 100644
--- a/lib/dml/tests/dml_test.php
+++ b/lib/dml/tests/dml_test.php
@@ -2833,13 +2833,13 @@ class dml_testcase extends database_driver_testcase {
         $table->add_key('primary', XMLDB_KEY_PRIMARY, array('id'));
         $dbman->create_table($table);
 
-        $this->assertEquals(0, $DB->count_records($tablename));
+        $this->assertSame(0, $DB->count_records($tablename));
 
         $DB->insert_record($tablename, array('course' => 3));
         $DB->insert_record($tablename, array('course' => 4));
         $DB->insert_record($tablename, array('course' => 5));
 
-        $this->assertEquals(3, $DB->count_records($tablename));
+        $this->assertSame(3, $DB->count_records($tablename));
 
         // test for exception throwing on text conditions being compared. (MDL-24863, unwanted auto conversion of param to int)
         $conditions = array('onetext' => '1');
@@ -2868,13 +2868,13 @@ class dml_testcase extends database_driver_testcase {
         $table->add_key('primary', XMLDB_KEY_PRIMARY, array('id'));
         $dbman->create_table($table);
 
-        $this->assertEquals(0, $DB->count_records($tablename));
+        $this->assertSame(0, $DB->count_records($tablename));
 
         $DB->insert_record($tablename, array('course' => 3));
         $DB->insert_record($tablename, array('course' => 4));
         $DB->insert_record($tablename, array('course' => 5));
 
-        $this->assertEquals(2, $DB->count_records_select($tablename, 'course > ?', array(3)));
+        $this->assertSame(2, $DB->count_records_select($tablename, 'course > ?', array(3)));
     }
 
     public function test_count_records_sql() {
@@ -2886,16 +2886,32 @@ class dml_testcase extends database_driver_testcase {
 
         $table->add_field('id', XMLDB_TYPE_INTEGER, '10', XMLDB_UNSIGNED, XMLDB_NOTNULL, XMLDB_SEQUENCE, null);
         $table->add_field('course', XMLDB_TYPE_INTEGER, '10', XMLDB_UNSIGNED, XMLDB_NOTNULL, null, '0');
+        $table->add_field('onechar', XMLDB_TYPE_CHAR, '100', null, null, null);
         $table->add_key('primary', XMLDB_KEY_PRIMARY, array('id'));
         $dbman->create_table($table);
 
-        $this->assertEquals(0, $DB->count_records($tablename));
+        $this->assertSame(0, $DB->count_records($tablename));
 
-        $DB->insert_record($tablename, array('course' => 3));
-        $DB->insert_record($tablename, array('course' => 4));
-        $DB->insert_record($tablename, array('course' => 5));
+        $DB->insert_record($tablename, array('course' => 3, 'onechar' => 'a'));
+        $DB->insert_record($tablename, array('course' => 4, 'onechar' => 'b'));
+        $DB->insert_record($tablename, array('course' => 5, 'onechar' => 'c'));
 
-        $this->assertEquals(2, $DB->count_records_sql("SELECT COUNT(*) FROM {{$tablename}} WHERE course > ?", array(3)));
+        $this->assertSame(2, $DB->count_records_sql("SELECT COUNT(*) FROM {{$tablename}} WHERE course > ?", array(3)));
+
+        // test invalid use
+        try {
+            $DB->count_records_sql("SELECT onechar FROM {{$tablename}} WHERE course = ?", array(3));
+            $this->fail('Exception expected when non-number field used in count_records_sql');
+        } catch (exception $e) {
+            $this->assertInstanceOf('coding_exception', $e);
+        }
+
+        try {
+            $DB->count_records_sql("SELECT course FROM {{$tablename}} WHERE 1 = 2");
+            $this->fail('Exception expected when non-number field used in count_records_sql');
+        } catch (exception $e) {
+            $this->assertInstanceOf('coding_exception', $e);
+        }
     }
 
     public function test_record_exists() {