search

sonata-project / SonataDoctrineORMAdminBundle

Integrate Doctrine ORM into the SonataAdminBundle
https://docs.sonata-project.org/projects/SonataDoctrineORMAdminBundle
MIT License
445 stars 344 forks source link

Export not working when sorting on ManyToOne column #1586

Closed willemverspyck closed 2 years ago

willemverspyck commented 2 years ago

Environment

Sonata packages

show

``` $ composer show --latest 'sonata-project/*' sonata-project/admin-bundle 4.3.2 4.3.2 The missing Symfony Admin Generator sonata-project/block-bundle 4.8.0 4.8.0 Symfony SonataBlockBundle sonata-project/cache 2.2.0 2.2.0 Cache library sonata-project/doctrine-extensions 1.15.0 1.15.0 Doctrine2 behavioral extensions sonata-project/doctrine-orm-admin-bundle 4.2.0 4.2.0 Integrate Doctrine ORM into the SonataAdminBundle sonata-project/exporter 2.9.1 2.9.1 Lightweight Exporter library sonata-project/form-extensions 1.12.1 1.12.1 Symfony form extensions sonata-project/twig-extensions 1.9.0 1.9.0 Sonata twig extensions ```

Symfony packages

show

``` $ composer show --latest 'symfony/*' symfony/amqp-messenger v5.3.7 v5.3.7 Symfony AMQP extension Messenger Bridge symfony/apache-pack v1.0.1 v1.0.1 A pack for Apache support in Symfony symfony/asset v5.3.11 v5.3.11 Manages URL generation and versioning of web assets such as CSS stylesheets, Jav... symfony/cache v5.3.12 v5.3.12 Provides an extended PSR-6, PSR-16 (and tags) implementation symfony/cache-contracts v2.5.0 v3.0.0 Generic abstractions related to caching symfony/config v5.3.11 v5.3.11 Helps you find, load, combine, autofill and validate configuration values of any... symfony/console v5.3.11 v5.3.11 Eases the creation of beautiful and testable command line interfaces symfony/css-selector v5.3.4 v5.3.4 Converts CSS selectors to XPath expressions symfony/debug-bundle v5.3.4 v5.3.4 Provides a tight integration of the Symfony Debug component into the Symfony ful... symfony/dependency-injection v5.3.11 v5.3.11 Allows you to standardize and centralize the way objects are constructed in your... symfony/deprecation-contracts v2.5.0 v3.0.0 A generic function and convention to trigger deprecation notices symfony/doctrine-bridge v5.3.11 v5.3.11 Provides integration for Doctrine with various Symfony components symfony/doctrine-messenger v5.3.10 v5.3.10 Symfony Doctrine Messenger Bridge symfony/dom-crawler v5.3.7 v5.3.7 Eases DOM navigation for HTML and XML documents symfony/dotenv v5.3.10 v5.3.10 Registers environment variables from a .env file symfony/error-handler v5.3.11 v5.3.11 Provides tools to manage errors and ease debugging PHP code symfony/event-dispatcher v5.3.11 v5.3.11 Provides tools that allow your application components to communicate with each o... symfony/event-dispatcher-contracts v2.5.0 v3.0.0 Generic abstractions related to dispatching event symfony/expression-language v5.3.11 v5.3.11 Provides an engine that can compile and evaluate expressions symfony/filesystem v5.3.4 v5.3.4 Provides basic utilities for the filesystem symfony/finder v5.3.7 v5.3.7 Finds files and directories via an intuitive fluent interface symfony/flex v1.17.5 v1.17.5 Composer plugin for Symfony symfony/form v5.3.11 v5.3.11 Allows to easily create, process and reuse HTML forms symfony/framework-bundle v5.3.11 v5.3.11 Provides a tight integration between Symfony components and the Symfony full-sta... symfony/http-client v5.3.11 v5.3.11 Provides powerful methods to fetch HTTP resources synchronously or asynchronously symfony/http-client-contracts v2.5.0 v3.0.0 Generic abstractions related to HTTP clients symfony/http-foundation v5.3.11 v5.3.11 Defines an object-oriented layer for the HTTP specification symfony/http-kernel v5.3.12 v5.3.12 Provides a structured process for converting a Request into a Response symfony/intl v5.3.11 v5.3.11 Provides a PHP replacement layer for the C intl extension that includes addition... symfony/lock v5.3.10 v5.3.10 Creates and manages locks, a mechanism to provide exclusive access to a shared r... symfony/mailer v5.3.9 v5.3.9 Helps sending emails symfony/maker-bundle v1.36.3 v1.36.3 Symfony Maker helps you create empty commands, controllers, form classes, tests ... symfony/messenger v5.3.11 v5.3.11 Helps applications send and receive messages to/from other applications or via m... symfony/mime v5.3.11 v5.3.11 Allows manipulating MIME messages symfony/monolog-bridge v5.3.7 v5.3.7 Provides integration for Monolog with various Symfony components symfony/monolog-bundle v3.7.1 v3.7.1 Symfony MonologBundle symfony/options-resolver v5.3.7 v5.3.7 Provides an improved replacement for the array_replace PHP function symfony/password-hasher v5.3.8 v5.3.8 Provides password hashing utilities symfony/phpunit-bridge v5.3.11 v5.3.11 Provides utilities for PHPUnit, especially user deprecation notices management symfony/polyfill-ctype v1.23.0 v1.23.0 Symfony polyfill for ctype functions symfony/polyfill-intl-grapheme v1.23.1 v1.23.1 Symfony polyfill for intl's grapheme_* functions symfony/polyfill-intl-icu v1.23.0 v1.23.0 Symfony polyfill for intl's ICU-related data and classes symfony/polyfill-intl-idn v1.23.0 v1.23.0 Symfony polyfill for intl's idn_to_ascii and idn_to_utf8 functions symfony/polyfill-intl-normalizer v1.23.0 v1.23.0 Symfony polyfill for intl's Normalizer class and related functions symfony/polyfill-mbstring v1.23.1 v1.23.1 Symfony polyfill for the Mbstring extension symfony/polyfill-php72 v1.23.0 v1.23.0 Symfony polyfill backporting some PHP 7.2+ features to lower PHP versions symfony/polyfill-php73 v1.23.0 v1.23.0 Symfony polyfill backporting some PHP 7.3+ features to lower PHP versions symfony/polyfill-php80 v1.23.1 v1.23.1 Symfony polyfill backporting some PHP 8.0+ features to lower PHP versions symfony/polyfill-php81 v1.23.0 v1.23.0 Symfony polyfill backporting some PHP 8.1+ features to lower PHP versions symfony/process v5.3.12 v5.3.12 Executes commands in sub-processes symfony/property-access v5.3.8 v5.3.8 Provides functions to read and write from/to an object or array using a simple s... symfony/property-info v5.3.8 v5.3.8 Extracts information about PHP class' properties using metadata of popular sources symfony/proxy-manager-bridge v5.3.11 v5.3.11 Provides integration for ProxyManager with various Symfony components symfony/psr-http-message-bridge v2.1.2 v2.1.2 PSR HTTP message bridge symfony/rate-limiter v5.3.11 v5.3.11 Provides a Token Bucket implementation to rate limit input and output in your ap... symfony/redis-messenger v5.3.10 v5.3.10 Symfony Redis extension Messenger Bridge symfony/routing v5.3.11 v5.3.11 Maps an HTTP request to a set of configuration variables symfony/runtime v5.3.11 v5.3.11 Enables decoupling PHP applications from global state symfony/security-acl v3.2.0 v3.2.0 Symfony Security Component - ACL (Access Control List) symfony/security-bundle v5.3.12 v5.3.12 Provides a tight integration of the Security component into the Symfony full-sta... symfony/security-core v5.3.11 v5.3.11 Symfony Security Component - Core Library symfony/security-csrf v5.3.4 v5.3.4 Symfony Security Component - CSRF Library symfony/security-guard v5.3.7 v5.3.7 Symfony Security Component - Guard symfony/security-http v5.3.11 v5.3.11 Symfony Security Component - HTTP Integration symfony/serializer v5.3.12 v5.3.12 Handles serializing and deserializing data structures, including object graphs, ... symfony/service-contracts v2.5.0 v3.0.0 Generic abstractions related to writing services symfony/stopwatch v5.3.4 v5.3.4 Provides a way to profile code symfony/string v5.3.10 v5.3.10 Provides an object-oriented API to strings and deals with bytes, UTF-8 code poin... symfony/templating v5.3.7 v5.3.7 Provides all the tools needed to build any kind of template system symfony/translation v5.3.11 v5.3.11 Provides tools to internationalize your application symfony/translation-contracts v2.5.0 v3.0.0 Generic abstractions related to translation symfony/twig-bridge v5.3.11 v5.3.11 Provides integration for Twig with various Symfony components symfony/twig-bundle v5.3.10 v5.3.10 Provides a tight integration of Twig into the Symfony full-stack framework symfony/validator v5.3.12 v5.3.12 Provides tools to validate values symfony/var-dumper v5.3.11 v5.3.11 Provides mechanisms for walking through any arbitrary PHP variable symfony/var-exporter v5.3.11 v5.3.11 Allows exporting any serializable PHP data structure to plain PHP code symfony/web-link v5.3.4 v5.3.4 Manages links between resources symfony/web-profiler-bundle v5.3.8 v5.3.8 Provides a development tool that gives detailed information about the execution ... symfony/yaml v5.3.11 v5.3.11 Loads and dumps YAML files ```

