Skip to content

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

Open
doctrinebot opened this Issue Apr 1, 2014 · 13 comments

2 participants

@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?

@Ocramius Ocramius was assigned by doctrinebot Dec 6, 2015
@doctrinebot doctrinebot added the Bug label Dec 7, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.