MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:mhansson Date:August 13 2007 12:20pm
Subject:bk commit into 5.0 tree (mhansson:1.2502) BUG#14669
View as plain text  
Below is the list of changes that have just been committed into a local
5.0 repository of martin. When martin does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2007-08-13 14:20:26+02:00, mhansson@stripped +6 -0
  Bug#14669: Optimizer unneccesary re-evaluates static parts
  
  The class Arg_comparator is used to access records using a conditional 
  expression when there is no key on the field(s) in question. It has 
  caching funcionality but it was only used for DATETIME types. This caused
  deterministic functions to be re-evaluated for every row.
  Fixed by implementing caching for all DETERMINISTIC functions with constant
  arguments that return type INT, CHAR, REAL, BINARY, or DECIMAL.

  mysql-test/r/func_test.result@stripped, 2007-08-13 14:20:21+02:00, mhansson@stripped +84 -0
    Bug#14669: Test Result

  mysql-test/t/func_test.test@stripped, 2007-08-13 14:20:21+02:00, mhansson@stripped +58 -0
    Bug#14669: Test Case

  sql/item.cc@stripped, 2007-08-13 14:20:21+02:00, mhansson@stripped +9 -1
    Bug#14669: The Item_cache_*::store methods were modified to call Item_cache::setup

  sql/item.h@stripped, 2007-08-13 14:20:21+02:00, mhansson@stripped +1 -0
    Bug#14669: Added method to retrieve the cache's backing Item.

  sql/item_cmpfunc.cc@stripped, 2007-08-13 14:20:21+02:00, mhansson@stripped +98 -8
    Bug#14669: 
    
    - Method Arg_comparator::item_b_is_cacheable() determines cacheability
    - Method Arg_comparator::get_item_b() takes care of maintaining the cache.

  sql/item_cmpfunc.h@stripped, 2007-08-13 14:20:21+02:00, mhansson@stripped +34 -2
    Bug#14669: Comments, declaration of new methods, and initialization of 
    b_is_cacheable in constructors.

diff -Nrup a/mysql-test/r/func_test.result b/mysql-test/r/func_test.result
--- a/mysql-test/r/func_test.result	2007-03-09 22:18:42 +01:00
+++ b/mysql-test/r/func_test.result	2007-08-13 14:20:21 +02:00
@@ -279,3 +279,87 @@ NULL
 SELECT GREATEST(1.5E+2,1.3E+2,NULL) FROM DUAL;
 GREATEST(1.5E+2,1.3E+2,NULL)
 NULL
+CREATE TABLE t1( a INT, b CHAR(1), c BINARY(1) );
+INSERT INTO t1 VALUES (1, 'a', 'a'), (2, 'b', 'b'), (3, 'c', 'c'), (4, 'd', 'd'),
+(5, 'e', 'e'), (6, 'f', 'f'), (7, 'g', 'g'), (8, 'h', 'h'),
+(9, 'i', 'i'), (1, 'j', 'j');
+CREATE FUNCTION f1( a INT ) RETURNS INT DETERMINISTIC 
+BEGIN
+SET @a = @a + 1;      
+RETURN 5;
+END |
+CREATE FUNCTION f2( a INT ) RETURNS CHAR DETERMINISTIC 
+BEGIN
+SET @a = @a + 1;
+RETURN 'e';
+END |
+CREATE FUNCTION f3( a INT ) RETURNS REAL DETERMINISTIC 
+BEGIN
+SET @a = @a + 1;
+RETURN 5.0;
+END |
+CREATE FUNCTION f4( a INT ) RETURNS BINARY DETERMINISTIC 
+BEGIN
+SET @a = @a + 1;
+RETURN 'a';
+END |
+CREATE FUNCTION f5( a INT ) RETURNS DECIMAL DETERMINISTIC 
+BEGIN
+SET @a = @a + 1;
+RETURN 5.0;
+END |
+SET @a = 0;
+SELECT * FROM t1 WHERE a = f1( 1 );
+a	b	c
+5	e	e
+SELECT @a;
+@a
+1
+SET @a = 0;
+SELECT * FROM t1 WHERE b = f2( 1 );
+a	b	c
+5	e	e
+SELECT @a;
+@a
+1
+SET @a = 0;
+SELECT * FROM t1 WHERE b <=> f2( 1 );
+a	b	c
+5	e	e
+SELECT @a;
+@a
+1
+SET @a = 0;
+SELECT * FROM t1 WHERE a = f3( 1 );
+a	b	c
+5	e	e
+SELECT @a;
+@a
+1
+SET @a = 0;
+SELECT * FROM t1 WHERE c = f4( 1 );
+a	b	c
+1	a	a
+SELECT @a;
+@a
+1
+SET @a = 0;
+SELECT * FROM t1 WHERE c <=> f4( 1 );
+a	b	c
+1	a	a
+SELECT @a;
+@a
+1
+SET @a = 0;
+SELECT * FROM t1 WHERE a = f5( 1 );
+a	b	c
+5	e	e
+SELECT @a;
+@a
+1
+DROP TABLE t1;
+DROP FUNCTION f1;
+DROP FUNCTION f2;
+DROP FUNCTION f3;
+DROP FUNCTION f4;
+DROP FUNCTION f5;
diff -Nrup a/mysql-test/t/func_test.test b/mysql-test/t/func_test.test
--- a/mysql-test/t/func_test.test	2006-11-06 23:45:46 +01:00
+++ b/mysql-test/t/func_test.test	2007-08-13 14:20:21 +02:00
@@ -160,3 +160,61 @@ SELECT LEAST(1.1,1.2,NULL,1.0) FROM DUAL
 SELECT GREATEST(1.5E+2,1.3E+2,NULL) FROM DUAL;
 
 # End of 4.1 tests
