[BUGFIX] Prevent statements with COUNT() and ORDER BY parts for PostgreSQL 91/21091/10
authorAnja Leichsenring <aleichsenring@ab-softlab.de>
Sat, 4 Apr 2015 15:51:45 +0000 (17:51 +0200)
committerAnja Leichsenring <aleichsenring@ab-softlab.de>
Sat, 18 Jul 2015 14:11:30 +0000 (16:11 +0200)
In PostgreSQL it's not allowed to use a COUNT statement with an ORDER BY
statement as long as the field for sorting not available in GROUP BY
clause. Therefor we have to parse the SQL and drop ORDER BY clause for
selections with a count.

Resolves: #43262
Releases: master
Change-Id: I30cedfa2c25a5b2a8c6f7cb56c2dd2bc28e185ec
Reviewed-on: http://review.typo3.org/21091
Reviewed-by: Markus Klein <markus.klein@typo3.org>
Tested-by: Markus Klein <markus.klein@typo3.org>
Reviewed-by: Morton Jonuschat <m.jonuschat@mojocode.de>
Tested-by: Morton Jonuschat <m.jonuschat@mojocode.de>
Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
typo3/sysext/dbal/Classes/Database/DatabaseConnection.php
typo3/sysext/dbal/Classes/Database/Specifics/AbstractSpecifics.php
typo3/sysext/dbal/Classes/Database/Specifics/PostgresSpecifics.php
typo3/sysext/dbal/Tests/Unit/Database/DatabaseConnectionPostgresqlTest.php
typo3/sysext/dbal/Tests/Unit/Database/DatabaseConnectionTest.php

