[BUGFIX] dbal: Cast field to CHAR for FIND_IN_SET() 42/41842/2
authorMorton Jonuschat <m.jonuschat@mojocode.de>
Thu, 28 May 2015 17:50:47 +0000 (19:50 +0200)
committerMarkus Klein <markus.klein@typo3.org>
Wed, 22 Jul 2015 18:23:29 +0000 (20:23 +0200)
Implement explicit casting of fields to a character representation.
Most DBMS are stricter in regard to data type checking and emit an
error when trying to use FIND_IN_SET() on non-text field types.

On the DBAL side of things the DBMS specifics are used to define that
an explicit cast is required for FIND_IN_SET() so that a query including
the CAST() statement gets generated.

A PostgreSQL Specific has been added to enable the explicit casting in
conjuction with DBAL. To avoid checking repeatedly if a DBMS has defined
specific requirements a NullSpecific has been implemented that gets used
as a default.

In the DatabaseTreeDataProvider the listFieldQuery() function has been
changed to use an explicit CAST() instead of relying on the implicit
cast done by MySQL when comparing it to an empty string.

The SqlParser has been extended with the support for CAST().

Resolves: #67155
Resolves: #67172
Resolves: #46271
Releases: master, 6.2
Change-Id: Ic77d1700e0fb4e3723c90b34e131dafb456038e0
Reviewed-on: http://review.typo3.org/39779
Reviewed-by: Andreas Fernandez <typo3@scripting-base.de>
Tested-by: Andreas Fernandez <typo3@scripting-base.de>
Reviewed-by: Markus Klein <markus.klein@typo3.org>
Tested-by: Markus Klein <markus.klein@typo3.org>
Reviewed-on: http://review.typo3.org/41842

typo3/sysext/core/Classes/Database/SqlParser.php
typo3/sysext/core/Classes/Tree/TableConfiguration/DatabaseTreeDataProvider.php
typo3/sysext/core/Tests/Unit/Database/SqlParserTest.php
typo3/sysext/dbal/Classes/Database/DatabaseConnection.php
typo3/sysext/dbal/Classes/Database/Specifics/AbstractSpecifics.php
typo3/sysext/dbal/Classes/Database/Specifics/Postgres.php [new file with mode: 0644]
typo3/sysext/dbal/Tests/Unit/Database/DatabaseConnectionPostgresqlTest.php
typo3/sysext/dbal/Tests/Unit/Database/SqlParserTest.php

