[Paginator] Fix regression when using Custom ID type

[ostrolucky]

Continuation of #7890. Created new PR to clean the mess with huge diff, though I still left attributions via co-authors.

I have reused previous code from @Ocramius for getting id type, however if preferred, I can replace it with this smaller piece of code:

$AST           = $subQuery->getAST();
$entityManager = $subQuery->getEntityManager();
$connection    = $this->query->getEntityManager()->getConnection();
$classMetadata = $entityManager->getClassMetadata(
    $AST->fromClause->identificationVariableDeclarations[0]->rangeVariableDeclaration->abstractSchemaName
);

$identifierType = PersisterHelper::getTypeOfField(
    $classMetadata->getSingleIdentifierFieldName(),
    $classMetadata,
    $entityManager
)[0];

$ids =  array_map(static function(array $row) use ($identifierType, $connection) {
    return current($row);
    return $connection->convertToPHPValue(current($row), $identifierType);
}, $foundIdRows);

Please check if this approach is right one, as it's weird that we are now doing PHP<>DB conversions in both Paginator and WhereInWalker

[lcobucci]

@ostrolucky this is basically what @Ocramius had in 39d2113 and then moved to the walker in c67a515. Which basically invalidades the changes in WhereInWalker...

I'd still prefer to keep things contained in the WhereInWalker as much as possible, have you found the root cause of why the tests fail when using custom query walkers?

[ostrolucky]

It's not same. In 39d2113 was convertToDatabaseValue call happening in Paginator. It was moved in c67a515 into WhereInWalker. What I've done here is to call convertToPHPValue in Paginator. This is a reverse call. Without that, WhereInWalker will have access to ids already in database values only. That said, I don't know how much this bug has to do with custom walkers, but it's the only failing testcase we received.

[lcobucci]

@ostrolucky you're right. However, I think this should have been solved by the hydrator already. I'm debugging it now.

[ostrolucky]

Thanks. Initially I wanted to just call getResult() in Paginator, but that didn't turn out to be easy.

[lcobucci]

@ostrolucky it's related to this wonderful piece of code:

// WARNING: BC break! We know this is the desired behavior to type convert values, but this
// erroneous behavior exists since 2.0 and we're forced to keep compatibility.

The LimitSubqueryWalker creates a field which isn't mapped, causing this mess.

[lcobucci]

Yeah, I managed to solve this only by addressing the issue in AbstractHydrator. Didn't do it pretty because I wanted to validate my thoughts...

I believe we should ensure that things are properly hydrated in the hydrator instead of pulling this responsibility to the paginator. Do you have any concerns @Ocramius @guilhermeblanco?

[ostrolucky]

I like that idea

[akorz]

@ostrolucky Thank you for help :)

[ostrolucky]

I have simplified the test case more. It doesn't have anything to do with custom walkers indeed. @lcobucci can we see your patch?

[lcobucci]

Superseded by #7905