Fix bug lp:809266

Analysis:
This is a bug in MWL#68, where it was incorrectly assumed
that if there is a match in the only non-null key, then
if there is a covering NULL row on all remaining NULL-able
columns there is a partial match. However, this is not the case,
because even if there is such a null-only sub-row, it is not
guaranteed to be part of the matched sub-row. The matched sub-row
and the NULL-only sub-row may be parts of different rows.

In fact there are two cases:
- there is a complete row with only NULL values, and
- all nullable columns contain only NULL values.

These two cases were incorrectly mixed up in the class member
  subselect_partial_match_engine::covering_null_row_width.


Solution:
The solution is to:
- split covering_null_row_width into two members:
  has_covering_null_row, and has_covering_null_columns, and
- take into account each state during initialization and
  execution.

mysql-test/r/subselect_partial_match.result

@@ -99,4 +99,50 @@ FROM t3 LEFT JOIN t2 ON t3.a = t2.a);
 d
 r
 drop table t1, t2, t3;
+#
+# LP BUG#809266 Diverging results with partial_match_rowid_merge=on
+#
+CREATE TABLE t1 (c int) ;
+INSERT INTO t1 VALUES (0),(0);
+CREATE TABLE t2 (a int, b int) ;
+INSERT INTO t2 VALUES (6,3), (9,NULL);
+set @@optimizer_switch='materialization=on,partial_match_rowid_merge=on,partial_match_table_scan=off,in_to_exists=off';
+EXPLAIN
+SELECT * FROM t1 WHERE (6, 4 ) NOT IN (SELECT b, a FROM t2);
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	PRIMARY	t1	ALL	NULL	NULL	NULL	NULL	2	
+2	SUBQUERY	t2	ALL	NULL	NULL	NULL	NULL	2	
+SELECT * FROM t1 WHERE (6, 4 ) NOT IN (SELECT b, a FROM t2);
+c
+0
+0
+EXPLAIN
+SELECT * FROM t1 WHERE (6, 4 ) NOT IN (SELECT a, b FROM t2);
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	PRIMARY	t1	ALL	NULL	NULL	NULL	NULL	2	
+2	SUBQUERY	t2	ALL	NULL	NULL	NULL	NULL	2	
+SELECT * FROM t1 WHERE (6, 4 ) NOT IN (SELECT a, b FROM t2);
+c
+0
+0
+set @@optimizer_switch='materialization=off,in_to_exists=on';
+EXPLAIN
+SELECT * FROM t1 WHERE (6, 4 ) NOT IN (SELECT b, a FROM t2);
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	PRIMARY	t1	ALL	NULL	NULL	NULL	NULL	2	
+2	DEPENDENT SUBQUERY	t2	ALL	NULL	NULL	NULL	NULL	2	Using where
+SELECT * FROM t1 WHERE (6, 4 ) NOT IN (SELECT b, a FROM t2);
+c
+0
+0
+EXPLAIN
+SELECT * FROM t1 WHERE (6, 4 ) NOT IN (SELECT a, b FROM t2);
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	PRIMARY	t1	ALL	NULL	NULL	NULL	NULL	2	
+2	DEPENDENT SUBQUERY	t2	ALL	NULL	NULL	NULL	NULL	2	Using where
+SELECT * FROM t1 WHERE (6, 4 ) NOT IN (SELECT a, b FROM t2);
+c
+0
+0
+drop table t1, t2;
 set @@optimizer_switch=@save_optimizer_switch;

mysql-test/t/subselect_partial_match.test

