MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:kgeorge Date:November 26 2007 5:16pm
Subject:bk commit into 5.0 tree (gkodinov:1.2580) BUG#30355
View as plain text  
Below is the list of changes that have just been committed into a local
5.0 repository of kgeorge. When kgeorge 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-11-26 19:16:51+02:00, gkodinov@stripped +5 -0
  Bug #30355: Incorrect ordering of UDF results
  
  Currently the initialization of an UDF is plumbed 
  together with determining if it returns a const result.
  This doesn't take into account the optimizer's pass to
  re-evaluate if a function is a const when marking tables
  as const or doing source transformations.
  This caused the sequence() UDF to be considered 
  deterministic and returning constant by the optimizer.
  Fixed by:
  1. Marking sequence() as returning a non-const.
  2. When an UDF returns a non-const flag and 
  has no tables it depends on consider this function 
  non-deterministic and set RAND_TABLE_BIT
  3. Implemented a shim update_used_tables() to 
  prevent calling update_used_tables() for 
  non-deterministic UDF.
  Note that update_used_tables() will still be called
  for deterministic UDFs and will allow for a good 
  optimization of such calls.

  mysql-test/r/udf.result@stripped, 2007-11-26 19:16:49+02:00, gkodinov@stripped +17 -0
    Bug #30355: test case

  mysql-test/t/udf.test@stripped, 2007-11-26 19:16:49+02:00, gkodinov@stripped +14 -0
    Bug #30355: test case

  sql/item_func.cc@stripped, 2007-11-26 19:16:49+02:00, gkodinov@stripped +6 -0
    Bug #30355: keep const_item_cache and used_tables_cache in sync

  sql/item_func.h@stripped, 2007-11-26 19:16:49+02:00, gkodinov@stripped +22 -0
    Bug #30355: 
     - a shim implementation of update_used_tables()
     - keep const_item_cache and used_tables_cache in sync

  sql/udf_example.c@stripped, 2007-11-26 19:16:49+02:00, gkodinov@stripped +5 -6
    Bug #30355: Wrong value for const_item fixed.

diff -Nrup a/mysql-test/r/udf.result b/mysql-test/r/udf.result
--- a/mysql-test/r/udf.result	2007-10-19 01:39:51 +03:00
+++ b/mysql-test/r/udf.result	2007-11-26 19:16:49 +02:00
@@ -327,4 +327,21 @@ DROP FUNCTION check_const_len;
 DROP PROCEDURE check_const_len_sp;
 DROP TRIGGER check_const_len_trigger;
 DROP TABLE const_len_bug;
+CREATE FUNCTION sequence RETURNS INTEGER SONAME "UDF_EXAMPLE_LIB";
+CREATE TABLE t1 (a INT);
+INSERT INTO t1 VALUES (4),(3),(2),(1);
+SELECT sequence() AS seq, a FROM t1 ORDER BY seq ASC;
+seq	a
+1	4
+2	3
+3	2
+4	1
+SELECT sequence() AS seq, a FROM t1 ORDER BY seq DESC;
+seq	a
+4	1
+3	2
+2	3
+1	4
+DROP FUNCTION sequence;
+DROP TABLE t1;
 End of 5.0 tests.
diff -Nrup a/mysql-test/t/udf.test b/mysql-test/t/udf.test
--- a/mysql-test/t/udf.test	2007-10-19 01:39:52 +03:00
+++ b/mysql-test/t/udf.test	2007-11-26 19:16:49 +02:00
@@ -362,4 +362,18 @@ DROP PROCEDURE check_const_len_sp;
 DROP TRIGGER check_const_len_trigger;
 DROP TABLE const_len_bug;
 
+
+#
+# Bug #30355: Incorrect ordering of UDF results
+#
+
+--replace_result $UDF_EXAMPLE_LIB UDF_EXAMPLE_LIB
+eval CREATE FUNCTION sequence RETURNS INTEGER SONAME "$UDF_EXAMPLE_LIB";
+CREATE TABLE t1 (a INT);
+INSERT INTO t1 VALUES (4),(3),(2),(1);
+SELECT sequence() AS seq, a FROM t1 ORDER BY seq ASC;
+SELECT sequence() AS seq, a FROM t1 ORDER BY seq DESC;
+DROP FUNCTION sequence;
+DROP TABLE t1;
+
 --echo End of 5.0 tests.
diff -Nrup a/sql/item_func.cc b/sql/item_func.cc
--- a/sql/item_func.cc	2007-11-14 15:24:02 +02:00
+++ b/sql/item_func.cc	2007-11-26 19:16:49 +02:00
@@ -2968,6 +2968,12 @@ udf_handler::fix_fields(THD *thd, Item_r
     func->max_length=min(initid.max_length,MAX_BLOB_WIDTH);
     func->maybe_null=initid.maybe_null;
     const_item_cache=initid.const_item;
+    /* 
+      Keep used_tables_cache in sync with const_item_cache.
+      TODO: get rid of the two members that mean connected things.
+    */  
+    if (!const_item_cache && !used_tables_cache)
+      used_tables_cache= RAND_TABLE_BIT;
     func->decimals=min(initid.decimals,NOT_FIXED_DEC);
   }
   initialized=1;
diff -Nrup a/sql/item_func.h b/sql/item_func.h
--- a/sql/item_func.h	2007-10-29 13:39:54 +02:00
+++ b/sql/item_func.h	2007-11-26 19:16:49 +02:00
@@ -1016,6 +1016,28 @@ public:
     fixed= 1;
     return res;
   }
+  void update_used_tables() 
+  {
+    /*
+      TODO: This is a shim implementation.
+      It expoits the fact that update_used_tables() is generally 
+      called by the optimizer to detect new constant items after 
+      zero or more tables are marked as constant.
+      Thus it's safe to assume that if an UDF is already a
+      constant we may safely skip calling update_used_tables() for
+      its arguments.
+      Ideally we should separate udf_handler::init() into 
+      udf_handler::check_const() and a real udf_handler::init() 
+      and call both in fix_fields and only udf_handler::check_const()
+      here.
+    */  
+    if (!const_item_cache && used_tables_cache != RAND_TABLE_BIT)
+    {
+      Item_func::update_used_tables();
+      if (!const_item_cache && !used_tables_cache)
+        used_tables_cache= RAND_TABLE_BIT;
+    }
+  }
   void cleanup();
   Item_result result_type () const { return udf.result_type(); }
   table_map not_null_tables() const { return 0; }
diff -Nrup a/sql/udf_example.c b/sql/udf_example.c
--- a/sql/udf_example.c	2007-10-19 01:39:52 +03:00
+++ b/sql/udf_example.c	2007-11-26 19:16:49 +02:00
@@ -648,13 +648,12 @@ my_bool sequence_init(UDF_INIT *initid, 
     return 1;
   }
   bzero(initid->ptr,sizeof(longlong));
-  /*
-    Fool MySQL to think that this function is a constant
-    This will ensure that MySQL only evalutes the function
-    when the rows are sent to the client and not before any ORDER BY
-    clauses
+  /* 
+    sequence() is a non-deterministic function : it has different value 
+    even if called with the same arguments.
+    TODO: need to move that out of <func>_init().
   */
-  initid->const_item=1;
+  initid->const_item=0;
   return 0;
 }
 
Thread
bk commit into 5.0 tree (gkodinov:1.2580) BUG#30355kgeorge26 Nov
  • Re: bk commit into 5.0 tree (gkodinov:1.2580) BUG#30355Gleb Shchepa27 Nov