PHP version

$ php -v
PHP 8.0.12 (cli) (built: Oct 22 2021 12:34:48) ( NTS )

Subject

Steps to reproduce

I have this "Admin" definitions:

/**
 * Configure list fields
 *
 * @param ListMapper $listMapper
 */
protected function configureListFields(ListMapper $listMapper): void
{
    $listMapper
        ->add('company')
        ->add('cost.name')
        ->add('cost.reference')
        ->add('cost.amount', FieldDescriptionInterface::TYPE_CURRENCY, [
            'currency' => 'EUR',
        ])
        ->add(ListMapper::NAME_ACTIONS, null, [
            'actions' => [
                'show' => [],
                'edit' => [],
                'delete' => [],
            ],
        ]);
}

/**
 * @param ProxyQueryInterface $proxyQuery
 *
 * @return ProxyQueryInterface
 */
protected function configureQuery(ProxyQueryInterface $proxyQuery): ProxyQueryInterface
{
    $rootAlias = $proxyQuery->getRootAlias();

    $proxyQuery
        ->addSelect('cost')
        ->innerJoin(sprintf('%s.cost', $rootAlias), 'cost');

    return $proxyQuery;
}

When I sort on the column "cost.reference". The list overview works fine and the results are sorted. But when I want to export these results, I get:

