Function 'in' of class Expr does not work with an array of strings as parameter

You have to use literal values.

Example:

$qb->where($qb>expr()->in('ofr.state', array(
    $qb->expr()->literal('test'),
    $qb->expr()->literal('second value'),
    $qb->expr()->literal('third value')
)));

Marking ticket as invalid.

It doesnt make sense to me that

->where($qb->expr()->in('u.id', array(1, 2, 3)))

works and with strings it doesnt. Both are literals.

I guess you can use:

->where($qb>expr()->in('ofr.state', array("'test'","'second value'", "'third value'")));

Note the single quotes around the values. in DQL string literals must be quoted in single quotes.

doesn't the dql parser know what type of value this is and how it should be quoted? I am all for convenience for IN(), its a very important construct and often used

@Benjamin: If it's not in single-quotes, its not a string literal per definition of the grammar The DQL you get for:

->where($qb>expr()->in('ofr.state', array('test','second value','third value')));

is:

... WHERE ofr.state IN(test, second value, third value)

Thus the error:

Expected Literal, got 'test'

when it encounters test where a literal (a string in single quotes, an integer, ...) is expected.

I'm looking into a possible fix for this without affecting too much in our structure, since IN should also supports subqueries.

I think the best solution is to implement a Literal class, since it won't affect if user passes the construction as I sent.
A simple class check would solve it smoothly.

Working on a patch for it. Should be committed soon.

@Guilherme:

->where($qb>expr()->in('ofr.state', array("'test'","'second value'", "'third value'")));

is fine, dont you think? Doesnt that work? Dont make something complex if this works!

@romanb: It works, but it should NOT be done like that.

It should either be done automatically or make it strict as it's currently (called $qb->expr()->literal('foo')).

Since it's SO common this usage (array of values), I'm thinking to support auto-conversion natively. I always thought Literals SHOULD be a class... I even spoke with you about it when building the DQL parser. =\

Think carefully about that. I didnt do that in the parser for performance reasons.

Do you really want to wrap every single literal value in an object? ->in('a.bla', $arrayWith50Integers) = 50 objects?

I am seeing overkill.

So please, please consider this.

What you want to do is providing some minor convenience at the cost of an order of magnitude higher memory consumption and slower performance.

Just compare 50 ints vs 50 objects holding ints in a microbenchmark performance & memory-wise.

I am already regretting reopening this!

@romanb: Ok... so let's consider a quote comparison.

If it's already wrapped in quotes, skip the literal call. Otherwise, apply individually.
So.. what would perform better? What do you think? By doing this, it would require to iterate through all items and do a string check (something like str[0] == "'" && str[strlen(str) - 1] == "'" OR via a regex (which I think would not perform better, need to check)) and if it doesn't contain, apply the literal call.

If you have any better idea, feel free. =)

Ok, how about this:

    public function in($x, $y)
    {
        if (is_array($y)) {
            foreach ($y as &$literal) {
                $literal = $this->literal($literal);
            }
        }
        return new Expr\Func($x . ' IN', (array) $y);
    }


    public function literal($literal)
    {
        if (is_numeric($literal)) {
            return (string) $literal;
        } else {
            return "'" . str_replace("'", "''", $literal) . "'"; // quote ' in the literal
        }
    }

The literal fix is needed either way, I think, otherwise single quotes in strings are misinterpreted.

This works perfect for me in my local copy. Here is a test I wrote:

    public function testWhereInWithStringLiterals()
    {
        $qb = $this->_em->createQueryBuilder();
        $qb->select('u')
           ->from('Doctrine\Tests\Models\CMS\CmsUser', 'u')
           ->where($qb->expr()->in('u.name', array('one', 'two', 'three')));
           
        $this->assertValidQueryBuilder($qb, "SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE u.name IN('one', 'two', 'three')");
        
        $qb->where($qb->expr()->in('u.name', array("O'Reilly", "O'Neil", 'Smith')));
           
        $this->assertValidQueryBuilder($qb, "SELECT u FROM Doctrine\Tests\Models\CMS\CmsUser u WHERE u.name IN('O''Reilly', 'O''Neil', 'Smith')");
    }

And subqueries still work, too.

@romanb that would work, of course.

BUT... It would break BC in the case I do something like:

$qb->where($qb>expr()->in('ofr.state', array(
    $qb->expr()->literal('test'),
    $qb->expr()->literal('second value'),
    $qb->expr()->literal('third value')
)));

That would be the same as array("'test'", "'second value'", "'third value'"), which would double the quote and give even more headaches.
I think we can consider this BC break now... what do you think?

@romanb I think since we're in the last alpha, it's still acceptable that we break BC...

I disagree with you that it's not used frequently. I think the opposite! While we still do not support IN ?, we're still struggled in the expr IN support, which is the exactly issue we're facing here.

SO... We need to agree if we break BC or of we add overhead by adding a Literal class.
Which one you think it'll be the best one to be merged?

I'll merge one of them... just need some thoughts from you. =)

Cheers,