Got sort-and-read single-pass window function computation to work

mysql-test/t/win.test

+create table t1(a int, b int, x char(32));
+insert into t1 values (2, 10, 'xx');
+insert into t1 values (2, 10, 'zz');
+insert into t1 values (2, 20, 'yy');
+insert into t1 values (3, 10, 'xxx');
+insert into t1 values (3, 20, 'vvv');
+# Uncommenting this line causes a crash in setup_group when executing the second
+# select.
+#select row_number() over (order by b) from t1;
+#select a, b, x, row_number() over (partition by a,b order by x),
+#                row_number() over (partition by a),
+#                row_number() over (partition by a order by x)
+#from t1;
+# Uncommenting this line causes a crash in filesort during init_for_filesort.
+#select a, b, x, row_number() over (partition by a order by x) from t1;
+
+select a, row_number() over (partition by a order by b) from t1;
+
+drop table t1;

sql/item_windowfunc.cc

 #include "item_windowfunc.h" 
+#include "my_dbug.h"
+#include "my_global.h"
+#include "sql_select.h" // test if group changed
+
 
 bool
 Item_window_func::fix_fields(THD *thd, Item **ref)
@@ -15,5 +19,37 @@ Item_window_func::fix_fields(THD *thd, Item **ref)
     return TRUE;
 
   fixed= 1;
+  read_value_from_result_field= false;
   return FALSE;
 }
+
+
+/*
+  This must be called before advance_window() can be called.
+
+  @detail
+    If we attempt to do it in fix_fields(), partition_fields will refer
+    to the original window function arguments.
+    We need it to refer to temp.table columns.
+*/
+
+void Item_window_func::setup_partition_border_check(THD *thd)
+{
+  for (ORDER * curr = window_spec->partition_list.first; curr; curr=curr->next) {
+    //curr->item_ptr->fix_fields(thd, curr->item);
+    Cached_item *tmp= new_Cached_item(thd, curr->item[0], TRUE);  
+    partition_fields.push_back(tmp);
+  }
+}
+
+
+void Item_window_func::advance_window() {
+
+  int changed = test_if_group_changed(partition_fields);
+
+  if (changed > -1) {
+    window_func->clear();
+  }
+  window_func->add();
+}
+

sql/item_windowfunc.h

@@ -147,15 +147,21 @@ class Item_window_func : public Item_result_field
 public:
   Item_window_func(THD *thd, Item_sum *win_func, LEX_STRING *win_name)
     : Item_result_field(thd), window_func(win_func),
-      window_name(win_name), window_spec(NULL) {}
+      window_name(win_name), window_spec(NULL), 
+      read_value_from_result_field(false) {}
 
   Item_window_func(THD *thd, Item_sum *win_func, Window_spec *win_spec)
     : Item_result_field(thd), window_func(win_func),
-      window_name(NULL), window_spec(win_spec) {}
+      window_name(NULL), window_spec(win_spec), 
+      read_value_from_result_field(false) {}
 
-  enum Item::Type type() const { return Item::WINDOW_FUNC_ITEM; }
+  /*
+    Computation functions.
+  */
+  void setup_partition_border_check(THD *thd);
 
   enum_field_types field_type() const { return window_func->field_type(); }
+  enum Item::Type type() const { return Item::WINDOW_FUNC_ITEM; }
   
   /* 
     TODO: Window functions are very special functions, so val_() methods have
@@ -170,19 +176,42 @@ class Item_window_func : public Item_result_field
       It calls window_func->val_int() so that current window function value 
       can be saved and stored in the temp.table.
 
-    - Phase#3: the temporaty table is read and passed to query output. (Do 
-      I understand correctly that Item_window_func::val_XXX won't be called 
-      at all in this phase? Need to check)
-
+    - Phase#3: the temporary table is read and passed to query output. 
+      However, Item_window_func still remains in the select list, so
+      item_windowfunc->val_int() will be called.
   */
-  double val_real() {  return window_func->val_real(); }
+private:
+  bool read_value_from_result_field;
 
