List:Commits« Previous MessageNext Message »
From:pem Date:December 16 2005 11:15am
Subject:bk commit into 5.0 tree (pem:1.1983) BUG#15737
View as plain text  
Below is the list of changes that have just been committed into a local
5.0 repository of pem. When pem 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
  1.1983 05/12/16 12:14:09 pem@stripped +4 -0
  Fixed BUG#15737: Stored procedure optimizer bug with LEAVE
    Make shortcutting of jumps to "no-ops" like "hpop 0" and "cpop 0" work in
    the stored procedure optimizer.

  sql/sp_head.h
    1.80 05/12/16 12:14:02 pem@stripped +21 -5
    Made sp_instr_hpop/cpop sublasses of sp_instr_jump (instead of sp_instr_jump),
    and dded opt_shortcut_jump methods for sp_instr_hpop/cpop.
    This makes the optimizer work for jumps to hpop/cpop 0 (no-ops which will be
    removed).

  sql/sp_head.cc
    1.204 05/12/16 12:14:02 pem@stripped +18 -0
    Added opt_shortcut_jump methods for sp_instr_hpop/cpop.

  mysql-test/t/sp-code.test
    1.3 05/12/16 12:14:02 pem@stripped +136 -0
    New test case for BUG#15737.

  mysql-test/r/sp-code.result
    1.3 05/12/16 12:14:02 pem@stripped +139 -0
    New test case for BUG#15737.

# This is a BitKeeper patch.  What follows are the unified diffs for the
# set of deltas contained in the patch.  The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User:	pem
# Host:	mysql.comhem.se
# Root:	/usr/home/pem/bug15737/mysql-5.0

--- 1.2/mysql-test/r/sp-code.result	2005-11-18 18:04:57 +01:00
+++ 1.3/mysql-test/r/sp-code.result	2005-12-16 12:14:02 +01:00
@@ -1,3 +1,5 @@
+drop procedure if exists empty;
+drop procedure if exists code_sample;
 create procedure empty()
 begin
 end;
@@ -60,3 +62,140 @@
 20	cpop 1
 21	stmt 0 "select t.name, t.idx from t2 t order ..."
 drop procedure code_sample;
