MDEV-11746: Wrong result upon using FIRST_VALUE with a window frame

Reimplement FIRST_VALUE to act as NTH_VALUE with 0 offset. The previous
implementation was flawed when the window frame would remove values.

3 files changed, 17 insertions, 32 deletions

diff --git a/sql/item_windowfunc.cc b/sql/item_windowfunc.cc
index fb2ad666018..59a22c63ed5 100644
--- a/sql/item_windowfunc.cc
+++ b/sql/item_windowfunc.cc
@@ -221,18 +221,6 @@ void Item_sum_percent_rank::setup_window_func(THD *thd, Window_spec *window_spec
   clear();
 }
 
-bool Item_sum_first_value::add()
-{
-  if (value_added)
-    return false;
-
-  /* TODO(cvicentiu) This is done like this due to how Item_sum_hybrid works.
-     For this usecase we can actually get rid of arg_cache. arg_cache is just
-     for running a comparison function. */
-  value_added= true;
-  Item_sum_hybrid_simple::add();
-  return false;
-}
 
 bool Item_sum_hybrid_simple::fix_fields(THD *thd, Item **ref)
 {
diff --git a/sql/item_windowfunc.h b/sql/item_windowfunc.h
index 2beed69c0fa..b4953c8a6ac 100644
--- a/sql/item_windowfunc.h
+++ b/sql/item_windowfunc.h
@@ -331,16 +331,8 @@ class Item_sum_first_value : public Item_sum_hybrid_simple
 {
  public:
   Item_sum_first_value(THD* thd, Item* arg_expr) :
-    Item_sum_hybrid_simple(thd, arg_expr),
-    value_added(false) {}
-
-  bool add();
+    Item_sum_hybrid_simple(thd, arg_expr) {}
 
-  void clear()
-  {
-    value_added= false;
-    Item_sum_hybrid_simple::clear();
-  }
 
   enum Sumfunctype sum_func () const
   {
@@ -354,9 +346,6 @@ class Item_sum_first_value : public Item_sum_hybrid_simple
 
   Item *get_copy(THD *thd, MEM_ROOT *mem_root)
   { return get_item_copy<Item_sum_first_value>(thd, mem_root, this); }
-
- private:
-  bool value_added;
 };
 
 /*
diff --git a/sql/sql_window.cc b/sql/sql_window.cc
index e9e531714f6..668c6a41e08 100644
--- a/sql/sql_window.cc
+++ b/sql/sql_window.cc
@@ -2311,14 +2311,6 @@ void add_special_frame_cursors(THD *thd, Cursor_manager *cursor_manager,
       fc->add_sum_func(item_sum);
       cursor_manager->add_cursor(fc);
       break;
-    case Item_sum::FIRST_VALUE_FUNC:
-      fc= get_frame_cursor(thd, spec, true);
-      fc->set_no_action();
-      cursor_manager->add_cursor(fc);
-      fc= new Frame_positional_cursor(*fc);
-      fc->add_sum_func(item_sum);
-      cursor_manager->add_cursor(fc);
-      break;
     case Item_sum::LAST_VALUE_FUNC:
       fc= get_frame_cursor(thd, spec, false);
       fc->add_sum_func(item_sum);
@@ -2347,6 +2339,22 @@ void add_special_frame_cursors(THD *thd, Cursor_manager *cursor_manager,
       cursor_manager->add_cursor(fc);
       break;
     }
+    case Item_sum::FIRST_VALUE_FUNC:
+    {
+      Frame_cursor *bottom_bound= get_frame_cursor(thd, spec, false);
+      Frame_cursor *top_bound= get_frame_cursor(thd, spec, true);
+      cursor_manager->add_cursor(bottom_bound);
+      cursor_manager->add_cursor(top_bound);
+      DBUG_ASSERT(item_sum->fixed);
+      Item *offset_item= new (thd->mem_root) Item_int(thd, 0);
+      offset_item->fix_fields(thd, &offset_item);
+      fc= new Frame_positional_cursor(*top_bound,
+                                      *top_bound, *bottom_bound,
+                                      *offset_item, false);
+      fc->add_sum_func(item_sum);
+      cursor_manager->add_cursor(fc);
+      break;
+    }
     case Item_sum::NTH_VALUE_FUNC:
     {
       Frame_cursor *bottom_bound= get_frame_cursor(thd, spec, false);