Kreyu / data-table-bundle

Streamlines creation process of the data tables in Symfony applications. NOT PRODUCTION READY.
https://data-table-bundle.swroblewski.pl
MIT License
75 stars 14 forks source link

The Paginator does not support Queries which only yield ScalarResults exception #118

Closed davidromani closed 3 months ago

davidromani commented 3 months ago

Hi again.

Now I'm getting this exception error:

The Paginator does not support Queries which only yield ScalarResults

// Controller

public function batchAnalysisAction(Request $request, WorkOrderResultRepository $wor): Response
{
    $dataTable = $this->createDataTable(BatchAnalysisDataTableType::class, $wor->batchAnalysisQB());
    $dataTable->handleRequest($request);
    if ($dataTable->isExporting()) {
        return $this->file($dataTable->export());
    }

    return $this->render('@App/admin/reports/batch_analysis.html.twig', [
        'batchAnalysisTable' => $dataTable->createView(),
    ]);
}
// Repository

public function batchAnalysisQB(): QueryBuilder
{
    return $this->createQueryBuilder('wor')
        ->select('b.code AS batchCode')
        ->addSelect('CAST(SUM(wor.workstationSeconds) AS UNSIGNED) AS sumTotalWorkstationSeconds')
        ->addSelect('b.status AS batchStatus')
        ->join('wor.workOrder', 'wo')
        ->join('wo.batch', 'b')
        ->groupBy('b.code')
    ;
}
// BatchAnalysisDataTableType

public function buildDataTable(DataTableBuilderInterface $builder, array $options): void
{
    $builder
        ->addColumn(
            'batchCode',
            TextColumnType::class,
            [
                'label' => 'batchCode',
                'property_path' => '[batchCode]',
                'sort' => 'batchCode',
                'export' => true,
            ]
        )
        ->addColumn(
            'batchStatus',
            TextColumnType::class,
            [
                'label' => 'batchStatus',
                'property_path' => '[batchStatus]',
                'sort' => 'batchStatus',
                'export' => true,
            ]
        )
        ->addColumn(
            'sumTotalWorkstationSeconds',
            TextColumnType::class,
            [
                'label' => 'sumTotalWorkstationSeconds',
                'property_path' => '[sumTotalWorkstationSeconds]',
                'sort' => 'sumTotalWorkstationSeconds',
                'export' => true,
            ]
        )
        ->addFilter(
            'batchCode',
            StringFilterType::class,
            [
                'query_path' => 'b.code',
            ]
        )
        ->addFilter(
            'batchStatus',
            BatchStatusFilterType::class,
            [
                'query_path' => 'b.status',
            ]
        )
        ->addFilter(
            'totalWorkstationSeconds',
            CallbackFilterType::class,
            [
                'form_type' => NumberType::class,
                'callback' => function (DoctrineOrmProxyQuery $query, FilterData $data) {
                    $query
                        ->andHaving('CAST(SUM(wor.workstationSeconds) AS UNSIGNED) = :totalWorkstationSeconds')
                        ->setParameter('totalWorkstationSeconds', (int) $data->getValue())
                    ;
                },
            ]
        )
        ->addExporter('csv', CsvExporterType::class)
        ->addExporter('xlsx', XlsxExporterType::class)
        ->setDefaultSortingData(SortingData::fromArray([
            'batchCode' => strtolower(SortOrderEnum::ASC->value),
        ]))
        ->setDefaultPaginationData(new PaginationData(
            page: 1,
            perPage: 25,
        ))
    ;
}
davidromani commented 3 months ago

There are 3 filters defined inside BatchAnalysisDataTableType, batchStatus & batchCode are working well.

The problems arise when I insert a value inside totalWorkstationSeconds, if I filter by a value that doesn't match the result set is showing an empty table (so this case is right). But, if I filter by a value that match then de Doctrine runtime exception is thrown.

Kreyu commented 3 months ago

Hey @davidromani.

As the exception suggests, this is not supported by the Doctrine Paginator, used by the bundle. My previous suggestion of using callback filter with HAVING seems incompatible - sorry!

Now, this is no longer related with the bundle, but with Doctrine itself. This would require disabling output walkers, but that is not supported for queries with HAVING clause: https://github.com/doctrine/orm/issues/8278#issue-705517756 and that's what we're doing in the bundle (logic copied from SonataDoctrineOrmAdminBundle):

https://github.com/Kreyu/data-table-bundle/blob/7cadde7b0d4a6206aff89733cc90a3d4e24a2ee5/src/Bridge/Doctrine/Orm/Paginator/PaginatorFactory.php#L52-L60

Queries like that are typically handled using raw SQL queries (e.g. with Doctrine DBAL instead of an ORM), but that would be incompatible with the bundle (without creating custom proxy query and filters for DBAL). I would suggest some workaround like using a subquery with HAVING clause, instead of using it in main query. I suspect those workarounds will be slower (using an ORM in this case will be slower anyway), so please test them out first:

