Fix Multi-recordset cache when calling procedure.

Change UT so that it tests the cache of a multi-recordset.

Author: silvakid

common/result.cc

@@ -187,6 +187,16 @@ Result_impl::~Result_impl()
 
 
 bool Result_impl::next_result()
+{
+  pop_row_cache();
+  if(!m_result_cache.empty())
+    return true;
+
+  // Nothing in cache... jump to next resultset and read it
+  return read_next_result();
+}
+
+bool Result_impl::read_next_result()
 {
   /*
     Note: closing cursor discards previous rset. Only then
@@ -203,8 +213,6 @@ bool Result_impl::next_result()
 
   delete m_cursor;
   m_cursor = nullptr;
-  m_mdata.reset();
-  clear_cache();
   m_pending_rows = false;
   m_inited = true;
 
@@ -226,30 +234,38 @@ bool Result_impl::next_result()
   // Wait for cursor to fetch result meta-data and copy it to local storage.
 
   m_cursor->wait();
-  m_mdata.reset(new Meta_data(*m_cursor));
 
   m_pending_rows = true;
+  //Push new row cache
+  push_row_cache();
 
   return true;
 }
 
+void Result_impl::push_row_cache()
+{
+  m_result_mdata.push(Shared_meta_data(new Meta_data(*m_cursor)));
+  m_result_cache.push(Row_cache());
+  m_result_cache_size.push(0);
+}
+
 
 const Row_data* Result_impl::get_row()
 {
   // TODO: Session parameter for cache prefetch size
 
-  if (!load_cache(16))
+  load_cache(16);
+
+  if (m_result_cache.empty() || m_result_cache.front().empty())
   {
     if (m_reply->entry_count() > 0)
       m_reply->get_error().rethrow();
     return nullptr;
   }
 
-  assert(!m_row_cache.empty());
-
-  m_row = m_row_cache.front();
-  m_row_cache.pop_front();
-  m_row_cache_size--;
+  m_row = m_result_cache.front().front();
+  m_result_cache.front().pop_front();
+  m_result_cache_size.front()--;
   return &m_row;
 }
 
@@ -260,14 +276,19 @@ const Row_data* Result_impl::get_row()
   prefetch_size rows into the cache. If prefetch_size is 0, it loads
   all remaining rows into the cache (even if cache currently contains some
   rows).
+  It caches elements to the last queue element, since more resultsets could have
+  been cached before.
 */
 
 bool Result_impl::load_cache(row_count_t prefetch_size)
 {
   if (!m_inited)
     next_result();
 
-  if (!m_row_cache.empty() && 0 != prefetch_size)
+  if(m_result_cache.empty())
+    return false;
+
+  if (!m_result_cache.back().empty() && 0 != prefetch_size)
     return true;
 
   if (!m_pending_rows)
@@ -278,8 +299,8 @@ bool Result_impl::load_cache(row_count_t prefetch_size)
     element in the cache.
   */
 
-  if (m_row_cache.empty())
-    m_cache_it = m_row_cache.before_begin();
+  if (m_result_cache.back().empty())
+    m_cache_it = m_result_cache.back().before_begin();
 
   // Initiate row reading operation
 
@@ -303,7 +324,7 @@ bool Result_impl::load_cache(row_count_t prefetch_size)
     m_pending_rows = false;
   }
 
-  return !m_row_cache.empty();
+  return !m_result_cache.back().empty();
 }
 
 
@@ -330,8 +351,8 @@ void Result_impl::row_end(row_count_t)
   if (!m_row_filter(m_row))
     return;
 
-  m_cache_it = m_row_cache.emplace_after(m_cache_it, std::move(m_row));
-  m_row_cache_size++;
+  m_cache_it = m_result_cache.back().emplace_after(m_cache_it, std::move(m_row));
+  m_result_cache_size.back()++;
 }
 
 void Result_impl::end_of_data()

common/result.h

@@ -42,7 +42,7 @@
 
 
 PUSH_SYS_WARNINGS
-#include <vector>
+#include <queue>
 POP_SYS_WARNINGS
 
 
@@ -882,6 +882,10 @@ class Result_impl
 
   void store();
 
+  // Store all results to cache
+
+  void store_all_results();
+
   /*
     Return the number of rows remaining in the result (the rows that have been
     already fetched with get_row() are not counted).
@@ -927,9 +931,9 @@ class Result_impl
 
   const Column_info& get_column(col_count_t pos) const
   {
-    if (!m_cursor || !m_mdata)
+    if (m_result_mdata.empty() || !m_result_mdata.front())
       THROW("No result set");
-    return m_mdata->get_column(pos);
+    return m_result_mdata.front()->get_column(pos);
   }
 
 protected:
@@ -958,7 +962,7 @@ class Result_impl
 
   // Note: meta-data can be shared with Row instances
 
-  Shared_meta_data    m_mdata;
+  std::queue<Shared_meta_data>  m_result_mdata;
 
   /*
     Method fetch_meta_data() creates a new Meta_data_base instance with
@@ -990,8 +994,10 @@ class Result_impl
 
   using Row_cache = std::forward_list<Row_data>;
 
-  Row_cache   m_row_cache;
-  row_count_t m_row_cache_size = 0;
+  // Each queue elements represents a resultset.
+
+  std::queue<Row_cache> m_result_cache;
+  std::queue<row_count_t> m_result_cache_size;
   Row_cache::iterator m_cache_it;
 
   /*
@@ -1000,16 +1006,32 @@ class Result_impl
     preftetch_size is non-zero then at most that many rows are loaded.
 
     Returns true if some rows are present in the cache.
+
+    This will cache the current resultset to the back of the queue
   */
 
   bool load_cache(row_count_t prefetch_size = 0);
 
-  void clear_cache()
+  // Jumps to new resultset without poping the cache element
+
+  bool read_next_result();
+
+  // Called after resultset has been get by user
+
+  void pop_row_cache()
   {
-    m_row_cache.clear();
-    m_row_cache_size = 0;
+    if(!m_result_mdata.empty())
+      m_result_mdata.pop();
+    if (!m_result_cache.empty())
+      m_result_cache.pop();
+    if (!m_result_cache_size.empty())
+      m_result_cache_size.pop();
   }
 
+  // Called on each resultset to be read.
+
+  void push_row_cache();
+
 public:
 
   // -- Diagnostic information
@@ -1071,9 +1093,9 @@ class Result_impl
 inline
 col_count_t Result_impl::get_col_count() const
 {
-  if (!m_cursor || !m_mdata)
+  if (m_result_mdata.empty())
     THROW("No result set");
-  return m_mdata->col_count();
+  return m_result_mdata.front()->col_count();
 }
 
 inline
@@ -1110,14 +1132,14 @@ const std::vector<std::string>& Result_impl::get_generated_ids() const
 inline
 bool Result_impl::has_data() const
 {
-  return !m_row_cache.empty() || m_pending_rows;
+  return (!m_result_cache.empty() && !m_result_cache.front().empty()) || m_pending_rows;
 }
 
 
 inline
 const Shared_meta_data& Result_impl::get_mdata() const
 {
-  return m_mdata;
+  return m_result_mdata.front();
 }
 
 
@@ -1127,13 +1149,24 @@ void Result_impl::store()
   load_cache();
 }
 
+inline
+void Result_impl::store_all_results()
+{
+  do{
+  store();
+  }while(read_next_result());
+}
+
 inline
 row_count_t Result_impl::count()
 {
   store();
   if (entry_count() > 0)
     get_error().rethrow();
-  return m_row_cache_size;
+  row_count_t rc = 0;
+  if(!m_result_cache_size.empty())
+    rc = m_result_cache_size.front();
+  return rc;
 }
 
 

common/session.cc

@@ -537,7 +537,10 @@ void Settings_impl::get_attributes(cdk::ds::Attr_processor &prc)
 void Session_impl::prepare_for_cmd()
 {
   if (m_current_result)
-    m_current_result->store();
+  {
+    m_current_result->store_all_results();
+  }
+
   m_current_result = nullptr;
 }
 

devapi/tests/bugs-t.cc

@@ -168,6 +168,9 @@ TEST_F(Bugs, bug_27727505_multiple_results)
       .execute();
   SqlResult res = sess.sql("call test").execute();
 
+  //Force result caching
+  SqlResult res2 = sess.sql("call test").execute();
+
   Row row;
   int setNo = 0;
   do
@@ -224,6 +227,9 @@ TEST_F(Bugs, bug_27727505_multiple_results)
     setNo++;
   }
   while(res.nextResult());
+
+  EXPECT_EQ(2, setNo);
+
   sess.sql("drop procedure if exists test").execute();
   sess.sql("CREATE PROCEDURE test() BEGIN select f0, f1 from newtable "
            "where f0 > 1000; select f0, f1  from newtable where f0 <= 10;"
@@ -257,6 +263,31 @@ TEST_F(Bugs, bug_27727505_multiple_results)
     setNo++;
   }
   while(res.nextResult());
+
+  EXPECT_EQ(2, setNo);
+
+  sess.sql("drop procedure if exists test").execute();
+  sess.sql("CREATE PROCEDURE test() BEGIN select f0 from newtable; select f1 from newtable "
+           "where f0 > 100; select f0 as new_f0  from newtable where f0 <= 10;"
+           " END").execute();
+
+  {
+    SqlResult res = sess.sql("call test").execute();
+
+    //Force result caching
+    SqlResult res2 = sess.sql("call test").execute();
+
+    //All resultsets are now cached
+    EXPECT_EQ(100, res.count());
+    EXPECT_EQ(string("f0"), res.getColumn(0).getColumnName());
+    EXPECT_TRUE(res.nextResult());
+    EXPECT_EQ(static_cast<unsigned long>(0), res.count());
+    EXPECT_EQ(string("f1"), res.getColumn(0).getColumnName());
+    EXPECT_TRUE(res.nextResult());
+    EXPECT_EQ(11, res.count());
+    EXPECT_EQ(string("new_f0"), res.getColumn(0).getColumnLabel());
+    EXPECT_FALSE(res.nextResult());
+  }
 }
 
 

xapi/crud_internal.h

@@ -93,7 +93,7 @@ struct mysqlx_result_struct
     if (!data)
       return nullptr;
 
-    m_row_set.emplace_back(*data, m_mdata);
+    m_row_set.emplace_back(*data, m_result_mdata.front());
     return &m_row_set.back();
   }
 

xapi/result.cc

@@ -119,9 +119,9 @@ uint32_t get_type(const Format_info &fi)
 
 const char * mysqlx_result_struct::read_json(size_t *json_byte_size)
 {
-  assert(m_mdata);
-  assert(1 == m_mdata->col_count());
-  assert(cdk::TYPE_DOCUMENT == m_mdata->get_type(0));
+  assert(!m_result_mdata.empty());
+  assert(1 == m_result_mdata.front()->col_count());
+  assert(cdk::TYPE_DOCUMENT == m_result_mdata.front()->get_type(0));
 
   mysqlx_row_struct *row = read_row();