Fix a PS re-execution problem and code cleanup

- Make Item_XXX::cleanup() clean List<Cached_item> orderby_fields.
  (Items survive across PS re-executions. Cached_item don't, because
   they keep pointers to fix_field'ed items, etc)
- Move List<Cached_item> out into Group_bound_tracker.

sql/item_windowfunc.cc

@@ -178,29 +178,21 @@ void Item_window_func::setup_partition_border_check(THD *thd)
 void Item_sum_rank::setup_window_func(THD *thd, Window_spec *window_spec)
 {
   /* TODO: move this into Item_window_func? */
-  for (ORDER *curr= window_spec->order_list->first; curr; curr=curr->next)
+  peer_tracker.init(thd, window_spec->order_list);
-  {
-    Cached_item *tmp= new_Cached_item(thd, curr->item[0], TRUE);
-    orderby_fields.push_back(tmp);
-  }
   clear();
 }
 
 void Item_sum_dense_rank::setup_window_func(THD *thd, Window_spec *window_spec)
 {
   /* TODO: consider moving this && Item_sum_rank's implementation */
-  for (ORDER *curr= window_spec->order_list->first; curr; curr=curr->next)
+  peer_tracker.init(thd, window_spec->order_list);
-  {
-    Cached_item *tmp= new_Cached_item(thd, curr->item[0], TRUE);
-    orderby_fields.push_back(tmp);
-  }
   clear();
 }
 
 bool Item_sum_dense_rank::add()
 {
-  if (test_if_group_changed(orderby_fields) > -1)
+  if (peer_tracker.check_if_next_group())
-   dense_rank++;
+    dense_rank++;
 
   return false;
 }
@@ -209,7 +201,7 @@ bool Item_sum_dense_rank::add()
 bool Item_sum_rank::add()
 {
   row_number++;
-  if (test_if_group_changed(orderby_fields) > -1)
+  if (peer_tracker.check_if_next_group())
   {
     /* Row value changed */
     cur_rank= row_number;
@@ -237,7 +229,7 @@ void Item_window_func::advance_window()
 bool Item_sum_percent_rank::add()
 {
   row_number++;
-  if (test_if_group_changed(orderby_fields) > -1)
+  if (peer_tracker.check_if_next_group())
   {
     /* Row value changed. */
     cur_rank= row_number;
@@ -248,11 +240,7 @@ bool Item_sum_percent_rank::add()
 void Item_sum_percent_rank::setup_window_func(THD *thd, Window_spec *window_spec)
 {
   /* TODO: move this into Item_window_func? */
-  for (ORDER *curr= window_spec->order_list->first; curr; curr=curr->next)
+  peer_tracker.init(thd, window_spec->order_list);
-  {
-    Cached_item *tmp= new_Cached_item(thd, curr->item[0], TRUE);
-    orderby_fields.push_back(tmp);
-  }
   clear();
 }
 

sql/item_windowfunc.h

@@ -6,6 +6,54 @@
 
 class Window_spec;
 
+
+int test_if_group_changed(List<Cached_item> &list);
+
+/* A wrapper around test_if_group_changed */
+class Group_bound_tracker
+{
+  List<Cached_item> group_fields;
+public:
+  void init(THD *thd, SQL_I_List<ORDER> *list)
+  {
+    for (ORDER *curr = list->first; curr; curr=curr->next) 
+    {
+      Cached_item *tmp= new_Cached_item(thd, curr->item[0], TRUE);
+      group_fields.push_back(tmp);
+    }
+  }
+
+  void cleanup()
+  {
+    group_fields.empty();
+  }
+
+  /*
+    Check if the current row is in a different group than the previous row
+    this function was called for.
+    The new row's group becomes the current row's group.
+  */
+  bool check_if_next_group()
+  {
+    if (test_if_group_changed(group_fields) > -1)
+      return true;
+    return false;
+  }
+
+  int compare_with_cache()
+  {
+    List_iterator<Cached_item> li(group_fields);
+    Cached_item *ptr;
+    int res;
+    while ((ptr= li++))
+    {
+      if ((res= ptr->cmp_read_only()))
+        return res;
+    }
+    return 0;
+  }
+};
+
 /*
   ROW_NUMBER() OVER (...)
 
@@ -74,7 +122,7 @@ protected:
   longlong row_number; // just ROW_NUMBER()
   longlong cur_rank;   // current value
   
-  List<Cached_item> orderby_fields;
+  Group_bound_tracker peer_tracker;
 public:
   void clear()
   {
@@ -111,6 +159,11 @@ public:
   }
 
   void setup_window_func(THD *thd, Window_spec *window_spec);
+  void cleanup()
+  {
+    peer_tracker.cleanup();
+    Item_sum_int::cleanup();
+  }
 };
 
 
@@ -136,7 +189,7 @@ public:
 class Item_sum_dense_rank: public Item_sum_int
 {
   longlong dense_rank;
-  List<Cached_item> orderby_fields;
+  Group_bound_tracker peer_tracker;
   /*
      XXX(cvicentiu) This class could potentially be implemented in the rank
      class, with a switch for the DENSE case.
@@ -167,6 +220,11 @@ class Item_sum_dense_rank: public Item_sum_int
 
   void setup_window_func(THD *thd, Window_spec *window_spec);
 
+  void cleanup()
+  {
+    peer_tracker.cleanup();
+    Item_sum_int::cleanup();
+  }
 };
 
 /* TODO-cvicentiu
@@ -365,7 +423,13 @@ class Item_sum_percent_rank: public Item_sum_window_with_context,
   longlong cur_rank;   // Current rank of the current row.
   longlong row_number; // Value if this were ROW_NUMBER() function.
 
-  List<Cached_item> orderby_fields;
+  Group_bound_tracker peer_tracker;
+
+  void cleanup()
+  {
+    peer_tracker.cleanup();
+    Item_sum_window_with_context::cleanup();
+  }
 
   /* Helper function so that we don't cast the context every time. */
   Window_context_row_count* get_context_()

sql/sql_window.cc

@@ -387,46 +387,6 @@ public:
 
 /////////////////////////////////////////////////////////////////////////////
 
-/* A wrapper around test_if_group_changed */
-class Group_bound_tracker
-{
-  List<Cached_item> group_fields;
-public:
-  void init(THD *thd, SQL_I_List<ORDER> *list)
-  {
-    for (ORDER *curr = list->first; curr; curr=curr->next) 
-    {
-      Cached_item *tmp= new_Cached_item(thd, curr->item[0], TRUE);
-      group_fields.push_back(tmp);
-    }
-  }
-
-  /*
-    Check if the current row is in a different group than the previous row
-    this function was called for.
-    The new row's group becomes the current row's group.
-  */
-  bool check_if_next_group()
-  {
-    if (test_if_group_changed(group_fields) > -1)
-      return true;
-    return false;
-  }
-
-  int compare_with_cache()
-  {
-    List_iterator<Cached_item> li(group_fields);
-    Cached_item *ptr;
-    int res;
-    while ((ptr= li++))
-    {
-      if ((res= ptr->cmp_read_only()))
-        return res;
-    }
-    return 0;
-  }
-};
-
 
 /*
   Window frame bound cursor. Abstract interface.