$builder->addFilter(
    'totalWorkstationSeconds',
    CallbackFilterType::class,
    [
        'callback' => function (DoctrineOrmProxyQuery $query, FilterData $data) {
            // QueryBuilder that selects only the batch code having SUM equal to the given value.
            // Please note that this is probably wrong (especially those joins) as I don't see your database schema.
            // But that should give you an idea on how to work around this issue.

            // Inject repository to the data table type, put it in separate repository method, or just keep here
            // Below I'll assume that we're injecting your BatchRepository
            $subQuery = $this->batchRepository->createQueryBuilder('_b')
                ->select('_b.code')
                // I think left joins are correct in this case?
                ->leftJoin('_b.workOrder', '_wo')
                ->leftJoin('_wo.workOrderResult', '_wor')
                // This is the most crucial part - here we are limiting the subquery to given criteria
                ->having('CAST(SUM(_wor.workstationSeconds) AS UNSIGNED) = :totalWorkstationSeconds')
                ->groupBy('_b.code')
            ;

            // why "_" in aliases instead of a simple "b", "wor", etc.?
            // because Doctrine would throw an exception that alias already exists in the query
            // so we need to use something different

            $query
                // ...and here we are filtering by identifiers present in the subquery ("code" column in this case?)
                ->andWhere(sprintf('b.code IN (%s)', $subQuery->getDQL()))
                ->setParameter('totalWorkstationSeconds', (int) $data->getValue())
            ;
        }
    ]
)
Kreyu commented 3 months ago

For reference, I'll include a code snippet I tested in one of my projects. I have a Document entity that has many DocumentFile. I assume that we wan't to create a simple data table with document ID and count of related document files, so that makes it somewhat similar to your case:

// The query builder used for the data table - returns an array with "id" and "documentFilesCount"
$this->entityManager->createQueryBuilder()
    ->select('document.id AS id')
    ->addSelect('COUNT(document.id) AS documentFilesCount')
    ->from(Document::class, 'document')
    ->groupBy('document.id')
;

// The filter definition on column that uses COUNT
$builder->addFilter(
    'documentFilesCount',
    CallbackFilterType::class,
    [
        'callback' => function (DoctrineOrmProxyQuery $query, FilterData $data) {
            $aggregateQuery = $this->entityManager->createQueryBuilder()
                ->select('documentAggregate.id')
                ->from(Document::class, 'documentAggregate')
                ->leftJoin('document.documentFiles', 'documentFileAggregate')
                ->having('COUNT(documentFileAggregate.id) = :documentFilesCount')
                ->groupBy('documentAggregate.id')
            ;

            $query
                ->andWhere(sprintf('document.id IN (%s)', $aggregateQuery->getDQL()))
                ->setParameter('documentFilesCount', (int) $data->getValue())
            ;
        }
    ]
)

However, I understand that this is a lot of work for a simple use-case, and maybe there's better ways to handle that, but here we're forcing the ORM to work instead of making a whole integration with DBAL instead - and unless you're planning to do it in every data table, I think it's better this way.

davidromani commented 3 months ago

@Kreyu many thanks again!

Based on your notes, this solve the problem:

// BatchAnalysisDataTableType

public function buildDataTable(DataTableBuilderInterface $builder, array $options): void
{
    $builder
    // ...
        ->addFilter(
            'sumTotalWorkstationSeconds',
            SumTotalWorkstationSecondsFilterType::class,
            [
                'query_path' => 'wor.sumTotalWorkstationSeconds',
            ]
        )
    // ...  
// SumTotalWorkstationSecondsFilterType

<?php

namespace App\Datatable\Filter;

use App\Repository\Production\BatchRepository;
use Kreyu\Bundle\DataTableBundle\Filter\FilterData;
use Kreyu\Bundle\DataTableBundle\Filter\FilterHandlerInterface;
use Kreyu\Bundle\DataTableBundle\Filter\FilterInterface;
use Kreyu\Bundle\DataTableBundle\Filter\FilterBuilderInterface;
use Kreyu\Bundle\DataTableBundle\Filter\Type\AbstractFilterType;
use Kreyu\Bundle\DataTableBundle\Query\ProxyQueryInterface;
use Symfony\Component\Form\Extension\Core\Type\NumberType;
use Symfony\Component\OptionsResolver\OptionsResolver;

class SumTotalWorkstationSecondsFilterType extends AbstractFilterType implements FilterHandlerInterface
{
    public function __construct(private readonly BatchRepository $br) {}

    public function buildFilter(FilterBuilderInterface $builder, array $options): void
    {
        $builder->setHandler($this);
    }

    public function configureOptions(OptionsResolver $resolver): void
    {
        $resolver->setDefaults([
            'form_type' => NumberType::class,
        ]);
    }

    public function handle(ProxyQueryInterface $query, FilterData $data, FilterInterface $filter): void
    {
        $subQuery = $this->br->createQueryBuilder('_b')
            ->select('_b.code')
            ->leftJoin('_b.workOrders', '_wo')
            ->leftJoin('_wo.workOrderResult', '_wor')
            ->having('CAST(SUM(_wor.workstationSeconds) AS UNSIGNED) = :totalWorkstationSeconds')
            ->groupBy('_b.code')
        ;
        $query
            ->andWhere(sprintf('b.code IN (%s)', $subQuery->getDQL()))
            ->setParameter('totalWorkstationSeconds', (int) $data->getValue())
        ;
    }
}

Finally, I've decided to avoid the CallbackFilterType in favor of a custom SumTotalWorkstationSecondsFilterType (just to keep code thinner).