-  longlong val_int() { return window_func->val_int(); }
+public:
+  void set_read_value_from_result_field() 
+  {
+    read_value_from_result_field= true;
+  }
 
-  String* val_str(String* str) { return window_func->val_str(str); }
+  double val_real() 
+  {
+    return read_value_from_result_field? result_field->val_real() :
+                                         window_func->val_real();
+  }
+
+  longlong val_int()
+  { 
+    return read_value_from_result_field? result_field->val_int() : 
+                                          window_func->val_int(); 
+  }
+
+  String* val_str(String* str)
+  {
+    return read_value_from_result_field? result_field->val_str(str) : 
+                                         window_func->val_str(str);
+  }
 
   my_decimal* val_decimal(my_decimal* dec)
-  { return window_func->val_decimal(dec); }
+  { 
+    return read_value_from_result_field? result_field->val_decimal(dec) : 
+                                         window_func->val_decimal(dec);
+  }
 
   void fix_length_and_dec() { }
 

sql/sql_select.cc

@@ -19304,6 +19304,8 @@ end_send(JOIN *join, JOIN_TAB *join_tab __attribute__((unused)),
 
   if (!end_of_records)
   {
+#if 0    
+#endif
     if (join->table_count &&
         join->join_tab->is_using_loose_index_scan())
     {

sql/sql_window.cc

 #include "sql_select.h"
 #include "item_windowfunc.h"
 #include "filesort.h"
+#include "sql_base.h"
 #include "sql_window.h"
+
 //TODO: why pass List<Window_spec> by value??
 
 bool
@@ -258,10 +260,11 @@ bool JOIN::process_window_functions(List<Item> *curr_fields_list)
         for (ORDER* curr = spec->order_list.first; curr; curr=curr->next, pos++)
           s_order[pos].item = *curr->item;
         
-        //psergey-todo: need the below:?? 
+        /* This is free'd by free_io_cache call below. */
         table[0]->sort.io_cache=(IO_CACHE*) my_malloc(sizeof(IO_CACHE),
                                                    MYF(MY_WME | MY_ZEROFILL|
                                                        MY_THREAD_SPECIFIC));
+
         Filesort_tracker dummy_tracker(false);
         filesort_retval= filesort(thd, table[0], s_order,
                                   total_size,
@@ -274,29 +277,38 @@ bool JOIN::process_window_functions(List<Item> *curr_fields_list)
         join_tab->records= found_rows;
 
         my_free(s_order);
-        //psergey-todo: use the created sorted-index to compute the window
-        //function we're looking at.
-        handler *file= table[0]->file;
         
-        // TODO: We should read in sorted order here, not in rnd_next order!
-        // note: we can use the same approach as filesort uses to compare
-        // sort_keys..
+        /*
+          Go through the sorted array and compute the window function
+        */
         READ_RECORD info;
-
         if (init_read_record(&info, thd, table[0], select, 0, 1, FALSE))
           return true;
 
+        item_win->setup_partition_border_check(thd);
+
         int err;
-        while (!(error=info.read_record(&info)))
+        TABLE *tbl= *table;
+        while (!(err=info.read_record(&info)))
         {
-          //TODO: What happens for "PARTITION BY (item value...) ? 
-          // TODO: Sort keys are available in the record. Can we just check
-          //      them?
-          // TODO: how does one check only 'PARTITION BY' part?
+          store_record(tbl,record[1]);
+          
+          /* 
+            This will cause window function to compute its value for the
+            current row : 
+          */
+          item_win->advance_window();
+
+          /* Put the new value into temptable's field */
+          item_win->save_in_field(item_win->result_field, true);
+          err= tbl->file->ha_update_row(tbl->record[1], tbl->record[0]);
+          if (err && err != HA_ERR_RECORD_IS_THE_SAME)
+            return true;
         }
+        item_win->set_read_value_from_result_field();
         end_read_record(&info);
-
-        // TODO-TODO: also check how the values are read...
+        filesort_free_buffers(table[0], true);
+        free_io_cache(table[0]);
       }
     }
   }