DDC-3063: Unexpected behavior with 'WHERE NOT IN' and empty array

[doctrinebot]

Jira issue originally created by user timstamp:

I tried to set version to 2.4.0 but was prevented.

Assume the set 'n' contains 10 records, all with id > 0.

->andWhere('n.id NOT IN (:ids)')->setParameter('ids', [])
returns 0 records.

->andWhere('n.id NOT IN (:ids)')->setParameter('ids', [0])
returns 10 records.

[doctrinebot]

Comment created by @Ocramius:

This is a simple misunderstanding of how SQL IN() works

[doctrinebot]

Comment created by timstamp:

You're right that its invalid SQL, but this is DQL. In SQL this would raise an error, in DQL it says nothing and returns nothing.

Isnt there a way to check if a statement has caused this error?

[doctrinebot]

Comment created by @Ocramius:

[~timstamp] yeah, when using IN() and NOT IN() you should actually check if the parameters are empty or not before adding the clause.

[doctrinebot]

Comment created by @deeky666:

It's still weird that no error is raised by that. [~timstamp] can you check what SQL is generated by your query?

[doctrinebot]

Comment created by @deeky666:

Hehe I think I got the problem. It might be DBAL related. See:

https://github.com/doctrine/dbal/blob/master/lib/Doctrine/DBAL/SQLParserUtils.php#L140
https://github.com/doctrine/dbal/blob/master/lib/Doctrine/DBAL/SQLParserUtils.php#L169

Looks like the implementing person did that by intention but don't ask me why.

[doctrinebot]

Comment created by @Ocramius:

Thats to avoid having an SQL statement like
SELECT * FROM foo WHERE bar IN(), which is invalid.

Indeed, the NOT IN() case is not contemplated by that.

[doctrinebot]

Comment created by @deeky666:

Just wondering if that kind of implementation isn't changing expectations because it looks like IN(NULL) will select all NULL rows then, even though this is not explicitly requested by the user. I don't care really TBH but this behaviour is not very transparent to the user. Personally I would like to see an error instead... Then at least I know what's going on and can fix that by additional checks instead.

[doctrinebot]

Comment created by @Ocramius:

Makes sense. Re-opening.

[doctrinebot]

Comment created by @deeky666:

The question is if we can change this behaviour without breaking applications that rely on the current one. Because changing the code to throw an error breaks applications that insist on returning 0 rows for an empty array. I don't know what rules apply here concerning BC. What do you think [~ocramius]?

[doctrinebot]

Comment created by @Ocramius:

[~deeky] existing apps should do following anyway:

if ($productIds) { $qb->andWhere('p.id IN (:productIds)')->setParameter('productIds', $productIds); }

If they are not, that's most likely a bug in their codebase.

[doctrinebot]

Comment created by @deeky666:

Agree. Was just wondering about what we can expect and what we can't expect.

[doctrinebot]

Comment created by @guilhermeblanco:

[~deeky666] feel free to patch DBAL.
It's a bug, not a supported feature. If we have 2 tickets:
1- If I pass an empty array, I get nullable rows and I can't fix this
2- If I pass an empty array, it used to return nullable rows, now it returns nothing
Which one would you fix?

[doctrinebot]

Comment created by timstamp:

[~guilhermeblanco] shouldn't both these use cases throw an exception, as both cases in MySQL would return an error?

[jackdpeterson]

See PR 7108.
#7108

[driescroons]

Still a problem that i'm continuously bumping into. Ugly solutions do fix it but are only temporary.

[tomtomklima]

Hit the same cause. When calling NOT IN DQL quietly returns zero rows, on the other hand SQL returns Exception. Must use if check for empty array :/

[KhanMaytok]

Just check that an empty array is not used, or initialize it with an element, for example, a zero

[Ocramius]

@KhanMaytok I suggest helping @jackdpeterson with finishing up their fix.

[I-Valchev]

+1