+drop procedure if exists sudoku_solve;
+create procedure sudoku_solve(p_naive boolean, p_all boolean)
+deterministic
+modifies sql data
+begin
+drop temporary table if exists sudoku_work, sudoku_schedule;
+create temporary table sudoku_work
+(
+row smallint not null,
+col smallint not null,
+dig smallint not null,
+cnt smallint,
+key using btree (cnt),
+key using btree (row),
+key using btree (col),
+unique key using hash (row,col)
+);
+create temporary table sudoku_schedule
+(
+idx int not null auto_increment primary key,
+row smallint not null,
+col smallint not null
+);
+call sudoku_init();
+if p_naive then
+update sudoku_work set cnt = 0 where dig = 0;
+else
+call sudoku_count();
+end if;
+insert into sudoku_schedule (row,col)
+select row,col from sudoku_work where cnt is not null order by cnt desc;
+begin
+declare v_scounter bigint default 0;
+declare v_i smallint default 1;
+declare v_dig smallint;
+declare v_schedmax smallint;
+select count(*) into v_schedmax from sudoku_schedule;
+more: 
+loop
+begin
+declare v_tcounter bigint default 0;
+sched:
+while v_i <= v_schedmax do
+begin
+declare v_row, v_col smallint;
+select row,col into v_row,v_col from sudoku_schedule where v_i = idx;
+select dig into v_dig from sudoku_work
+where v_row = row and v_col = col;
+case v_dig
+when 0 then
+set v_dig = 1;
+update sudoku_work set dig = 1
+where v_row = row and v_col = col;
+when 9 then
+if v_i > 0 then
+update sudoku_work set dig = 0
+where v_row = row and v_col = col;
+set v_i = v_i - 1;
+iterate sched;
+else
+select v_scounter as 'Solutions';
+leave more;
+end if;
+else
+set v_dig = v_dig + 1;
+update sudoku_work set dig = v_dig
+where v_row = row and v_col = col;
+end case;
+set v_tcounter = v_tcounter + 1;
+if not sudoku_digit_ok(v_row, v_col, v_dig) then
+iterate sched;
+end if;
+set v_i = v_i + 1;
+end;
+end while sched;
+select dig from sudoku_work;
+select v_tcounter as 'Tests';
+set v_scounter = v_scounter + 1;
+if p_all and v_i > 0 then
+set v_i = v_i - 1;
+else
+leave more;
+end if;
+end;
+end loop more;
+end;
+drop temporary table sudoku_work, sudoku_schedule;
+end//
+show procedure code sudoku_solve;
+Pos	Instruction
+0	stmt 9 "drop temporary table if exists sudoku..."
+1	stmt 1 "create temporary table sudoku_work ( ..."
+2	stmt 1 "create temporary table sudoku_schedul..."
+3	stmt 95 "call sudoku_init("
+4	jump_if_not 7 p_naive@0
+5	stmt 4 "update sudoku_work set cnt = 0 where ..."
+6	jump 8
+7	stmt 95 "call sudoku_count("
+8	stmt 6 "insert into sudoku_schedule (row,col)..."
+9	set v_scounter@2 0
+10	set v_i@3 1
+11	set v_dig@4 NULL
+12	set v_schedmax@5 NULL
+13	stmt 0 "select count(*) into v_schedmax from ..."
+14	set v_tcounter@6 0
+15	jump_if_not 39 (v_i@3 <= v_schedmax@5)
+16	set v_row@7 NULL
+17	set v_col@8 NULL
+18	stmt 0 "select row,col into v_row,v_col from ..."
+19	stmt 0 "select dig into v_dig from sudoku_wor..."
+20	set_case_expr 0 v_dig@4
+21	jump_if_not 25 (case_expr@0 = 0)
+22	set v_dig@4 1
+23	stmt 4 "update sudoku_work set dig = 1 where ..."
+24	jump 34
+25	jump_if_not 32 (case_expr@0 = 9)
+26	jump_if_not 30 (v_i@3 > 0)
+27	stmt 4 "update sudoku_work set dig = 0 where ..."
+28	set v_i@3 (v_i@3 - 1)
+29	jump 15
+30	stmt 0 "select v_scounter as 'Solutions'"
+31	jump 45
+32	set v_dig@4 (v_dig@4 + 1)
+33	stmt 4 "update sudoku_work set dig = v_dig wh..."
+34	set v_tcounter@6 (v_tcounter@6 + 1)
+35	jump_if_not 37 not(`test`.`sudoku_digit_ok`(v_row@7,v_col@8,v_dig@4))
+36	jump 15
+37	set v_i@3 (v_i@3 + 1)
+38	jump 15
+39	stmt 0 "select dig from sudoku_work"
+40	stmt 0 "select v_tcounter as 'Tests'"
+41	set v_scounter@2 (v_scounter@2 + 1)
+42	jump_if_not 45 (p_all@1 and (v_i@3 > 0))
+43	set v_i@3 (v_i@3 - 1)
+44	jump 14
+45	stmt 9 "drop temporary table sudoku_work, sud..."
+drop procedure sudoku_solve;

--- 1.2/mysql-test/t/sp-code.test	2005-11-18 18:04:57 +01:00
+++ 1.3/mysql-test/t/sp-code.test	2005-12-16 12:14:02 +01:00
@@ -4,6 +4,11 @@
 
 -- source include/is_debug_build.inc
 
+--disable_warnings
+drop procedure if exists empty;
+drop procedure if exists code_sample;
+--enable_warnings
+
 create procedure empty()
 begin
 end;
@@ -47,3 +52,134 @@
 delimiter ;//
 show procedure code code_sample;
 drop procedure code_sample;