index d70ea3d..2b4eae7 100644 (file)
@@ -1252,6 +1252,7 @@ class DatabaseConnection extends \TYPO3\CMS\Core\Database\DatabaseConnection {
                $where_clause = $this->quoteWhereClause($where_clause);
                $groupBy = $this->quoteGroupBy($groupBy);
                $orderBy = $this->quoteOrderBy($orderBy);
+               $this->dbmsSpecifics->transformQueryParts($select_fields, $from_table, $where_clause, $groupBy, $orderBy, $limit);
                // Call parent method to build actual query
                $query = parent::SELECTquery($select_fields, $from_table, $where_clause, $groupBy, $orderBy, $limit);
                if ($this->debugOutput || $this->store_lastBuiltQuery) {
@@ -1285,6 +1286,7 @@ class DatabaseConnection extends \TYPO3\CMS\Core\Database\DatabaseConnection {
                }
                // Compile the SELECT parameters
                list($select_fields, $from_table, $where_clause, $groupBy, $orderBy) = $this->compileSelectParameters($params);
+               $this->dbmsSpecifics->transformQueryParts($select_fields, $from_table, $where_clause, $groupBy, $orderBy);
                // Call parent method to build actual query
                $query = parent::SELECTquery($select_fields, $from_table, $where_clause, $groupBy, $orderBy);
                if ($this->debugOutput || $this->store_lastBuiltQuery) {
@@ -1509,6 +1511,7 @@ class DatabaseConnection extends \TYPO3\CMS\Core\Database\DatabaseConnection {
                                $precompiledParts['queryParts'] = explode($parameterWrap, $query);
                                break;
                        case 'adodb':
+                               $this->dbmsSpecifics->transformQueryParts($select_fields, $from_table, $where_clause, $groupBy, $orderBy, $limit);
                                $query = parent::SELECTquery($select_fields, $from_table, $where_clause, $groupBy, $orderBy);
                                $precompiledParts['queryParts'] = explode($parameterWrap, $query);
                                $precompiledParts['LIMIT'] = $limit;
index cc85743..722aaeb 100644 (file)
@@ -150,6 +150,19 @@ abstract class AbstractSpecifics {
        }
 
        /**
+        * Adjust query parts for DBMS
+        *
+        * @param string $select_fields
+        * @param string $from_table
+        * @param string $where_clause
+        * @param string $groupBy
+        * @param string $orderBy
+        * @param string $limit
+        * @return void
+        */
+       public function transformQueryParts(&$select_fields, &$from_table, &$where_clause, &$groupBy = '', &$orderBy = '', &$limit = '') {}
+
+       /**
         * Transforms a database specific representation of field information and translates it
         * as close as possible to the MySQL standard.
         *
index 02a375c..6f9138f 100644 (file)
@@ -14,6 +14,8 @@ namespace TYPO3\CMS\Dbal\Database\Specifics;
  * The TYPO3 project - inspiring people to share!
  */
 
+use TYPO3\CMS\Core\Utility\GeneralUtility;
+
 /**
  * This class contains the specifics for PostgreSQL DBMS.
  * Any logic is in AbstractSpecifics.
@@ -132,4 +134,38 @@ class PostgresSpecifics extends AbstractSpecifics {
                return '';
        }
 
+       /**
+        * Adjust query parts for various DBMS
+        *
+        * @param string $select_fields
+        * @param string $from_table
+        * @param string $where_clause
+        * @param string $groupBy
+        * @param string $orderBy
+        * @param string $limit
+        * @return void
+        */
+       public function transformQueryParts(&$select_fields, &$from_table, &$where_clause, &$groupBy = '', &$orderBy = '', &$limit = '') {
+               // Strip orderBy part if select statement is a count
+               if (preg_match_all('/count\(([^)]*)\)/i', $select_fields, $matches)) {
+                       $orderByFields = GeneralUtility::trimExplode(',', $orderBy);
+                       $groupByFields = GeneralUtility::trimExplode(',', $groupBy);
+                       foreach ($matches[1] as $matchedField) {
+                               $field = $matchedField;
+                               // Lookup if the field in COUNT() statement is used in GROUP BY statement
+                               $index = array_search($field, $groupByFields, TRUE);
+                               if ($index !== FALSE) {
+                                       // field is used in GROUP BY, continue with next field
+                                       continue;
+                               }
+                               // If that field isn't used in GROUP BY statement, drop the ordering for compatibility reason
+                               $index = array_search($field, $orderByFields, TRUE);
+                               if ($index !== FALSE) {
+                                       unset($orderByFields[$index]);
+                               }
+                       }
+                       $orderBy = implode(', ', $orderByFields);
+               }
+       }
+
 }
index fedffc0..a4c905d 100644 (file)
@@ -172,4 +172,53 @@ class DatabaseConnectionPostgresqlTest extends AbstractTestCase {
                $this->assertSame($expected, $this->cleanSql($result));
        }
 
+       /**
+        * @test
+        * @see http://forge.typo3.org/issues/43262
+        */
+       public function countFieldInOrderByIsInGroupBy() {
+               $result = $this->subject->SELECTquery('COUNT(title)', 'pages', '', 'title', 'title');
+               $expected = 'SELECT COUNT("title") FROM "pages" GROUP BY "title" ORDER BY "title"';
+               $this->assertEquals($expected, $this->cleanSql($result));
+       }
+
+       /**
+        * @test
+        * @see http://forge.typo3.org/issues/43262
+        */
+       public function multipleCountFieldsInOrderByAreInGroupBy() {
+               $result = $this->subject->SELECTquery('COUNT(title), COUNT(pid)', 'pages', '', 'title, pid', 'title, pid');
+               $expected = 'SELECT COUNT("title"), COUNT("pid") FROM "pages" GROUP BY "title", "pid" ORDER BY "title", "pid"';
+               $this->assertEquals($expected, $this->cleanSql($result));
+       }
+
+       /**
+        * @test
+        * @see http://forge.typo3.org/issues/43262
+        */
+       public function countFieldInOrderByIsNotInGroupBy() {
+               $result = $this->subject->SELECTquery('COUNT(title)', 'pages', '', '', 'title');
+               $expected = 'SELECT COUNT("title") FROM "pages"';
+               $this->assertEquals($expected, $this->cleanSql($result));
+       }
+
+       /**
+        * @test
+        * @see http://forge.typo3.org/issues/43262
+        */
+       public function multipleCountFieldsInOrderByAreNotInGroupBy() {
+               $result = $this->subject->SELECTquery('COUNT(title), COUNT(pid)', 'pages', '', '', 'title, pid');
+               $expected = 'SELECT COUNT("title"), COUNT("pid") FROM "pages"';
+               $this->assertEquals($expected, $this->cleanSql($result));
+       }
+
+       /**
+        * @test
+        * @see http://forge.typo3.org/issues/43262
+        */
+       public function someCountFieldsInOrderByAreNotInGroupBy() {
+               $result = $this->subject->SELECTquery('COUNT(title), COUNT(pid)', 'pages', '', 'title', 'title, pid');
+               $expected = 'SELECT COUNT("title"), COUNT("pid") FROM "pages" GROUP BY "title" ORDER BY "title"';
+               $this->assertEquals($expected, $this->cleanSql($result));
+       }
 }
index 2afc2f8..c34b4fb 100644 (file)
@@ -49,6 +49,9 @@ class DatabaseConnectionTest extends AbstractTestCase {
                $installerSqlMock->expects($this->any())->method('getFieldDefinitions_fileContent')->will($this->returnValue(array()));
                $subject->_set('installerSql', $installerSqlMock);
 
+               // Inject DBMS specifics
+               $subject->_set('dbmsSpecifics', GeneralUtility::makeInstance(\TYPO3\CMS\Dbal\Database\Specifics\NullSpecifics::class));
+
                $subject->initialize();
                $subject->lastHandlerKey = '_DEFAULT';