@@ -109,4 +109,36 @@ WHERE (t1.d , t1.d) NOT IN (
 
 drop table t1, t2, t3;
 
+--echo #
+--echo # LP BUG#809266 Diverging results with partial_match_rowid_merge=on
+--echo #
+
+CREATE TABLE t1 (c int) ;
+INSERT INTO t1 VALUES (0),(0);
+
+CREATE TABLE t2 (a int, b int) ;
+INSERT INTO t2 VALUES (6,3), (9,NULL);
+
+set @@optimizer_switch='materialization=on,partial_match_rowid_merge=on,partial_match_table_scan=off,in_to_exists=off';
+
+EXPLAIN
+SELECT * FROM t1 WHERE (6, 4 ) NOT IN (SELECT b, a FROM t2);
+SELECT * FROM t1 WHERE (6, 4 ) NOT IN (SELECT b, a FROM t2);
+
+EXPLAIN
+SELECT * FROM t1 WHERE (6, 4 ) NOT IN (SELECT a, b FROM t2);
+SELECT * FROM t1 WHERE (6, 4 ) NOT IN (SELECT a, b FROM t2);
+
+set @@optimizer_switch='materialization=off,in_to_exists=on';
+
+EXPLAIN
+SELECT * FROM t1 WHERE (6, 4 ) NOT IN (SELECT b, a FROM t2);
+SELECT * FROM t1 WHERE (6, 4 ) NOT IN (SELECT b, a FROM t2);
+
+EXPLAIN
+SELECT * FROM t1 WHERE (6, 4 ) NOT IN (SELECT a, b FROM t2);
+SELECT * FROM t1 WHERE (6, 4 ) NOT IN (SELECT a, b FROM t2);
+
+drop table t1, t2;
+
 set @@optimizer_switch=@save_optimizer_switch;

sql/item_subselect.cc

@@ -4572,29 +4572,36 @@ int subselect_hash_sj_engine::exec()
   if (strategy == PARTIAL_MATCH)
   {
     uint count_pm_keys; /* Total number of keys needed for partial matching. */
-    MY_BITMAP *nn_key_parts; /* The key parts of the only non-NULL index. */
-    uint covering_null_row_width;
+    MY_BITMAP *nn_key_parts= NULL; /* Key parts of the only non-NULL index. */
+    uint count_non_null_columns= 0; /* Number of columns in nn_key_parts. */
+    bool has_covering_null_row;
+    bool has_covering_null_columns;
     select_materialize_with_stats *result_sink=
       (select_materialize_with_stats *) result;
+    uint field_count= tmp_table->s->fields;
 
-    nn_key_parts= (count_partial_match_columns < tmp_table->s->fields) ?
-                  &non_null_key_parts : NULL;
-
-    if (result_sink->get_max_nulls_in_row() ==
-        tmp_table->s->fields -
-        (nn_key_parts ? bitmap_bits_set(nn_key_parts) : 0))
-      covering_null_row_width= result_sink->get_max_nulls_in_row();
-    else
-      covering_null_row_width= 0;
+    if (count_partial_match_columns < field_count)
+    {
+      nn_key_parts= &non_null_key_parts;
+      count_non_null_columns= bitmap_bits_set(nn_key_parts);
+    }
+    has_covering_null_row= (result_sink->get_max_nulls_in_row() == field_count);
+    has_covering_null_columns= (count_non_null_columns +
+                                count_null_only_columns == field_count);
 
-    if (covering_null_row_width)
-      count_pm_keys= nn_key_parts ? 1 : 0;
+    if (has_covering_null_row)
+    {
+      DBUG_ASSERT(count_partial_match_columns = field_count);
+      count_pm_keys= 0;
+    }
+    else if (has_covering_null_columns)
+      count_pm_keys= 1;
     else
       count_pm_keys= count_partial_match_columns - count_null_only_columns +
-        (nn_key_parts ? 1 : 0);
+                     (nn_key_parts ? 1 : 0);
 
     choose_partial_match_strategy(test(nn_key_parts),
-                                  test(covering_null_row_width),
+                                  has_covering_null_row,
                                   &partial_match_key_parts);
     DBUG_ASSERT(strategy == PARTIAL_MATCH_MERGE ||
                 strategy == PARTIAL_MATCH_SCAN);
@@ -4604,7 +4611,8 @@ int subselect_hash_sj_engine::exec()
         new subselect_rowid_merge_engine(thd, (subselect_uniquesubquery_engine*)
                                          lookup_engine, tmp_table,
                                          count_pm_keys,
-                                         covering_null_row_width,
+                                         has_covering_null_row,
+                                         has_covering_null_columns,
                                          item, result,
                                          semi_join_conds->argument_list());
       if (!pm_engine ||
@@ -4629,7 +4637,8 @@ int subselect_hash_sj_engine::exec()
                                             lookup_engine, tmp_table,
                                             item, result,
                                             semi_join_conds->argument_list(),
-                                            covering_null_row_width)))
+                                            has_covering_null_row,
+                                            has_covering_null_columns)))
       {
         /* This is an irrecoverable error. */
         res= 1;
@@ -5065,11 +5074,13 @@ subselect_partial_match_engine::subselect_partial_match_engine(
   TABLE *tmp_table_arg, Item_subselect *item_arg,
   select_result_interceptor *result_arg,
   List<Item> *equi_join_conds_arg,
-  uint covering_null_row_width_arg)
+  bool has_covering_null_row_arg,
+  bool has_covering_null_columns_arg)
   :subselect_engine(thd_arg, item_arg, result_arg),
    tmp_table(tmp_table_arg), lookup_engine(engine_arg),
    equi_join_conds(equi_join_conds_arg),