index a1693e9..391e664 100644 (file)
@@ -207,7 +207,7 @@ class SqlParser {
                                // Get field/value pairs:
                                while ($comma) {
                                        if ($fieldName = $this->nextPart($parseString, '^([[:alnum:]_]+)[[:space:]]*=')) {
-                                               // Strip of "=" sign.
+                                               // Strip off "=" sign.
                                                $this->nextPart($parseString, '^(=)');
                                                $value = $this->getValue($parseString);
                                                $result['FIELDS'][$fieldName] = $value;
@@ -712,7 +712,7 @@ class SqlParser {
                                        // Looking for a known function (only known functions supported)
                                        $func = $this->nextPart($parseString, '^(count|max|min|floor|sum|avg)[[:space:]]*\\(');
                                        if ($func) {
-                                               // Strip of "("
+                                               // Strip off "("
                                                $parseString = trim(substr($parseString, 1));
                                                $stack[$pnt]['type'] = 'function';
                                                $stack[$pnt]['function'] = $func;
@@ -964,7 +964,7 @@ class SqlParser {
                                // See if condition is EXISTS with a subquery
                                if (preg_match('/^EXISTS[[:space:]]*[(]/i', $parseString)) {
                                        $stack[$level][$pnt[$level]]['func']['type'] = $this->nextPart($parseString, '^(EXISTS)[[:space:]]*');
-                                       // Strip of "("
+                                       // Strip off "("
                                        $parseString = trim(substr($parseString, 1));
                                        $stack[$level][$pnt[$level]]['func']['subquery'] = $this->parseSELECT($parseString, $parameterReferences);
                                        // Seek to new position in parseString after parsing of the subquery
@@ -977,7 +977,7 @@ class SqlParser {
                                        // See if LOCATE function is found
                                        if (preg_match('/^LOCATE[[:space:]]*[(]/i', $parseString)) {
                                                $stack[$level][$pnt[$level]]['func']['type'] = $this->nextPart($parseString, '^(LOCATE)[[:space:]]*');
-                                               // Strip of "("
+                                               // Strip off "("
                                                $parseString = trim(substr($parseString, 1));
                                                $stack[$level][$pnt[$level]]['func']['substr'] = $this->getValue($parseString);
                                                if (!$this->nextPart($parseString, '^(,)')) {
@@ -1005,7 +1005,7 @@ class SqlParser {
                                        } elseif (preg_match('/^IFNULL[[:space:]]*[(]/i', $parseString)) {
                                                $stack[$level][$pnt[$level]]['func']['type'] = $this->nextPart($parseString, '^(IFNULL)[[:space:]]*');
                                                $parseString = trim(substr($parseString, 1));
-                                               // Strip of "("
+                                               // Strip off "("
                                                if ($fieldName = $this->nextPart($parseString, '^([[:alnum:]\\*._]+)[[:space:]]*')) {
                                                        // Parse field name into field and table:
                                                        $tableField = explode('.', $fieldName, 2);
@@ -1025,9 +1025,32 @@ class SqlParser {
                                                if (!$this->nextPart($parseString, '^([)])')) {
                                                        return $this->parseError('No ) parenthesis at end of function', $parseString);
                                                }
+                                       } elseif (preg_match('/^CAST[[:space:]]*[(]/i', $parseString)) {
+                                               $stack[$level][$pnt[$level]]['func']['type'] = $this->nextPart($parseString, '^(CAST)[[:space:]]*');
+                                               $parseString = trim(substr($parseString, 1));
+                                               // Strip off "("
+                                               if ($fieldName = $this->nextPart($parseString, '^([[:alnum:]\\*._]+)[[:space:]]*')) {
+                                                       // Parse field name into field and table:
+                                                       $tableField = explode('.', $fieldName, 2);
+                                                       if (count($tableField) === 2) {
+                                                               $stack[$level][$pnt[$level]]['func']['table'] = $tableField[0];
+                                                               $stack[$level][$pnt[$level]]['func']['field'] = $tableField[1];
+                                                       } else {
+                                                               $stack[$level][$pnt[$level]]['func']['table'] = '';
+                                                               $stack[$level][$pnt[$level]]['func']['field'] = $tableField[0];
+                                                       }
+                                               } else {
+                                                       return $this->parseError('No field name found as expected in parseWhereClause()', $parseString);
+                                               }
+                                               if ($this->nextPart($parseString, '^([[:space:]]*AS[[:space:]]*)')) {
+                                                       $stack[$level][$pnt[$level]]['func']['datatype'] = $this->getValue($parseString);
+                                               }
+                                               if (!$this->nextPart($parseString, '^([)])')) {
+                                                       return $this->parseError('No ) parenthesis at end of function', $parseString);
+                                               }
                                        } elseif (preg_match('/^FIND_IN_SET[[:space:]]*[(]/i', $parseString)) {
                                                $stack[$level][$pnt[$level]]['func']['type'] = $this->nextPart($parseString, '^(FIND_IN_SET)[[:space:]]*');
-                                               // Strip of "("
+                                               // Strip off "("
                                                $parseString = trim(substr($parseString, 1));
                                                if ($str = $this->getValue($parseString)) {
                                                        $stack[$level][$pnt[$level]]['func']['str'] = $str;
@@ -1848,6 +1871,11 @@ class SqlParser {
                                                $output .= ($v['func']['table'] ? $v['func']['table'] . '.' : '') . $v['func']['field'];
                                                $output .= ', ' . $v['func']['default'][1] . $this->compileAddslashes($v['func']['default'][0]) . $v['func']['default'][1];
                                                $output .= ')';
+                                       } elseif (isset($v['func']) && $v['func']['type'] === 'CAST') {
+                                               $output .= ' ' . trim($v['modifier']) . ' CAST(';
+                                               $output .= ($v['func']['table'] ? $v['func']['table'] . '.' : '') . $v['func']['field'];
+                                               $output .= ' AS ' . $v['func']['datatype'][0];
+                                               $output .= ')';
                                        } elseif (isset($v['func']) && $v['func']['type'] === 'FIND_IN_SET') {
                                                $output .= ' ' . trim($v['modifier']) . ' FIND_IN_SET(';
                                                $output .= $v['func']['str'][1] . $v['func']['str'][0] . $v['func']['str'][1];
index 689e166..2e78955 100644 (file)
@@ -419,7 +419,7 @@ class DatabaseTreeDataProvider extends AbstractTableConfigurationTreeDataProvide
         * @return integer[] all uids found
         */
        protected function listFieldQuery($fieldName, $queryId) {
-               $records = $GLOBALS['TYPO3_DB']->exec_SELECTgetRows('uid', $this->getTableName(), $GLOBALS['TYPO3_DB']->listQuery($fieldName, (int)$queryId, $this->getTableName()) . ((int)$queryId == 0 ? ' OR ' . $fieldName . ' = \'\'' : ''));
+               $records = $GLOBALS['TYPO3_DB']->exec_SELECTgetRows('uid', $this->getTableName(), $GLOBALS['TYPO3_DB']->listQuery($fieldName, (int)$queryId, $this->getTableName()) . ((int)$queryId === 0 ? ' OR CAST(' . $fieldName . ' AS CHAR) = \'\'' : ''));
                $uidArray = array();
                foreach ($records as $record) {
                        $uidArray[] = $record['uid'];
index 95260d5..ba784a1 100644 (file)
@@ -99,6 +99,21 @@ class SqlParserTest extends \TYPO3\CMS\Core\Tests\UnitTestCase {
                                        'field' => 'fe_group'
                                ),
                                'comparator' => ''
+                       ),
+                       5 => array(
+                               'operator' => 'OR',
+                               'modifier' => '',
+                               'func' => array(
+                                       'type' => 'CAST',
+                                       'table' => 'pages',
+                                       'field' => 'fe_group',
+                                       'datatype' => 'CHAR'
+                               ),
+                               'comparator' => '=',
+                               'value' => array(
+                                       0 => '',
+                                       1 => '\''
+                               )
                        )
                );
                $output = $this->fixture->compileWhereClause($clauses);
index f738431..a567248 100644 (file)
@@ -1750,7 +1750,17 @@ class DatabaseConnection extends \TYPO3\CMS\Core\Database\DatabaseConnection {
                                                // but it's not overridden from \TYPO3\CMS\Core\Database\DatabaseConnection at the moment...
                                                $patternForLike = $this->escapeStrForLike($pattern, $where_clause[$k]['func']['table']);
                                                $where_clause[$k]['func']['str_like'] = $patternForLike;
-                                               // Intentional fallthrough
+                                               if ($where_clause[$k]['func']['table'] !== '') {
+                                                       $where_clause[$k]['func']['table'] = $this->quoteName($v['func']['table']);
+                                               }
+                                               if ($where_clause[$k]['func']['field'] !== '') {
+                                                       if (!empty($this->dbmsSpecifics) && $this->dbmsSpecifics->getSpecific(Specifics\AbstractSpecifics::CAST_FIND_IN_SET)) {
+                                                               $where_clause[$k]['func']['field'] = 'CAST(' . $this->quoteName($v['func']['field']) . ' AS CHAR)';
+                                                       } else {
+                                                               $where_clause[$k]['func']['field'] = $this->quoteName($v['func']['field']);
+                                                       }
+                                               }
+                                               break;
                                        case 'IFNULL':
                                                // Intentional fallthrough
                                        case 'LOCATE':
index 7fb7e3a..b3e837e 100644 (file)
@@ -25,6 +25,7 @@ abstract class AbstractSpecifics {
        const TABLE_MAXLENGTH = 'table_maxlength';
        const FIELD_MAXLENGTH = 'field_maxlength';
        const LIST_MAXEXPRESSIONS = 'list_maxexpressions';
+       const CAST_FIND_IN_SET = 'cast_find_in_set';
 
        /**
         * Contains the specifics of a DBMS.
@@ -68,4 +69,4 @@ abstract class AbstractSpecifics {
 
                return array_chunk($expressionList, $this->getSpecific(self::LIST_MAXEXPRESSIONS), $preserveArrayKeys);
        }
-}
\ No newline at end of file
+}
diff --git a/typo3/sysext/dbal/Classes/Database/Specifics/Postgres.php b/typo3/sysext/dbal/Classes/Database/Specifics/Postgres.php
new file mode 100644 (file)
index 0000000..034a8bb
--- /dev/null
@@ -0,0 +1,33 @@
+<?php
+namespace TYPO3\CMS\Dbal\Database\Specifics;
+
+/*
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * 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.
+ */
+class Postgres extends AbstractSpecifics {
+       /**
+        * Contains the specifics that need to be taken care of for PostgreSQL DBMS.
+        *
+        * @var array
+        */
+       protected $specificProperties = array(
+               self::CAST_FIND_IN_SET => TRUE
+       );
+
+}
index 944289b..1f52954 100644 (file)
@@ -90,7 +90,7 @@ class DatabaseConnectionPostgresqlTest extends AbstractTestCase {
         */
        public function findInSetIsProperlyRemapped() {
                $result = $this->subject->SELECTquery('*', 'fe_users', 'FIND_IN_SET(10, usergroup)');
-               $expected = 'SELECT * FROM "fe_users" WHERE FIND_IN_SET(10, "usergroup") != 0';
+               $expected = 'SELECT * FROM "fe_users" WHERE FIND_IN_SET(10, CAST("usergroup" AS CHAR)) != 0';
                $this->assertEquals($expected, $this->cleanSql($result));
        }
 
index 84f0cb4..86f18c4 100644 (file)
@@ -271,6 +271,31 @@ class SqlParserTest extends AbstractTestCase {
 
        /**
         * @test
+        * @see http://forge.typo3.org/issues/67155
+        */
+       public function canParseCastOperator() {
+               $parseString = 'CAST(parent AS CHAR) != \'\'';
+               $result = $this->subject->parseWhereClause($parseString);
+               $this->assertInternalType('array', $result);
+               $this->assertEmpty($parseString);
+       }
+
+       /**
+        * @test
+        * @see http://forge.typo3.org/issues/67155
+        */
+       public function canCompileCastOperator() {
+               $parseString = 'SELECT * FROM sys_category WHERE CAST(parent AS CHAR) != \'\'';
+               $components = $this->subject->_callRef('parseSELECT', $parseString);
+               $this->assertInternalType('array', $components);
+
+               $result = $this->subject->_callRef('compileSELECT', $components);
+               $expected = 'SELECT * FROM sys_category WHERE CAST(parent AS CHAR) != \'\'';
+               $this->assertEquals($expected, $this->cleanSql($result));
+       }
+
+       /**
+        * @test
         * @see http://forge.typo3.org/issues/22695
         */
        public function canParseAlterEngineStatement() {