Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

[Test failing...] DDC-1048 reported by cordoval #647

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
7 participants

Hello,

thank you for creating this pull request. I have automatically opened an issue
on our Jira Bug Tracker for you. See the issue link:

http://www.doctrine-project.org/jira/browse/DDC-2397

We use Jira to track the state of pull requests and the versions they got
included in.

Contributor

Padam87 commented Apr 10, 2013

👍

Ran into this issue with one of my bundles, fortunately I only needed it for the query builder, and made it work without creating an expression.

I believe the cause of the problem is this:
https://github.com/doctrine/doctrine2/blob/master/lib/Doctrine/ORM/Query/Expr/Comparison.php#L98

Member

Majkl578 commented Apr 10, 2013

This is a DQL issue (or DQL builder issue, to be specific), not a mapping or MySQL-specific issue. I think @Padam87 is correct, $rightExpr requires some inspection and I'm afraid that the problem affects also other Expr classes, not only Comparison.
The question is whether it's a bug or not, since DQL is a string and TRUE/FALSE is also a string in DQL.
Anyway, there should be two simple workarounds for this (in fact, second one is not a workaround, but a regular usage):
a) Use string representation:

$qb->expr()->eq('x.booleanField', 'FALSE');

or use parameter:

$qb->expr()->eq('x.booleanField', ':foo');
$qb->setParameter('foo', FALSE);

@Majkl578 The second one does not work for me, it works only if false is a string and not a boolean :

$qb->expr()->eq('x.booleanField', ':foo');
$qb->setParameter('foo', 'false');
Member

Majkl578 commented Apr 11, 2013

This should work then:

$qb->setParameter('foo', FALSE, \Doctrine\DBAL\Types\Type::BOOLEAN);

@stof stof commented on the diff Apr 11, 2013

.../Doctrine/Tests/ORM/Functional/Ticket/DDC1048Test.php
+ $false = new BooleanModel();
+ $false->booleanField = false;
+
+ $this->_em->persist($true);
+ $this->_em->persist($false);
+ $this->_em->flush();
+ $this->_em->clear();
+
+ $qb = $this->_em->createQueryBuilder()
+ ->select('x')
+ ->from('Doctrine\Tests\Models\Generic\BooleanModel', 'x')
+ ;
+
+ $false = $qb
+ ->where($qb->expr()->andX(
+ $qb->expr()->eq('x.booleanField', false),
@stof

stof Apr 11, 2013

Member

@cordoval This is using string concatenation. It is expected to work this way. You should use parameters instead

@Majkl578

Majkl578 Apr 11, 2013

Member

The thing is, DQL supports booleans without parameters so one would expect this to work. This behavior is at least strange and unexpected.

@stof

stof Apr 11, 2013

Member

@Majkl578 But it supports it by using a string, not a boolean.

@Majkl578

Majkl578 Apr 11, 2013

Member

I know, but that doesn't mean DQL builder can't use some extra logic for converting input to string representation (which would make sense here). Also, looking at Comparison::__construct, it says @param mixed $rightExpr, not @param string $rightExpr.

@stof

stof Apr 11, 2013

Member

yeah, because it could be another Expr

@Ocramius

Ocramius Apr 12, 2013

Owner

The query builder is a string builder and has nothing to do with casting constant values to their corresponding DQL representations. here, binding a string would mean using '"false"' and binding a boolean would require to use 'false', and not just the boolean false. these are all different

Owner

Ocramius commented Apr 12, 2013

Invalid

@Ocramius Ocramius closed this Apr 12, 2013

Contributor

cordoval commented Apr 12, 2013

i tried 👶

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment