Bug#26389508 INT JOIN_READ_KEY(QEP_TAB*): ASSERTION `!TABLE->HAS_NULL_ROW()' FAILED

Analysis:

When a window with buffering follows a equijoin on a unique index
(JT_EQ_REF) , we can get into trouble because windowing modifies the
input record, presuming that once the windowing has been handed the
record, next time control passes back to the join code a (new) record
will be read to the input record.

However, this does not hold with JT_EQ_REF, cf. the caching done in
join_read_key:

From its Doxygen:

  "Since the eq_ref access method will always return the same row, it
   is not necessary to read the row more than once, regardless of how
   many times it is needed in execution.  This cache element is used
   when a row is needed after it has been read once, unless a key
   conversion error has occurred, or the cache has been disabled."

Fix:

We solve this problem by reinstating the input record before handing
control back from end_write_wf. We optimize: only do this if the
window in question follows after such a JOIN, i.e. window #1, and it
has actually clobbered the input record. This can only happen if
the last qep_tab has type JT_EQ_REF.

Another, perhaps better approach, is to refactor to never touch the
input record but keep the copying between the out record and the frame
table record instead. Left for future refactoring.

Added some missing Window method "const"s, and folded a couple of
one-liners into window.h (from .cc).

Repro added.

Change-Id: I33bc43cd99ff79303b17d181abc3805ce226fb85

Author: Dag Wanvik

mysql-test/r/window_functions.result

@@ -8764,3 +8764,57 @@ FROM_UNIXTIME(LAG('-778:36:16.905133',246) RESPECT NULLS OVER(),
 REPLACE('%M%V   ','',''))
 NULL
 # End of test for Bug#26739028
+#
+# Bug#26389508: INT JOIN_READ_KEY(QEP_TAB*): ASSERTION
+#               `!TABLE->HAS_NULL_ROW()' FAILED
+#
+CREATE TABLE t1 (
+pk int(11) NOT NULL AUTO_INCREMENT,
+col_int int(11) DEFAULT NULL,
+PRIMARY KEY (pk)
+);
+INSERT INTO t1 VALUES (1,NULL),(2,4),(3,-501481472),(4,NULL),(5,3);
+CREATE TABLE t2 (
+col_int_key int(11) DEFAULT NULL,
+KEY col_int_key (col_int_key)
+);
+INSERT INTO t2 VALUES (NULL),(NULL),(NULL),(NULL),(5),(5);
+This JOIN needs the fix
+SELECT FIRST_VALUE( alias1.pk ) OVER
+(ROWS BETWEEN UNBOUNDED PRECEDING AND 1 FOLLOWING) AS field1
+FROM  t1 AS alias1 RIGHT JOIN t2 AS alias2
+ON  alias1.pk = alias2.col_int_key;
+field1
+NULL
+NULL
+NULL
+NULL
+NULL
+NULL
+This doesn't and shouldn't do save/restore it (not JT_EQ_REF). Verify
+with debugger.
+SELECT FIRST_VALUE( alias1.pk ) OVER
+(ROWS BETWEEN UNBOUNDED PRECEDING AND 1 FOLLOWING) AS field1
+FROM  t1 AS alias1 RIGHT JOIN t2 AS alias2
+ON  alias1.pk > alias2.col_int_key;
+field1
+NULL
+NULL
+NULL
+NULL
+NULL
+NULL
+Nor this one (not first window). Verify with debugger.
+SELECT ROW_NUMBER() OVER () AS `row#`,
+FIRST_VALUE( alias1.pk ) OVER
+(ROWS BETWEEN UNBOUNDED PRECEDING AND 1 FOLLOWING) AS field1
+FROM  t1 AS alias1 RIGHT JOIN t2 AS alias2
+ON  alias1.pk = alias2.col_int_key;
+row#	field1
+1	NULL
+2	NULL
+3	NULL
+4	NULL
+5	NULL
+6	NULL
+DROP TABLE t1, t2;

mysql-test/t/window_functions.test

@@ -3758,6 +3758,46 @@ SELECT FROM_UNIXTIME(LAG('-778:36:16.905133',246) RESPECT NULLS OVER(),
 
 --echo # End of test for Bug#26739028
 
+--echo #
+--echo # Bug#26389508: INT JOIN_READ_KEY(QEP_TAB*): ASSERTION
+--echo #               `!TABLE->HAS_NULL_ROW()' FAILED
+--echo #
+CREATE TABLE t1 (
+  pk int(11) NOT NULL AUTO_INCREMENT,
+  col_int int(11) DEFAULT NULL,
+  PRIMARY KEY (pk)
+);
+
+INSERT INTO t1 VALUES (1,NULL),(2,4),(3,-501481472),(4,NULL),(5,3);
+CREATE TABLE t2 (
+  col_int_key int(11) DEFAULT NULL,
+  KEY col_int_key (col_int_key)
+);
+
+INSERT INTO t2 VALUES (NULL),(NULL),(NULL),(NULL),(5),(5);
+
+--echo This JOIN needs the fix
+SELECT FIRST_VALUE( alias1.pk ) OVER
+       (ROWS BETWEEN UNBOUNDED PRECEDING AND 1 FOLLOWING) AS field1
+   FROM  t1 AS alias1 RIGHT JOIN t2 AS alias2
+         ON  alias1.pk = alias2.col_int_key;
+
+--echo This doesn't and shouldn't do save/restore it (not JT_EQ_REF). Verify
+--echo with debugger.
+SELECT FIRST_VALUE( alias1.pk ) OVER
+       (ROWS BETWEEN UNBOUNDED PRECEDING AND 1 FOLLOWING) AS field1
+   FROM  t1 AS alias1 RIGHT JOIN t2 AS alias2
+         ON  alias1.pk > alias2.col_int_key;
+
+--echo Nor this one (not first window). Verify with debugger.
+SELECT ROW_NUMBER() OVER () AS `row#`,
+       FIRST_VALUE( alias1.pk ) OVER
+       (ROWS BETWEEN UNBOUNDED PRECEDING AND 1 FOLLOWING) AS field1
+   FROM  t1 AS alias1 RIGHT JOIN t2 AS alias2
+         ON  alias1.pk = alias2.col_int_key;
+
+DROP TABLE t1, t2;
+
 # Local Variables:
 # mode: sql
 # sql-product: mysql

sql/sql_executor.cc

@@ -3936,6 +3936,12 @@ buffer_record_somewhere(THD *thd, Window *w, int64 rowno)
     w->save_special_record(rowno, t);
     DBUG_RETURN(false); // special record, don't put in frame buffer
   }