+
+#
+# Bug#14669: Optimizer unneccesary re-evaluates static parts
+#
+CREATE TABLE t1( a INT, b CHAR(1), c BINARY(1) );
+INSERT INTO t1 VALUES (1, 'a', 'a'), (2, 'b', 'b'), (3, 'c', 'c'), (4, 'd', 'd'),
+                      (5, 'e', 'e'), (6, 'f', 'f'), (7, 'g', 'g'), (8, 'h', 'h'),
+                      (9, 'i', 'i'), (1, 'j', 'j');
+
+DELIMITER |;
+
+CREATE FUNCTION f1( a INT ) RETURNS INT DETERMINISTIC 
+BEGIN
+  SET @a = @a + 1;      
+  RETURN 5;
+END |
+
+CREATE FUNCTION f2( a INT ) RETURNS CHAR DETERMINISTIC 
+BEGIN
+  SET @a = @a + 1;
+  RETURN 'e';
+END |
+
+CREATE FUNCTION f3( a INT ) RETURNS REAL DETERMINISTIC 
+BEGIN
+  SET @a = @a + 1;
+  RETURN 5.0;
+END |
+
+CREATE FUNCTION f4( a INT ) RETURNS BINARY DETERMINISTIC 
+BEGIN
+  SET @a = @a + 1;
+  RETURN 'a';
+END |
+
+CREATE FUNCTION f5( a INT ) RETURNS DECIMAL DETERMINISTIC 
+BEGIN
+  SET @a = @a + 1;
+  RETURN 5.0;
+END |
+
+DELIMITER ;|
+
+SET @a = 0; SELECT * FROM t1 WHERE a = f1( 1 ); SELECT @a;
+SET @a = 0; SELECT * FROM t1 WHERE b = f2( 1 ); SELECT @a;
+SET @a = 0; SELECT * FROM t1 WHERE b <=> f2( 1 ); SELECT @a;
+SET @a = 0; SELECT * FROM t1 WHERE a = f3( 1 ); SELECT @a;
+SET @a = 0; SELECT * FROM t1 WHERE c = f4( 1 ); SELECT @a;
+SET @a = 0; SELECT * FROM t1 WHERE c <=> f4( 1 ); SELECT @a;
+SET @a = 0; SELECT * FROM t1 WHERE a = f5( 1 ); SELECT @a;
+
+DROP TABLE t1;
+DROP FUNCTION f1;
+DROP FUNCTION f2;
+DROP FUNCTION f3;
+DROP FUNCTION f4;
+DROP FUNCTION f5;
+
diff -Nrup a/sql/item.cc b/sql/item.cc
--- a/sql/item.cc	2007-07-30 01:32:48 +02:00
+++ b/sql/item.cc	2007-08-13 14:20:21 +02:00
@@ -1126,7 +1126,10 @@ Item_case_expr::this_item()
 }
 
 