+
+
+#
+# BUG#15737: Stored procedure optimizer bug with LEAVE
+#
+
+--disable_warnings
+drop procedure if exists sudoku_solve;
+--enable_warnings
+
+delimiter //;
+create procedure sudoku_solve(p_naive boolean, p_all boolean)
+  deterministic
+  modifies sql data
+begin
+  drop temporary table if exists sudoku_work, sudoku_schedule;
+
+  create temporary table sudoku_work
+  (
+    row smallint not null,
+    col smallint not null,
+    dig smallint not null,
+    cnt smallint,
+    key using btree (cnt),
+    key using btree (row),
+    key using btree (col),
+    unique key using hash (row,col)
+  );
+
+  create temporary table sudoku_schedule
+  (
+    idx int not null auto_increment primary key,
+    row smallint not null,
+    col smallint not null
+  );
+
+  call sudoku_init();
+
+  if p_naive then
+    update sudoku_work set cnt = 0 where dig = 0;
+  else
+    call sudoku_count();
+  end if;
+  insert into sudoku_schedule (row,col)
+    select row,col from sudoku_work where cnt is not null order by cnt desc;
+
+  begin
+    declare v_scounter bigint default 0;
+    declare v_i smallint default 1;
+    declare v_dig smallint;
+    declare v_schedmax smallint;
+
+    select count(*) into v_schedmax from sudoku_schedule;
+
+   more: 
+    loop
+    begin
+      declare v_tcounter bigint default 0;
+
+     sched:
+      while v_i <= v_schedmax do
+      begin
+        declare v_row, v_col smallint;
+
+        select row,col into v_row,v_col from sudoku_schedule where v_i = idx;
+
+        select dig into v_dig from sudoku_work
+          where v_row = row and v_col = col;
+
+        case v_dig
+        when 0 then
+          set v_dig = 1;
+          update sudoku_work set dig = 1
+            where v_row = row and v_col = col;
+        when 9 then
+          if v_i > 0 then
+            update sudoku_work set dig = 0
+              where v_row = row and v_col = col;
+            set v_i = v_i - 1;
+            iterate sched;
+          else
+            select v_scounter as 'Solutions';
+            leave more;
+          end if;
+        else
+          set v_dig = v_dig + 1;
+          update sudoku_work set dig = v_dig
+            where v_row = row and v_col = col;
+        end case;
+
+        set v_tcounter = v_tcounter + 1;
+        if not sudoku_digit_ok(v_row, v_col, v_dig) then
+          iterate sched;
+        end if;
+        set v_i = v_i + 1;
+      end;
+      end while sched;
+
+      select dig from sudoku_work;
+      select v_tcounter as 'Tests';
+      set v_scounter = v_scounter + 1;
+
+      if p_all and v_i > 0 then
+        set v_i = v_i - 1;
+      else
+        leave more;
+      end if;
+    end;
+    end loop more;
+  end;
+
+  drop temporary table sudoku_work, sudoku_schedule;
+end//
+delimiter ;//
+
+# The interestings parts are where the code for the two "leave" are:
+# ...
+#|  26 | jump_if_not 30 (v_i@3 > 0)                                            |
+# ...
+#|  30 | stmt 0 "select v_scounter as 'Solutions'"                             |
+#|  31 | jump 45                                                               |
+# ...
+#|  42 | jump_if_not 45 (p_all@1 and (v_i@3 > 0))                              |
+#|  43 | set v_i@3 (v_i@3 - 1)                                                 |
+#|  44 | jump 14                                                               |
+#|  45 | stmt 9 "drop temporary table sudoku_work, sud..."                     |
+#+-----+-----------------------------------------------------------------------+
+# The bug appeared at position 42 (with the wrong destination).
+show procedure code sudoku_solve;
+
+drop procedure sudoku_solve;

--- 1.203/sql/sp_head.cc	2005-12-14 13:18:19 +01:00
+++ 1.204/sql/sp_head.cc	2005-12-16 12:14:02 +01:00
@@ -2677,6 +2677,15 @@
   m_count= m_ctx->diff_handlers(dst_ctx);
 }
 
