Project

General

Profile

Actions

Bug #100999

closed

Incoherent QueryInterface

Added by Adrian Rudnik 11 months ago. Updated 11 months ago.

Status:
Resolved
Priority:
Should have
Category:
Extbase
Target version:
-
Start date:
2023-06-12
Due date:
% Done:

100%

Estimated time:
TYPO3 Version:
11
PHP Version:
8.1
Tags:
Complexity:
easy
Is Regression:
Sprint Focus:

Description

I just had the requirement to get the total count of a given query.

The query was created with `\TYPO3\CMS\Extbase\Persistence\RepositoryInterface::createQuery`.
The repository interface correctly returns an `\TYPO3\CMS\Extbase\Persistence\QueryInterface`.

From here on this gets pretty incoherent. The first attempt is to simply

$query->setOffset(null);
$query->setLimit(null);

Which is not possible, due to hard type checks. Trying to use a stupid `setLimit(-1)` or `setLimit(0)` just leads to an hard exit `The limit must be an integer >= 1` exception from `\TYPO3\CMS\Extbase\Persistence\Generic\Query::setLimit`, which implements `\TYPO3\CMS\Extbase\Persistence\QueryInterface::setLimit`.

Browsing the `QueryInterface` offers no option to unset the limit in any way. The implementation however seems to support a `\TYPO3\CMS\Extbase\Persistence\Generic\Query::unsetLimit`, which is not a part of the interface, therefore I need to do a

if (!($query instanceof Query)) {
    throw new \InvalidArgumentException('Query must be instance of ' . Query::class);
}

on every part where I interact with the repository interface.

It would be nice to have:

- A clean interface that documents how to unset limit (and maybe offset)
- A non-failure on `LIMIT 0`, it is allowed in many drivers. I do not see any reason for the ORM query builder to escalate those those calls, as the result is still valid. It might not be used often though.

Also there is a PHP warning triggered when using `unsetLimit` of the `Query` implementation:

Core: Error handler (FE): PHP Warning: Undefined property: TYPO3\\CMS\\Extbase\\Persistence\\Generic\\Query::$limit in /var/www/html/public/typo3/sysext/extbase/Classes/Persistence/Generic/Query.php line 315

This is due to using `unset($this->limit)` and then returning it with `return $this->limit` during execution without checking for its existance.


Related issues 1 (0 open1 closed)

Related to TYPO3 Core - Bug #101153: PHP Warning: Undefined property: TYPO3\CMS\Extbase\Persistence\Generic\Query::$limitClosedStefan Froemken2023-06-22

Actions
Actions #1

Updated by Chris Müller 11 months ago

  • Category changed from Database API (Doctrine DBAL) to Extbase
Actions #2

Updated by Gerrit Code Review 11 months ago

  • Status changed from New to Under Review

Patch set 1 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/79489

Actions #3

Updated by Stefan Froemken 11 months ago

  • Assignee set to Stefan Froemken

Hello,

thank you for providing this information to us. Yes, using unset() on an object property is pretty nasty. I have provided a patch which should solve that issue.
I have never used unsetLimit(), but I have created my own Query objects based on QueryInterface. Adapting unsetQuery to the interface is a breaking change. I don't think that we will adapt into QueryInterface.
If you need a better way to handle the extbase query I prefer using:

$query = $this->createQuery();
...
[build your extbase query]
...
$queryParser = GeneralUtility::makeInstance(Typo3DbQueryParser::class);
$queryBuilder = $queryParser->convertQueryToDoctrineQueryBuilder($query);
$queryBuilder->setMaxResults(null);

$query->statement($queryBuilder)->execute();

Stefan

Actions #4

Updated by Gerrit Code Review 11 months ago

Patch set 2 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/79489

Actions #5

Updated by Gerrit Code Review 11 months ago

Patch set 3 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/79489

Actions #6

Updated by Gerrit Code Review 11 months ago

Patch set 1 for branch 12.4 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/79423

Actions #7

Updated by Gerrit Code Review 11 months ago

Patch set 2 for branch 12.4 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/79423

Actions #8

Updated by Stefan Froemken 11 months ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100
Actions #9

Updated by Stefan Froemken 11 months ago

  • Related to Bug #101153: PHP Warning: Undefined property: TYPO3\CMS\Extbase\Persistence\Generic\Query::$limit added
Actions

Also available in: Atom PDF