-
+/*
+   This method segfaults unless fix_length_and_dec has been called
+   (m_thd == NULL).
+*/
 const Item *
 Item_case_expr::this_item() const
 {
@@ -6210,6 +6213,7 @@ void Item_cache::print(String *str)
 
 void Item_cache_int::store(Item *item)
 {
+  setup(item);
   value= item->val_int_result();
   null_value= item->null_value;
   unsigned_flag= item->unsigned_flag;
@@ -6218,6 +6222,7 @@ void Item_cache_int::store(Item *item)
 
 void Item_cache_int::store(Item *item, longlong val_arg)
 {
+  setup(item);
   value= val_arg;
   null_value= item->null_value;
   unsigned_flag= item->unsigned_flag;
@@ -6242,6 +6247,7 @@ my_decimal *Item_cache_int::val_decimal(
 
 void Item_cache_real::store(Item *item)
 {
+  setup(item);
   value= item->val_result();
   null_value= item->null_value;
 }
@@ -6272,6 +6278,7 @@ my_decimal *Item_cache_real::val_decimal
 
 void Item_cache_decimal::store(Item *item)
 {
+  setup(item);
   my_decimal *val= item->val_decimal_result(&decimal_value);
   if (!(null_value= item->null_value) && val != &decimal_value)
     my_decimal2decimal(val, &decimal_value);
@@ -6311,6 +6318,7 @@ my_decimal *Item_cache_decimal::val_deci
 
 void Item_cache_str::store(Item *item)
 {
+  setup(item);
   value_buff.set(buffer, sizeof(buffer), item->collation.collation);
   value= item->str_result(&value_buff);
   if ((null_value= item->null_value))
diff -Nrup a/sql/item.h b/sql/item.h
--- a/sql/item.h	2007-07-30 01:33:00 +02:00
+++ b/sql/item.h	2007-08-13 14:20:21 +02:00
@@ -2432,6 +2432,7 @@ public:
     unsigned_flag= item->unsigned_flag;
     return 0;
   };
+  Item *get_backing_item() { return example; }
   virtual void store(Item *)= 0;
   enum Type type() const { return CACHE_ITEM; }
   static Item_cache* get_cache(Item_result type);
diff -Nrup a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc
--- a/sql/item_cmpfunc.cc	2007-07-15 23:03:32 +02:00
+++ b/sql/item_cmpfunc.cc	2007-08-13 14:20:21 +02:00
@@ -418,6 +418,92 @@ void Item_bool_func2::fix_length_and_dec
   set_cmp_func();
 }
 
+/**
+   @brief Decides whether the right comparison argument is cacheable.
+   @return true if all of the following holds:
+
+     - The Item at **b is a function with constant arguments.
+     - The Item at **a is a FIELD_ITEM where fix_length_and_dec has been called.
+     - The DETERMINISTIC flag is set on the function.
+     - All arguments to the function are constants.
+*/
+bool Arg_comparator::item_b_is_cacheable() {
+  
+  Item *item_a= *a, *item_b= *b;
+
+  if (item_b->type() != Item::FUNC_ITEM ||
+      /* An Item_case_expr will segfault on type() unless it's fixed */
+      !item_a->fixed ||
+      item_a->type() != Item::FIELD_ITEM)
+    return false;
+
+  Field *a_field= ((Item_field*)*a)->field;
+
+  if (a_field == NULL ||
+      item_b->used_tables() & (a_field->table->map | RAND_TABLE_BIT))
+    return false;
+
+  if(item_b->fixed && item_b->type() == Item::FUNC_ITEM) 
+  {
+    Item_func *item_func_b= (Item_func*)item_b;
+    for (uint i= 0; i < item_func_b->argument_count(); i++)
+      if (!item_func_b->arguments()[i]->const_item())
+        return false;
+    return true;
+  }  
+  return false;
+}
+
+/**
+   @brief 
+   Returns the second argument of the comparison operation, possibly cached.
+   
+   @details 
+   This function encapsulates caching of the right hand argument to this
+   Arg_comparator. Deterministic functions with constant arguments are always
+   cached, i.e. built-in functions such as ROUND and SOUNDEX, as well as
+   stored functions declared as DETERMINISTIC.
+
+   It may happen that the client of Arg_comparator changes the item pointed to
+   by **a or **b. This method detects that and invalidates the cache
+   immediately.
+
+   @return 
+   An Item* that may be either the item at *b, or an instance of
+   Item_cache. However, it is guaranteed that the method val_xxx() will return
+   the same value as the corresponding method of the original item.
+ */
+
+Item *Arg_comparator::get_item_b() {
+
+  if (!b_is_cacheable)
+    return *b;
+
+  if (b_cache && b_cache->type() == Item::CACHE_ITEM)
+    if (*b == ((Item_cache*)b_cache)->get_backing_item())
+      return b_cache; // cache is valid
+    else
+    {
+      /* 
+         The backing item has been replaced, find out if the new one is 
+         cacheable.
+       */
+      b_is_cacheable= item_b_is_cacheable();
+      if (!b_is_cacheable)
+        return *b;
+    }
+
+  /* Cache is invalid, fill out the cache */
+  Item_cache *cached_item= Item_cache::get_cache((*b)->result_type());
+  
+  /* Mark the cache as non-const to prevent re-caching. */
+  cached_item->set_used_tables(1);
+  cached_item->store(*b);
+  b_cache= cached_item;
+  
+  return b_cache;
+}
+
 
 int Arg_comparator::set_compare_func(Item_bool_func2 *item, Item_result type)
 {
@@ -735,6 +821,9 @@ int Arg_comparator::set_cmp_func(Item_bo
   ulonglong const_value= (ulonglong)-1;
   a= a1;
   b= a2;
+  a_cache= 0;
+  b_cache= 0;
+  b_is_cacheable= item_b_is_cacheable();
 
   if ((cmp_type= can_compare_as_dates(*a, *b, &const_value)))
   {
@@ -949,7 +1038,7 @@ int Arg_comparator::compare_string()
   String *res1,*res2;
   if ((res1= (*a)->val_str(&owner->tmp_value1)))
   {
-    if ((res2= (*b)->val_str(&owner->tmp_value2)))
+    if ((res2= get_item_b()->val_str(&owner->tmp_value2)))
     {
       owner->null_value= 0;
       return sortcmp(res1,res2,cmp_collation.collation);
@@ -974,7 +1063,7 @@ int Arg_comparator::compare_binary_strin
   String *res1,*res2;
   if ((res1= (*a)->val_str(&owner->tmp_value1)))
   {
-    if ((res2= (*b)->val_str(&owner->tmp_value2)))
+    if ((res2= get_item_b()->val_str(&owner->tmp_value2)))
     {
       owner->null_value= 0;
       uint res1_length= res1->length();
@@ -996,7 +1085,7 @@ int Arg_comparator::compare_e_string()
 {
   String *res1,*res2;
   res1= (*a)->val_str(&owner->tmp_value1);
-  res2= (*b)->val_str(&owner->tmp_value2);
+  res2= get_item_b()->val_str(&owner->tmp_value2);
   if (!res1 || !res2)
     return test(res1 == res2);
   return test(sortcmp(res1, res2, cmp_collation.collation) == 0);
@@ -1007,7 +1096,7 @@ int Arg_comparator::compare_e_binary_str
 {
   String *res1,*res2;
   res1= (*a)->val_str(&owner->tmp_value1);
-  res2= (*b)->val_str(&owner->tmp_value2);
+  res2= get_item_b()->val_str(&owner->tmp_value2);
   if (!res1 || !res2)
     return test(res1 == res2);
   return test(stringcmp(res1, res2) == 0);
@@ -1025,7 +1114,7 @@ int Arg_comparator::compare_real()
   val1= (*a)->val_real();
   if (!(*a)->null_value)
   {
-    val2= (*b)->val_real();
+    val2= get_item_b()->val_real();
     if (!(*b)->null_value)
     {
       owner->null_value= 0;
@@ -1045,8 +1134,9 @@ int Arg_comparator::compare_decimal()
   if (!(*a)->null_value)
   {
     my_decimal value2;
-    my_decimal *val2= (*b)->val_decimal(&value2);
-    if (!(*b)->null_value)
+    Item *item_b= get_item_b();
+    my_decimal *val2= item_b->val_decimal(&value2);
+    if (!item_b->null_value)
     {
       owner->null_value= 0;
       return my_decimal_cmp(val1, val2);
@@ -1118,7 +1208,7 @@ int Arg_comparator::compare_int_signed()
   longlong val1= (*a)->val_int();
   if (!(*a)->null_value)
   {
-    longlong val2= (*b)->val_int();
+    longlong val2= get_item_b()->val_int();
     if (!(*b)->null_value)
     {
       owner->null_value= 0;
diff -Nrup a/sql/item_cmpfunc.h b/sql/item_cmpfunc.h
--- a/sql/item_cmpfunc.h	2007-07-15 19:49:58 +02:00
+++ b/sql/item_cmpfunc.h	2007-08-13 14:20:21 +02:00
@@ -28,6 +28,21 @@ typedef int (Arg_comparator::*arg_cmp_fu
 
 typedef int (*Item_field_cmpfunc)(Item_field *f1, Item_field *f2, void *arg); 
 
+/**
+   @brief Implements a comparison operation betweem dynamic Items.
+   @details
+   This class will cache the results of deterministic functions with constant
+   arguments. The folling conditions apply for caching:
+
+     -# The comparison operation is = or <=>.
+     -# The left argument (a1) is a Field where fix_length_and_dec has been 
+        called.
+     -# The right argument (a2) is a deterministic function. (In the case of 
+        stored functions they should be declared as DETERMINISTIC)
+
+   @see item_b_is_cacheable()
+   @see get_item_b()
+*/
 class Arg_comparator: public Sql_alloc
 {
   Item **a, **b;
@@ -38,18 +53,33 @@ class Arg_comparator: public Sql_alloc
   /* Fields used in DATE/DATETIME comparison. */
   THD *thd;
   enum_field_types a_type, b_type; // Types of a and b items
+  /** @todo: These should be type Item_cache */
   Item *a_cache, *b_cache;         // Cached values of a and b items
+  /**
+     This member is for efficiency, it holds the value computed by
+     item_b_is_cacheable(). Used to inform about the cacheability of the Item
+     at **b. It is initiated upon instantiation of the class, and is
+     automatically updated after a change in the Item pointer *b has been
+     detected.  
+     @see item_b_is_cacheable()
+   */
+  bool b_is_cacheable;
   bool is_nulls_eq;                // TRUE <=> compare for the EQUAL_FUNC
   enum enum_date_cmp_type { CMP_DATE_DFLT= 0, CMP_DATE_WITH_DATE,
                             CMP_DATE_WITH_STR, CMP_STR_WITH_DATE };
+  /** todo: Change type of cache_arg to Item_cahe** */
   ulonglong (*get_value_func)(THD *thd, Item ***item_arg, Item **cache_arg,
                               Item *warn_item, bool *is_null);
 public:
   DTCollation cmp_collation;
 
-  Arg_comparator(): thd(0), a_cache(0), b_cache(0) {};
+  Arg_comparator(): thd(0), a_cache(0), b_cache(0) {
+    b_is_cacheable= false;
+  };
   Arg_comparator(Item **a1, Item **a2): a(a1), b(a2), thd(0),
-    a_cache(0), b_cache(0) {};
+    a_cache(0), b_cache(0) {
+    b_is_cacheable= item_b_is_cacheable();
+  };
 
   int set_compare_func(Item_bool_func2 *owner, Item_result type);
   inline int set_compare_func(Item_bool_func2 *owner_arg)
@@ -95,6 +125,8 @@ public:
 
   void set_datetime_cmp_func(Item **a1, Item **b1);
   static arg_cmp_func comparator_matrix [5][2];
+  bool item_b_is_cacheable();
+  Item *get_item_b();
 
   friend class Item_func;
 };
Thread
bk commit into 5.0 tree (mhansson:1.2502) BUG#14669mhansson13 Aug