+uint
+sp_instr_hpop::opt_shortcut_jump(sp_head *sp, sp_instr *start)
+{
+  if (m_count)
+    return m_ip;                /* Normal hpop */
+  /* hpop 0 is treated as a jump ip+1 */
+  return sp_instr_jump::opt_shortcut_jump(sp, start);
+}
+
 
 /*
   sp_instr_hreturn class functions
@@ -2802,6 +2811,15 @@
 sp_instr_cpop::backpatch(uint dest, sp_pcontext *dst_ctx)
 {
   m_count= m_ctx->diff_cursors(dst_ctx);
+}
+
+uint
+sp_instr_cpop::opt_shortcut_jump(sp_head *sp, sp_instr *start)
+{
+  if (m_count)
+    return m_ip;                /* Normal hpop */
+  /* hpop 0 is treated as a jump ip+1 */
+  return sp_instr_jump::opt_shortcut_jump(sp, start);
 }
 
 

--- 1.79/sql/sp_head.h	2005-12-07 18:31:03 +01:00
+++ 1.80/sql/sp_head.h	2005-12-16 12:14:02 +01:00
@@ -833,7 +833,13 @@
 }; // class sp_instr_hpush_jump : public sp_instr_jump
 
 
-class sp_instr_hpop : public sp_instr
+/*
+  This is a subclass of sp_instr_jump (instead of sp_instr) to make it
+  easier to optimize the no-ops ("cpop 0") which sometimes appear
+  with "leave" and forward "goto". Treating it as a "jump i+1" enables
+  the optimizer to shortcut jumps to these instructions.
+*/
+class sp_instr_hpop : public sp_instr_jump
 {
   sp_instr_hpop(const sp_instr_hpop &);	/* Prevent use of these */
   void operator=(sp_instr_hpop &);
@@ -841,7 +847,7 @@
 public:
 
   sp_instr_hpop(uint ip, sp_pcontext *ctx, uint count)
-    : sp_instr(ip, ctx), m_count(count)
+    : sp_instr_jump(ip, ctx, ip+1), m_count(count)
   {}
 
   virtual ~sp_instr_hpop()
@@ -860,11 +866,13 @@
     return m_ip+1;
   }
 
+  virtual uint opt_shortcut_jump(sp_head *sp, sp_instr *start);
+
 private:
 
   uint m_count;
 
-}; // class sp_instr_hpop : public sp_instr
+}; // class sp_instr_hpop : public sp_instr_jump
 
 
 class sp_instr_hreturn : public sp_instr_jump
@@ -927,7 +935,13 @@
 }; // class sp_instr_cpush : public sp_instr
 
 
-class sp_instr_cpop : public sp_instr
+/*
+  This is a subclass of sp_instr_jump (instead of sp_instr) to make it
+  easier to optimize the no-ops ("cpop 0") which sometimes appear
+  with "leave" and forward "goto". Treating it as a "jump i+1" enables
+  the optimizer to shortcut jumps to these instructions.
+*/
+class sp_instr_cpop : public sp_instr_jump
 {
   sp_instr_cpop(const sp_instr_cpop &); /* Prevent use of these */
   void operator=(sp_instr_cpop &);
@@ -935,7 +949,7 @@
 public:
 
   sp_instr_cpop(uint ip, sp_pcontext *ctx, uint count)
-    : sp_instr(ip, ctx), m_count(count)
+    : sp_instr_jump(ip, ctx, ip+1), m_count(count)
   {}
 
   virtual ~sp_instr_cpop()
@@ -953,6 +967,8 @@
       marked= 1;
     return m_ip+1;
   }
+
+  virtual uint opt_shortcut_jump(sp_head *sp, sp_instr *start);
 
 private:
 
Thread
bk commit into 5.0 tree (pem:1.1983) BUG#15737pem16 Dec