QueryBuilder Update Query doesn't work if no alias is used.

[Hanmac]

Bug Report

Q	A
BC Break	yes/no
Version	2.12.2

Summary

If no alias is used, the Query Builder seems to have problems to build the query, or the lexer that is checking the query thinks it is invalid when it is not.

Current behavior

Currently it is crashing with an Exception, see below.

How to reproduce

having this QueryBuilder:

$qb = $em->createQueryBuilder();
$qb->update(User::class);
$qb->set('klassenstufe', 'klassenstufe+1');
$qb->where($qb->expr()->in('id', $users));
$qb->getQuery()->execute();

makes this Query:
UPDATE App\Entity\User\User SET klassenstufe = klassenstufe+1 WHERE id IN(188)

and causes this crash:

[Syntax Error] line 0, col 33: Error: Expected Doctrine\ORM\Query\Lexer::T_SET, got 'klassenstufe'
  at vendor/doctrine/orm/lib/Doctrine/ORM/Query/QueryException.php:32
  at Doctrine\ORM\Query\QueryException::syntaxError('line 0, col 33: Error: Expected Doctrine\\ORM\\Query\\Lexer::T_SET, got \'klassenstufe\'', object(QueryException))
     (vendor/doctrine/orm/lib/Doctrine/ORM/Query/Parser.php:509)
  at Doctrine\ORM\Query\Parser->syntaxError('Doctrine\\ORM\\Query\\Lexer::T_SET')
     (vendor/doctrine/orm/lib/Doctrine/ORM/Query/Parser.php:368)
  at Doctrine\ORM\Query\Parser->match(247)
     (vendor/doctrine/orm/lib/Doctrine/ORM/Query/Parser.php:1315)
  at Doctrine\ORM\Query\Parser->UpdateClause()
     (vendor/doctrine/orm/lib/Doctrine/ORM/Query/Parser.php:946)
  at Doctrine\ORM\Query\Parser->UpdateStatement()
 

But having this QueryBuilder:

$qb = $em->createQueryBuilder();
$qb->update(User::class, 'u');
$qb->set('u.klassenstufe', 'u.klassenstufe+1');
$qb->where($qb->expr()->in('u.id', $users));
$qb->getQuery()->execute();

makes this Query:
UPDATE App\Entity\User\User u SET u.klassenstufe = u.klassenstufe+1 WHERE u.id IN(188)

Causing it to work.

IF the alias is needed, then it shouldn't be an optional parameter for the update function.
Or it should be better documented.

i don't know if that is a BC or not, but i think it was working before.

Expected behavior

The Query to be executed.

[derrabus]

i think it was working before

Can you tell us in which version this piece of code worked?

[Hanmac]

i think it was working before

Can you tell us in which version this piece of code worked?

i'm not sure if that had worked or not, i would need to experiment.

But from the parameters of the QueryBuilder the alias isn't required.

[derrabus]

You can omit both parameters, but I don't think that it is intended that you specify the first parameter only. Since that would produce an invalid DQL anyway, I think we can throw an exception in that case. Do you want to open a PR?

[derrabus]

Oh, and this affects the delete() method as well, doesn't it?

[Hanmac]

You can omit both parameters, but I don't think that it is intended that you specify the first parameter only. Since that would produce an invalid DQL anyway, I think we can throw an exception in that case. Do you want to open a PR?

i checked the code, if you omit both parameters, than it does nothing. (for what ever reason that is possible)
Maybe if setting the form path manually with the add function?

the only possible way would be if the alias would be part of the update string. but that might be bad code.

[derrabus]

i checked the code, if you omit both parameters, than it does nothing. (for what ever reason that is possible) Maybe if setting the form path manually with the add function?

Correct, you would need to add the target class and alias by calling from() or add(), otherwise the query would be incomplete. And as you can tell from the signature of the from() method, omitting the alias is not intended.

the only possible way would be if the alias would be part of the update string. but that might be bad code.

You are correct. That would mean we cannot just throw an exception, but we should deprecate omitting the alias and turn the deprecation into an exception in 3.0.

[Hanmac]

i might not have enough understanding of Doctrine to write the possible PR myself.

like for example first question: should it still be possible to call update() without parameters and manually add the same thing with from() or add()?

for the second, i would just add a trigger warning for now that says that omitting the alias is deprecated.

[derrabus]

i might not have enough understanding of Doctrine to write the possible PR myself.

No worries, I can assist you during code review.

like for example first question: should it still be possible to call update() without parameters and manually add the same thing with from() or add()?

Yes.

for the second, i would just add a trigger warning for now that says that omitting the alias is deprecated.

Yes, please use this method for triggering the deprecation:

https://github.com/doctrine/deprecations/blob/0e2a4f1f8cdfc7a92ec3b01c9334898c806b30de/lib/Doctrine/Deprecations/Deprecation.php#L64-L73

Also, please document this change in UPGRADE.md. Please target the 2.13.x branch.

[derrabus]

Fixed via b931a59