An exception occurred while executing a query: SQLSTATE[HY000]: General error: 3065 Expression # 1 of ORDER BY clause is not in SELECT list, references column '[DATABASE].c1_.reference' which is not in SELECT list; this is incompatible with DISTINCT

The current DQL with the error is:

SELECT DISTINCT o FROM App\Entity\Entity o INNER JOIN o.cost cost ORDER BY cost.reference ASC, o.id ASC

I think it should be:

SELECT DISTINCT o, cost FROM App\Entity\Entity o INNER JOIN o.cost cost ORDER BY cost.reference ASC, o.id ASC

If I remove the "distinct()" row in https://github.com/sonata-project/SonataDoctrineORMAdminBundle/blob/4.x/src/Exporter/DataSource.php#L39 it works. But as mentioned in the comment "Distinct is needed to iterate, even if group by is used". So this can't be removed.

Or, if I remove the "select()" row in https://github.com/sonata-project/SonataDoctrineORMAdminBundle/blob/4.x/src/Exporter/DataSource.php#L40 it works. It looks like "o" is already added by default and in my "configureQuery", I already did "addSelect('cost')".

If I sort on "company" (column that is not part of the ManyToOne), it works fine.

Please advice what should be a good solution for this (for PR)?

Expected results

The exporter to export the results sorted

Actual results

MySQL "distinct" error

VincentLanglet commented 2 years ago

In 3.x there was this code: https://github.com/sonata-project/SonataDoctrineORMAdminBundle/blob/3.x/src/Exporter/DataSource.php#L42-L53 Did it work before with it ?

SELECT DISTINCT o, cost FROM App\Entity\Entity o INNER JOIN o.cost cost ORDER BY cost.reference ASC, o.id ASC

If I understand, the o entity has a OtM with cost ? If you do this query, with 200 entity o, and let's say 2 cost by entity o, I think you'll end up with 400 lines of exports, when you only have 200 lines in your admin.

You can look at the query on the admin list, you'll see the Doctrine Paginator is doing some playing with distinct, selecting id, etc.

Or, if I remove the "select()" row in https://github.com/sonata-project/SonataDoctrineORMAdminBundle/blob/4.x/src/Exporter/DataSource.php#L40 it works. It looks like "o" is already added by default and in my "configureQuery", I already did "addSelect('cost')".

In your example, the admin list is still working without the addSelect. I think you only reduce the number of queries. And we shouldn't rely on the the addSelect added in the configureQuery. This would be too tricky for the developper. Also, like I said, if we add the select, you'll end up with more line exported.

There is some discussion here which might help https://stackoverflow.com/questions/5391564/how-to-use-distinct-and-order-by-in-same-select-statement