-   covering_null_row_width(covering_null_row_width_arg)
+   has_covering_null_row(has_covering_null_row_arg),
+   has_covering_null_columns(has_covering_null_columns_arg)
 {}
 
 
@@ -5107,7 +5118,7 @@ int subselect_partial_match_engine::exec()
     }
   }
 
-  if (covering_null_row_width == tmp_table->s->fields)
+  if (has_covering_null_row)
   {
     /*
       If there is a NULL-only row that coveres all columns the result of IN
@@ -5187,19 +5198,25 @@ subselect_rowid_merge_engine::init(MY_BITMAP *non_null_key_parts,
   rownum_t cur_rownum= 0;
   select_materialize_with_stats *result_sink=
     (select_materialize_with_stats *) result;
-  uint cur_keyid;
+  uint cur_keyid= 0;
   Item_in_subselect *item_in= (Item_in_subselect*) item;
   int error;
 
   if (merge_keys_count == 0)
   {
+    DBUG_ASSERT(bitmap_bits_set(partial_match_key_parts) == 0 ||
+                has_covering_null_row);
     /* There is nothing to initialize, we will only do regular lookups. */
     return FALSE;
   }
 
-  DBUG_ASSERT(!covering_null_row_width || (covering_null_row_width &&
-                                           merge_keys_count == 1 &&
-                                           non_null_key_parts));
+  /*
+    If all nullable columns contain only NULLs, there must be one index
+    over all non-null columns.
+  */
+  DBUG_ASSERT(!has_covering_null_columns ||
+              (has_covering_null_columns &&
+               merge_keys_count == 1 && non_null_key_parts));
   /*
     Allocate buffers to hold the merged keys and the mapping between rowids and
     row numbers.
@@ -5213,23 +5230,20 @@ subselect_rowid_merge_engine::init(MY_BITMAP *non_null_key_parts,
   /* Create the only non-NULL key if there is any. */
   if (non_null_key_parts)
   {
-    non_null_key= new Ordered_key(0, tmp_table, item_in->left_expr,
+    non_null_key= new Ordered_key(cur_keyid, tmp_table, item_in->left_expr,
                                   0, 0, 0, row_num_to_rowid);
     if (non_null_key->init(non_null_key_parts))
       return TRUE;
-    merge_keys[0]= non_null_key;
-    merge_keys[0]->first();
-    cur_keyid= 1;
+    merge_keys[cur_keyid]= non_null_key;
+    merge_keys[cur_keyid]->first();
+    ++cur_keyid;
   }
-  else
-    cur_keyid= 0;
 
   /*
-    If there is a covering NULL row, the only key that is needed is the
-    only non-NULL key that is already created above. We create keys on
-    NULL-able columns only if there is no covering NULL row.
+    If all nullable columns contain NULLs, the only key that is needed is the
+    only non-NULL key that is already created above.
   */
-  if (!covering_null_row_width)
+  if (!has_covering_null_columns)
   {
     if (bitmap_init_memroot(&matching_keys, merge_keys_count, thd->mem_root) ||
         bitmap_init_memroot(&matching_outer_cols, merge_keys_count, thd->mem_root))
@@ -5460,11 +5474,10 @@ bool subselect_rowid_merge_engine::partial_match()
   }
 
   /*
-    If there is a NULL (sub)row that covers all NULL-able columns,
-    then there is a guranteed partial match, and we don't need to search
-    for the matching row.
-   */
-  if (covering_null_row_width)
+    If all nullable columns contain only NULLs, then there is a guranteed
+    partial match, and we don't need to search for a matching row.
+  */
+  if (has_covering_null_columns)
   {
     res= TRUE;
     goto end;
@@ -5581,10 +5594,12 @@ subselect_table_scan_engine::subselect_table_scan_engine(
   Item_subselect *item_arg,
   select_result_interceptor *result_arg,
   List<Item> *equi_join_conds_arg,
-  uint covering_null_row_width_arg)
+  bool has_covering_null_row_arg,
+  bool has_covering_null_columns_arg)
   :subselect_partial_match_engine(thd_arg, engine_arg, tmp_table_arg, item_arg,
                                   result_arg, equi_join_conds_arg,
-                                  covering_null_row_width_arg)
+                                  has_covering_null_row_arg,
+                                  has_covering_null_columns_arg)
 {}
 
 
