Page MenuHomePhabricator

SQL: NULL in an IN-list should be written as OR
Closed, ResolvedPublic

Description

print_r($dbr->makeList(array('a'=>array(null,2)),LIST_OR));

a IN (NULL,'2')

Expected: a IS NULL OR a = '2'


Version: 1.22.0
Severity: normal

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 1:33 AM
bzimport set Reference to bz48853.
bzimport added a subscriber: Unknown Object (MLST).

Seen manuel ORs in core around rd_interwiki but there maybe exist other wheres, where this maybe can make the code easier.

(In reply to comment #1)

Seen manuel ORs in core around rd_interwiki but there maybe exist other
wheres,
where this maybe can make the code easier.

Correct! I reported this bug when working with rd_interwiki.

Hi I'd like to work on this bug! How do i begin??

Fixing this bug involves changing the function DatabaseBase::makeList(), which resides in includes/db/Database.php.

Fixing this bug involves changing the function DatabaseBase::makeList(), which resides in includes/db/Database.php.

Ok I read the makelist() function, I did not see any NULL in an IN list. I'm probably missing something?

Ok I read the makelist() function, I did not see any NULL in an IN list. I'm probably missing something?

Nope. That's actually the point!

Here's the idea: right now, i.e., the current functionality, if makeList() sees something that looks like the following:

$db->makeList( array( 'column' => array( 'value1', 'value2', null ) ) );

It doesn't treat null as special, which is a mistake. As noted in the bug report, it produces the SQL output column IN ('value1','value2', NULL). This is wrong, and will not act as expected. The MySQL manual notes that you should "never mix quoted and unquoted values" for reasons having to do with comparison rules. I am not sure whether the aforementioned SQL is incorrect because of this or because of special NULL semantics, but nonetheless it is wrong.

What we actually want is column IN ('value1', 'value2') OR column IS NULL. This is particularly interesting because of certain optimizations with IS NULL. In other words, we want makeList() to treat null as special.

Ok so basically we have to check if NULL is an element of the array 'a' and if a NULL is indeed present we must have a seperate case so that it removes the NULL from the IN output and makes it an OR output.

Ok I read the makelist() function, I did not see any NULL in an IN list. I'm probably missing something?

Nope. That's actually the point!

Here's the idea: right now, i.e., the current functionality, if makeList() sees something that looks like the following:

$db->makeList( array( 'column' => array( 'value1', 'value2', null ) ) );

It doesn't treat null as special, which is a mistake. As noted in the bug report, it produces the SQL output column IN ('value1','value2', NULL). This is wrong, and will not act as expected. The MySQL manual notes that you should "never mix quoted and unquoted values" for reasons having to do with comparison rules. I am not sure whether the aforementioned SQL is incorrect because of this or because of special NULL semantics, but nonetheless it is wrong.

What we actually want is column IN ('value1', 'value2') OR column IS NULL. This is particularly interesting because of certain optimizations with IS NULL. In other words, we want makeList() to treat null as special.

Can the null occur at any position in the array or only at the last position?

Change 181681 had a related patch set uploaded (by Sumit):
Database.php:Changed makeList() to treat 'NULL' separately when present in array while forming IN clause

https://gerrit.wikimedia.org/r/181681

Patch-For-Review

I had already started fixing this bug but i guess it has been fixed by someone else :/

Change 181681 merged by jenkins-bot:
Database::makeList() : Handle NULL when building 'IN' clause

https://gerrit.wikimedia.org/r/181681

Parent5446 assigned this task to Sumit.