Project

General

Profile

Actions

Bug #83074

open

The inSet (FIND_IN_SET) function generates invalid SQL

Added by Thanos no-lastname-given over 6 years ago. Updated 9 months ago.

Status:
Accepted
Priority:
Could have
Assignee:
-
Category:
Database API (Doctrine DBAL)
Target version:
-
Start date:
2017-11-23
Due date:
% Done:

0%

Estimated time:
TYPO3 Version:
8
PHP Version:
7.1
Tags:
Complexity:
Is Regression:
Sprint Focus:

Description

When building a custom query with getQueryBuilderForTable using the The inSet function in ExpressionBuilder.php, the SQL is wrong because the function returns the pair of fieldName - value in reverse order.

The solution is to change the line 364 of ExpressionBuilder.php to

return sprintf(
  'FIND_IN_SET(%s, %s)',
   $this->connection->quoteIdentifier($fieldName),
   $value
);

or to use plain sql to the query builder: 'FIND_IN_SET(el.element_type, '. $allowedTypes . ')', but with the addition single quotes in the argument string : "'x,c,z'"


Files

patch.diff (570 Bytes) patch.diff Thanos no-lastname-given, 2017-11-23 19:34

Related issues 1 (1 open0 closed)

Related to TYPO3 Core - Task #99451: PHPDoc, name of arguments and documentation not very intuitive for ExpressionBuilder::inSetNew2023-01-03

Actions
Actions #1

Updated by Michael Stucki over 6 years ago

  • Project changed from 9 to TYPO3 Core
  • TYPO3 Version set to 9

Wrong project. Moving...

Actions #2

Updated by Morton Jonuschat over 6 years ago

  • Category set to Database API (Doctrine DBAL)
  • Status changed from New to Needs Feedback
  • Priority changed from Must have to -- undefined --

The inSet function as provided by the ExpressionBuilder is meant to find database rows which have a certain value $value in a column $fieldName which contains a list of comma separated values. InSet() is used quite frequently in the core and is almost certainly working as expected.

If you are trying to find all rows which have a single value in a column which is a member of a set of values you should b using the ->in() expression.

Otherwise a comprehensive code example/a complete example of what you are trying to accomplish might be helpful.

Actions #3

Updated by Thanos no-lastname-given over 6 years ago

  • TYPO3 Version changed from 9 to 8
  • PHP Version set to 7.1

I made an custom sql that looks like this:

  $queryBuilder = GeneralUtility::makeInstance(ConnectionPool::class)->getQueryBuilderForTable('table');
        $queryBuilder->getRestrictions()->removeAll();
        $queryBuilder->select('el.uid')
            ->from('table', 'el')
            ->where($queryBuilder->expr()->inSet('el.element_type', $queryBuilder->createNamedParameter($allowedTypes,\PDO::PARAM_STR)));

This outputs:

SELECT `el`.`uid` FROM `table` `el` WHERE FIND_IN_
   SET(:dcValue1, `el`.`element_type`)

which is an incorrect sql statement.

If i reverse the order of the return arguments inside the inSet function then i get:

SELECT `el`.`uid` FROM `table` `el` WHERE FIND_IN_
   SET(`el`.`element_type`, :dcValue1)

which is a valid sql query for the MySql that returns the correct values.

Actions #4

Updated by Alexander Opitz about 6 years ago

@Thanos

As Morton already wrote, you are using the function wrong.

The two parameters of inSet which you are using inSet(string $fieldName, string $value, ...) are defined:
$fieldName ... The name of the field, which contains a comma separated string/set which will be searched for.
$value ... The element which is searched inside this set (This string SHOULD NOT contain commas).

As you are using a NamedParameter you do not get an exception while calling the inSet method as the NamedParameter can't be quantified at that moment.

You should use the in() method which is defined as:

$fieldName The field to search in
$value ... The comma separated string/set( or array which will be imploded) for which will be searched inside the given field.

Actions #5

Updated by Georg Ringer about 6 years ago

  • Status changed from Needs Feedback to Closed

closed because of wrong usage + no feedback

Actions #6

Updated by Sybille Peters over 1 year ago

  • Related to Task #99451: PHPDoc, name of arguments and documentation not very intuitive for ExpressionBuilder::inSet added
Actions #7

Updated by Jo Hasenau over 1 year ago

  • Status changed from Closed to Accepted

Reopening the issue, because the usage would have been correct, if inSet would actually reflect the possible use cases of FIND_IN_SET .

According to the MySQL docs FIND_IN_SET will find a needle in a haystack of comma separated values and if there is a needle in the haystack, it returns the position of the needle.

https://dev.mysql.com/doc/refman/8.0/en/string-functions.html#function_find-in-set

The first paramater is the needle, the second one the haystack, both of them can be defined as a string or a field, bot none of them has to be a field.

So each of the following would be a correct SQL statement and each of them might be necessary to accomplish a desired FIND_IN SET

FIND_IN_SET(fieldname1, 'a,b,c')
FIND_IN_SET('a', fieldname1)
FIND_IN_SET(fieldname1, fieldname2)
FIND_IN_SET('a', 'a,b,c')

The core currently only allows

FIND_IN_SET('a', fieldname1)
FIND_IN_SET(fieldname1, fieldname2)

The variant with two fieldnames will be achieved using a switch

public function inSet(string $fieldName, string $value, bool $isColumn = false): string

See #99451

Actions #8

Updated by Christian Kuhn over 1 year ago

  • Priority changed from -- undefined -- to Could have
Actions #9

Updated by Michael Oehlhof about 1 year ago

I have to migrate this orderBy Statement from the $GLOBALS['TYPO3_DB']->exec_SELECTquery() to the QueryBuilder->orderBy()

FIND_IN_SET(tt_address.uid,' . $find_in_set . ')

Using QueryBuilder->expr()->inSet('tt_address.uid', $find_in_set) in the orderBy() method won't work because the result is escaped as a field (column) name.

Actions #10

Updated by David Bruchmann 9 months ago

Just for interest, in combination with #100970 I ended up with this as workaround:

    public function getCObjects()
    {
        $colPosPriority = $this->getColPosPriority();
        $results = [];
        foreach ($colPosPriority as $count => $colPos) {
            $connection = GeneralUtility::makeInstance(ConnectionPool::class)->getConnectionForTable('tt_content');
            $result = $connection->select(
                ['uid','pid','header','select_key','json_field', 'rowDescription'],  // fields
                'tt_content',                                                        // table
                [                                                                    // where conditions
                    'pid' => (int)$GLOBALS['TSFE']->id,
                    'CType' => 'my_type',
                    'colPos' => (int)$colPos
                ],
                [],                                                                  // grouping
                [                                                                    // sorting
                    // 'FIND_IN_SET(colPos, "' . implode(', ', $colPosPriority) . '")',
                    'sorting' => 'ASC'
                ]
            );
            $result = $result->fetchAll();
            if ($result) {
                $results = array_merge($results, $result);
            }
        }
        return $results;
    }
Actions

Also available in: Atom PDF