Bug#27573: MIN() on an indexed column which is always NULL sets _other_ results

to NULL

For queries of the form SELECT MIN(key_part_k) FROM t1 
WHERE key_part_1 = const and ... and key_part_k-1 = const,
the opt_sum_query optimization tries to
use an index to substitute MIN/MAX functions with their values according
to the following rules:
1) Insert the minimum non-null values where the WHERE clause still matches, or
3) A row of nulls

However, the correct semantics requires that there is a third case 2)
such that a NULL value is substituted if there are only NULL values for 
key_part_k.

The patch modifies opt_sum_query() to handle this missing case.


mysql-test/r/func_group.result:
  Bug #27573: Correct result
mysql-test/t/func_group.test:
  Bug #27573: test case
sql/opt_sum.cc:
  Bug #27573:
  Added code that will try to read the
  first non-null value for a given complete-field prefix, second
  choice is to read the null, and lastly set the error code if no row
  is found.

1 files changed, 76 insertions, 10 deletions

diff --git a/sql/opt_sum.cc b/sql/opt_sum.cc
index 9222e15ff91..b9de54dbf5c 100644
--- a/sql/opt_sum.cc
+++ b/sql/opt_sum.cc
@@ -206,12 +206,68 @@ int opt_sum_query(TABLE_LIST *tables, List<Item> &all_fields,COND *conds)
 
           if (!ref.key_length)
             error= table->file->index_first(table->record[0]);
-          else
-	    error= table->file->index_read(table->record[0],key_buff,
-					   ref.key_length,
-					   range_fl & NEAR_MIN ?
-					   HA_READ_AFTER_KEY :
-					   HA_READ_KEY_OR_NEXT);
+          else 
+          {
+            /*
+              Use index to replace MIN/MAX functions with their values
+              according to the following rules:
+           
+              1) Insert the minimum non-null values where the WHERE clause still
+                 matches, or
+              2) a NULL value if there are only NULL values for key_part_k.
+              3) Fail, producing a row of nulls
+
+              Implementation: Read the smallest value using the search key. If
+              the interval is open, read the next value after the search
+              key. If read fails, and we're looking for a MIN() value for a
+              nullable column, test if there is an exact match for the key.
+            */
+            if (!(range_fl & NEAR_MIN))
+              /* 
+                 Closed interval: Either The MIN argument is non-nullable, or
+                 we have a >= predicate for the MIN argument.
+              */
+              error= table->file->index_read(table->record[0], ref.key_buff,
+                                             ref.key_length, 
+                                             HA_READ_KEY_OR_NEXT);
+            else
+            {
+              /*
+                Open interval: There are two cases:
+                1) We have only MIN() and the argument column is nullable, or
+                2) there is a > predicate on it, nullability is irrelevant.
+                We need to scan the next bigger record first.
+              */
+              error= table->file->index_read(table->record[0], ref.key_buff, 
+                                             ref.key_length, HA_READ_AFTER_KEY);
+              /* 
+                 If the found record is outside the group formed by the search
+                 prefix, or there is no such record at all, check if all
+                 records in that group have NULL in the MIN argument
+                 column. If that is the case return that NULL.
+
+                 Check if case 1 from above holds. If it does, we should read
+                 the skipped tuple.
+              */
+              if (ref.key_buff[prefix_len] == 1 && 
+                  /* 
+                     Last keypart (i.e. the argument to MIN) is set to NULL by
+                     find_key_for_maxmin only if all other keyparts are bound
+                     to constants in a conjunction of equalities. Hence, we
+                     can detect this by checking only if the last keypart is
+                     NULL.
+                  */                     
+                  (error == HA_ERR_KEY_NOT_FOUND ||
+                   key_cmp_if_same(table, ref.key_buff, ref.key, prefix_len)))
+              {
+                DBUG_ASSERT(item_field->field->real_maybe_null());
+                error= table->file->index_read(table->record[0], ref.key_buff,
+                                               ref.key_length, 
+                                               HA_READ_KEY_EXACT);
+              }
+            }
+          }
+          /* Verify that the read tuple indeed matches the search key */
 	  if (!error && reckey_in_range(0, &ref, item_field->field, 
 			                conds, range_fl, prefix_len))
 	    error= HA_ERR_KEY_NOT_FOUND;
@@ -739,14 +795,24 @@ static bool find_key_for_maxmin(bool max_fl, TABLE_REF *ref,
           if (!max_fl && key_part_used == key_part_to_use && part->null_bit)
           {
             /*
-              SELECT MIN(key_part2) FROM t1 WHERE key_part1=const
-              If key_part2 may be NULL, then we want to find the first row
-              that is not null
+              The query is on this form:
+
+              SELECT MIN(key_part_k) 
+              FROM t1 
+              WHERE key_part_1 = const and ... and key_part_k-1 = const
+
+              If key_part_k is nullable, we want to find the first matching row
+              where key_part_k is not null. The key buffer is now {const, ...,
+              NULL}. This will be passed to the handler along with a flag
+              indicating open interval. If a tuple is read that does not match
+              these search criteria, an attempt will be made to read an exact
+              match for the key buffer.
             */
+            /* Set the first byte of key_part_k to 1, that means NULL */
             ref->key_buff[ref->key_length]= 1;
             ref->key_length+= part->store_length;
             *range_fl&= ~NO_MIN_RANGE;
-            *range_fl|= NEAR_MIN;                // > NULL
+            *range_fl|= NEAR_MIN; // Open interval
           }
           /*
             The following test is false when the key in the key tree is