I would have try to use a group by instead of a Distinct but according to https://github.com/doctrine/orm/issues/5868 I'm not sure it would work. Dunno if this is an sql issue or a doctrine one.

willemverspyck commented 2 years ago

Sorry for my late response.

https://github.com/sonata-project/SonataDoctrineORMAdminBundle/blob/3.x/src/Exporter/DataSource.php#L42-L53

$sortBy = $query->getSortBy();

if (null !== $sortBy) {
    $query->addOrderBy($sortBy, $query->getSortOrder());
    $doctrineQuery = $query->getQuery();
    $doctrineQuery->setHint(Query::HINT_CUSTOM_TREE_WALKERS, [OrderByToSelectWalker::class]);
} else {
    $doctrineQuery = $query->getQuery();
}

Yes, if I add this code from 3.x again and add the file "OrderByToSelectWalker" from 3.x, it works.

My mistake. It was MtO, not OtM. So you don't have 400 lines.

I was looking in the code and tried some different approaches, but when I fix the OneToMany, I will break the ManyToOne or ManyToMany.

Replacing the "toIterator" to "getResult" in DoctrineORMQuerySourceIterator (and the other iterators) will work, but has to big impact en "getResult" uses a lot of more memory.

Another solutions will be to remove "distinct" only when there is not an ManyToMany or ManyToOne relation, but not sure how to check that (also you don't depend on the configureQuery for this): https://github.com/sonata-project/SonataDoctrineORMAdminBundle/blob/4.x/src/Exporter/DataSource.php#L39-L40

VincentLanglet commented 2 years ago

My mistake. It was MtO, not OtM. So you don't have 400 lines.

If adding ->addSelect('cost') solve the issue, it would be interesting to see if we cannot automatically add it in the DoctrineORMQuerySourceIterator. Or maybe better we should consider that this code is correct https://github.com/sonata-project/SonataDoctrineORMAdminBundle/blob/4.x/src/Exporter/DataSource.php#L39-L44 and the issue is in the getDoctrineQuery.

Here: https://github.com/sonata-project/SonataDoctrineORMAdminBundle/blob/4.x/src/Datagrid/ProxyQuery.php#L174 We should call addSelect if the query use distinct to avoid an invalid query. WDYT ?

willemverspyck commented 2 years ago

Thanks @VincentLanglet. If I use this code in ProxyQuery it looks like it working for MtM, MtO and OtM:

$orderByDQLPart = $queryBuilder->getDQLPart('orderBy');
$queryBuilder->resetDQLPart('orderBy');

$sortByParts = explode('.', $sortBy);

if (count($sortByParts) > 1) {
    if ($sortByParts[0] !== $rootAlias) {
        $queryBuilder->addSelect($sortByParts[0]);
    }
} else {
    $sortBy = $rootAlias.'.'.$sortBy;
}

$queryBuilder->addOrderBy($sortBy, $this->getSortOrder());

Before the query was:

SELECT DISTINCT o FROM App\Entity\Entity o INNER JOIN o.cost cost ORDER BY cost.reference ASC, o.id ASC

And with this code the query is:

SELECT DISTINCT o, cost FROM App\Entity\Entity o INNER JOIN o.cost cost ORDER BY cost.reference ASC, o.id ASC

And that works. Sounds like a solution?

VincentLanglet commented 2 years ago

It could be.

Since the getDoctrineQuery method is also used for the list. We should check

And we should add a unit/functional test for this issue.

VincentLanglet commented 2 years ago

Do you want to start the PR @willemverspyck ?

willemverspyck commented 2 years ago

Yes, I will create a PR for this 👍

VincentLanglet commented 2 years ago

HI @willemverspyck, did you have time to take a look ?

willemverspyck commented 2 years ago

Hi @VincentLanglet,

Sorry for the delay!

I've changed the subject as it's ManyToOne, not ManyToMany.

We talked about maybe fixing it in the https://github.com/sonata-project/SonataDoctrineORMAdminBundle/blob/4.x/src/Datagrid/ProxyQuery.php#L174 file, but the problem is created in the https://github.com/sonata-project/SonataDoctrineORMAdminBundle/blob/4.x/src/Exporter/DataSource.php#L39-L44 where the "distinct" is added, so I think the fix should be done in DataSource.php.

In the list part of Sonata you can only sort on columns that are not ManyToMany or OneToMany, so if I check if the alias of the "sortBy" is not added to the select (and is not part of the root alias) and then add it, it works again.

Do you think this is the right solution in #1639? Then I can create a test for this.