+  else if (w->needs_restore_input_row())
+  {
+    w->save_special_record(Window::FBC_LAST_BUFFERED_ROW, t);
+    // Also put in frame buffer
+  }
+
 
   DBUG_ASSERT(t->is_created());
 
@@ -4030,8 +4036,7 @@ buffer_record_somewhere(THD *thd, Window *w, int64 rowno)
       if ((w->m_tmp_pos.m_position =
            (uchar*)sql_alloc(t->file->ref_length)) == nullptr)
         DBUG_RETURN(true);
-      
-   }
+    }
 
     error= t->file->ha_rnd_next(record);
     t->file->position(record);
@@ -4267,13 +4272,17 @@ bring_back_frame_row(THD *thd,
   DBUG_ASSERT(reason == Window::REA_MISC_POSITIONS || fno == 0);
 
   uchar *fb_rec= w.frame_buffer()->record[0];
+  w.set_input_row_clobbered(rowno != w.last_rowno_in_cache() &&
+                            rowno != Window::FBC_LAST_BUFFERED_ROW);
 
-  if (rowno == Window::FBC_FIRST_IN_NEXT_PARTITION)
+  if (rowno == Window::FBC_FIRST_IN_NEXT_PARTITION ||
+      rowno == Window::FBC_LAST_BUFFERED_ROW)
   {
     w.restore_special_record(rowno, fb_rec);
   }
   else
   {
+    DBUG_ASSERT(reason != Window::REA_WONT_UPDATE_HINT);
 
 #if !defined(DBUG_OFF) && defined(WF_DEBUG)
     /*
@@ -4287,6 +4296,7 @@ bring_back_frame_row(THD *thd,
     void *tmpbuff= std::malloc(v.size());
     std::memcpy(tmpbuff, v.data(), v.size());
 #endif
+
     if (read_frame_buffer_row(rowno, &w, reason == Window::REA_MISC_POSITIONS))
       DBUG_RETURN(true);
 
@@ -5573,7 +5583,7 @@ reestablish_new_partition_row(Window &w,
 
   /* bring back saved row for next partition */
   if (bring_back_frame_row(thd, w, Window::FBC_FIRST_IN_NEXT_PARTITION,
-                           Window::REA_FIRST_IN_PARTITION))
+                           Window::REA_WONT_UPDATE_HINT))
     DBUG_RETURN(true);
 
   DBUG_RETURN(false);
@@ -5840,6 +5850,21 @@ end_write_wf(JOIN *join, QEP_TAB *const qep_tab, bool end_of_records)
       {
         out_tbl->m_window->reset_partition_state();
       }
+      else if (win->input_row_clobbered() &&
+               win->needs_restore_input_row())
+      {
+        /*
+          Reestablish last row read from input table in case it is needed again
+          before reading a new row. May be necessary if this is the first window
+          following after a join, cf. the caching presumption in join_read_key.
+          This logic can be removed if we move to copying between out
+          tmp record and frame buffer record, instead of involving the in
+          record. FIXME.
+        */
+        if (bring_back_frame_row(thd, *win, Window::FBC_LAST_BUFFERED_ROW,
+                                 Window::REA_WONT_UPDATE_HINT))
+          DBUG_RETURN(NESTED_LOOP_ERROR);
+      }
     }
     else
     {

sql/sql_select.cc

@@ -4771,6 +4771,9 @@ bool JOIN::make_tmp_tables_info()
        */
       const uint widx= REF_SLICE_WIN_1 + wno;
       const int fbidx= widx + m_windows.elements; // use far area
+      m_windows[wno]->set_needs_restore_input_row(
+        wno == 0 && qep_tab[primary_tables - 1].type() == JT_EQ_REF);
+
 
       if (m_windows[wno]->needs_buffering())
       {

sql/window.cc

@@ -1362,13 +1362,6 @@ bool Window::setup_windows(THD* thd,
 }
 
 
-bool Window::needs_sorting()
-{
-  return (m_partition_by != nullptr ||
-          m_order_by != nullptr);
-}
-
-
 bool Window::has_dynamic_frame_upper_bound() const
 {
   return m_frame == nullptr && m_order_by != nullptr;
@@ -1526,12 +1519,6 @@ void Window::reset_execution_state(Reset_level level)
 }
 
 
-bool Window::is_last_row_in_frame ()
-{
-  return (m_is_last_row_in_frame ||
-          m_select->table_list.elements == 0);
-}
-
 //int64 Window::frame_cardinality()
 //{
 //  if (!m_needs_frame_buffering)