@@ -5623,10 +5638,6 @@ bool subselect_table_scan_engine::partial_match()
 
   tmp_table->file->extra_opt(HA_EXTRA_CACHE,
                              current_thd->variables.read_buff_size);
-  /*
-  TIMOUR:
-  scan_table() also calls "table->null_row= 0;", why, do we need it?
-  */
   for (;;)
   {
     error= tmp_table->file->ha_rnd_next(tmp_table->record[0]);

sql/item_subselect.h

@@ -1131,11 +1131,18 @@ protected:
   /* A list of equalities between each pair of IN operands. */
   List<Item> *equi_join_conds;
   /*
-    If there is a row, such that all its NULL-able components are NULL, this
-    member is set to the number of covered columns. If there is no covering
-    row, then this is 0.
+    True if there is an all NULL row in tmp_table. If so, then if there is
+    no complete match, there is a guaranteed partial match.
   */
-  uint covering_null_row_width;
+  bool has_covering_null_row;
+
+  /*
+    True if all nullable columns of tmp_table consist of only NULL values.
+    If so, then if there is a match in the non-null columns, there is a
+    guaranteed partial match.
+  */
+  bool has_covering_null_columns;
+
 protected:
   virtual bool partial_match()= 0;
 public:
@@ -1144,7 +1151,8 @@ public:
                                  TABLE *tmp_table_arg, Item_subselect *item_arg,
                                  select_result_interceptor *result_arg,
                                  List<Item> *equi_join_conds_arg,
-                                 uint covering_null_row_width_arg);
+                                 bool has_covering_null_row_arg,
+                                 bool has_covering_null_columns_arg);
   int prepare() { return 0; }
   int exec();
   void fix_length_and_dec(Item_cache**) {}
@@ -1232,13 +1240,15 @@ public:
   subselect_rowid_merge_engine(THD *thd_arg,
                                subselect_uniquesubquery_engine *engine_arg,
                                TABLE *tmp_table_arg, uint merge_keys_count_arg,
-                               uint covering_null_row_width_arg,
+                               bool has_covering_null_row_arg,
+                               bool has_covering_null_columns_arg,
                                Item_subselect *item_arg,
                                select_result_interceptor *result_arg,
                                List<Item> *equi_join_conds_arg)
     :subselect_partial_match_engine(thd_arg, engine_arg, tmp_table_arg,
                                     item_arg, result_arg, equi_join_conds_arg,
-                                    covering_null_row_width_arg),
+                                    has_covering_null_row_arg,
+                                    has_covering_null_columns_arg),
     merge_keys_count(merge_keys_count_arg), non_null_key(NULL)
   {}
   ~subselect_rowid_merge_engine();
@@ -1258,7 +1268,8 @@ public:
                               TABLE *tmp_table_arg, Item_subselect *item_arg,
                               select_result_interceptor *result_arg,
                               List<Item> *equi_join_conds_arg,
-                              uint covering_null_row_width_arg);
+                              bool has_covering_null_row_arg,
+                              bool has_covering_null_columns_arg);
   void cleanup();
   virtual enum_engine_type engine_type() { return TABLE_SCAN_ENGINE; }
 };