Database::makeList() : Handle NULL when building 'IN' clause

Amends to Database::makeList() to check if 'NULL' is present in array while
building IN clause. Presently a NULL in array intended to build IN clause, is
treated as a usual value. This creates
problem:https://dev.mysql.com/doc/refman/5.0/en/comparison-operators.html#function_in
Therefore null is appended at the end of the IN clause as " $field IS NULL"
rather than within the IN clause.
DatabaseSQLTest.php: Some test cases added for the modified behaviour.

Bug: T50853
Change-Id: Ic255c37e9702fd44d8173027ab8036ddc48f9d2e

diff --git a/RELEASE-NOTES-1.25 b/RELEASE-NOTES-1.25
index 4a6ee94..4e195c5 100644 (file)
--- a/RELEASE-NOTES-1.25
+++ b/RELEASE-NOTES-1.25
@@ -118,6 +118,7 @@ production.
 * The Special:WhatLinksHere page linked from 'Number of redirects to this page'
   on action=info about a file page does not list file links anymore.
 * (T78637) Search bar is not autofocused unless it is empty so that proper scrolling using arrow keys is possible.
+* (T50853) Database::makeList() modified to handle 'NULL' separately when building IN clause
 
 === Action API changes in 1.25 ===
 * (T67403) XML tag highlighting is now only performed for formats

diff --git a/includes/db/Database.php b/includes/db/Database.php
index 7938ce6..0b022d1 100644 (file)
--- a/includes/db/Database.php
+++ b/includes/db/Database.php
@@ -2160,16 +2160,36 @@ abstract class DatabaseBase implements IDatabase {
                        } elseif ( ( $mode == LIST_SET ) && is_numeric( $field ) ) {
                                $list .= "$value";
                        } elseif ( ( $mode == LIST_AND || $mode == LIST_OR ) && is_array( $value ) ) {
-                               if ( count( $value ) == 0 ) {
+                               // Remove null from array to be handled separately if found
+                               $includeNull = false;
+                               foreach ( array_keys( $value, null, true ) as $nullKey ) {
+                                       $includeNull = true;
+                                       unset( $value[$nullKey] );
+                               }
+                               if ( count( $value ) == 0 && !$includeNull ) {
                                        throw new MWException( __METHOD__ . ": empty input for field $field" );
-                               } elseif ( count( $value ) == 1 ) {
-                                       // Special-case single values, as IN isn't terribly efficient
-                                       // Don't necessarily assume the single key is 0; we don't
-                                       // enforce linear numeric ordering on other arrays here.
-                                       $value = array_values( $value );
-                                       $list .= $field . " = " . $this->addQuotes( $value[0] );
+                               } elseif ( count( $value ) == 0 ) {
+                                       // only check if $field is null
+                                       $list .= "$field IS NULL";
                                } else {
-                                       $list .= $field . " IN (" . $this->makeList( $value ) . ") ";
+                                       // IN clause contains at least one valid element
+                                       if ( $includeNull ) {
+                                               // Group subconditions to ensure correct precedence
+                                               $list .= '(';
+                                       }
+                                       if ( count( $value ) == 1 ) {
+                                               // Special-case single values, as IN isn't terribly efficient
+                                               // Don't necessarily assume the single key is 0; we don't
+                                               // enforce linear numeric ordering on other arrays here.
+                                               $value = array_values( $value );
+                                               $list .= $field . " = " . $this->addQuotes( $value[0] );
+                                       } else {
+                                               $list .= $field . " IN (" . $this->makeList( $value ) . ") ";
+                                       }
+                                       // if null present in array, append IS NULL
+                                       if ( $includeNull ) {
+                                               $list .= " OR $field IS NULL)";
+                                       }
                                }
                        } elseif ( $value === null ) {
                                if ( $mode == LIST_AND || $mode == LIST_OR ) {

diff --git a/tests/phpunit/includes/db/DatabaseSQLTest.php b/tests/phpunit/includes/db/DatabaseSQLTest.php
index 9d4765d..b13751f 100644 (file)
--- a/tests/phpunit/includes/db/DatabaseSQLTest.php
+++ b/tests/phpunit/includes/db/DatabaseSQLTest.php
@@ -755,6 +755,16 @@ class DatabaseSQLTest extends MediaWikiTestCase {
                                LIST_AND,
                                "field IS NULL AND (field2 != 'value2')"
                        ),
+                       array(
+                               array( 'field' => array( 'value', null, 'value2' ), 'field2' => 'value2' ),
+                               LIST_AND,
+                               "(field IN ('value','value2')  OR field IS NULL) AND field2 = 'value2'"
+                       ),
+                       array(
+                               array( 'field' => array( null ), 'field2' => null ),
+                               LIST_AND,
+                               "field IS NULL AND field2 IS NULL"
+                       ),
                        array(
                                array( 'field' => 'value', 'field2' => 'value2' ),
                                LIST_OR,
@@ -770,6 +780,11 @@ class DatabaseSQLTest extends MediaWikiTestCase {
                                LIST_OR,
                                "field IN ('value','value2')  OR field2 = 'value'"
                        ),
+                       array(
+                               array( 'field' => array( null, 'value', null, 'value2' ), "field2 != 'value2'" ),
+                               LIST_OR,
+                               "(field IN ('value','value2')  OR field IS NULL) OR (field2 != 'value2')"
+                       ),
                        array(
                                array( 'field' => 'value', 'field2' => 'value2' ),
                                LIST_SET,