Bug#17757914 MEMORY CORRUPTION/CRASH/ASSERTION FAILED: RECORD_LENGTH == M_RECORD_LENGTH

Resize the main filesort buffer if properties
(num_records, record_length) change.

Author: Tor Didriksen

sql/filesort.cc

@@ -303,19 +303,7 @@ ha_rows filesort(THD *thd, TABLE *table, Filesort *filesort,
     {
       ha_rows keys= memory_available / (param.rec_length + sizeof(char*));
       param.max_keys_per_buffer= (uint) min(num_rows, keys);
-      if (table_sort.get_sort_keys())
-      {
-        // If we have already allocated a buffer, it better have same size!
-        if (std::make_pair(param.max_keys_per_buffer, param.rec_length) !=
-            table_sort.sort_buffer_properties())
-        {
-          /*
-            table->sort will still have a pointer to the same buffer,
-            but that will be overwritten by the assignment below.
-          */
-          table_sort.free_sort_buffer();
-        }
-      }
+
       table_sort.alloc_sort_buffer(param.max_keys_per_buffer, param.rec_length);
       if (table_sort.get_sort_keys())
         break;

sql/filesort_utils.cc

@@ -93,6 +93,22 @@ uchar **Filesort_buffer::alloc_sort_buffer(uint num_records, uint record_length)
   DBUG_EXECUTE_IF("alloc_sort_buffer_fail",
                   DBUG_SET("+d,simulate_out_of_memory"););
 
+  /*
+    For subqueries we try to re-use the buffer, in order to save
+    expensive malloc/free calls. Both of the sizing parameters may change:
+    - num_records due to e.g. different statistics from the engine.
+    - record_length due to different buffer usage:
+      a heap table may be flushed to myisam, which allows us to sort by
+      <key, addon fields> rather than <key, rowid>
+    If we already have a buffer, but with wrong size, we simply delete it.
+   */
+  if (!m_idx_array.is_null())
+  {
+    if (num_records != m_idx_array.size() ||
+        record_length != m_record_length)
+      free_sort_buffer();
+  }
+
   if (m_idx_array.is_null())
   {
     uchar **sort_keys=
@@ -103,11 +119,7 @@ uchar **Filesort_buffer::alloc_sort_buffer(uint num_records, uint record_length)
     uchar **start_of_data= m_idx_array.array() + m_idx_array.size();
     m_start_of_data= reinterpret_cast<uchar*>(start_of_data);
   }
-  else
-  {
-    DBUG_ASSERT(num_records == m_idx_array.size());
-    DBUG_ASSERT(record_length == m_record_length);
-  }
+
   DBUG_RETURN(m_idx_array.array());
 }
 

sql/filesort_utils.h

@@ -1,4 +1,4 @@
-/* Copyright (c) 2010, 2012 Oracle and/or its affiliates. All rights reserved.
+/* Copyright (c) 2010, 2013 Oracle and/or its affiliates. All rights reserved.
 
    This program is free software; you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
@@ -92,13 +92,6 @@ class Filesort_buffer
   /// Allocates the buffer, but does *not* initialize pointers.
   uchar **alloc_sort_buffer(uint num_records, uint record_length);
 
-  /// What is the <num_records, record_length> for the buffer?
-  std::pair<uint, uint> sort_buffer_properties() const
-  {
-    return std::make_pair(static_cast<uint>(m_idx_array.size()),
-                          m_record_length);
-  }
-
   /// Frees the buffer.
   void free_sort_buffer();
 

sql/table.h

@@ -354,9 +354,6 @@ class Filesort_info
   uchar **alloc_sort_buffer(uint num_records, uint record_length)
   { return filesort_buffer.alloc_sort_buffer(num_records, record_length); }
 
-  std::pair<uint, uint> sort_buffer_properties() const
-  { return filesort_buffer.sort_buffer_properties(); }
-
   void free_sort_buffer()
   { filesort_buffer.free_sort_buffer(); }
 

unittest/gunit/filesort_buffer-t.cc

@@ -29,9 +29,6 @@ class FileSortBufferTest : public ::testing::Test
   virtual void TearDown()
   {
     fs_info.free_sort_buffer();
-    std::pair<uint, uint> buffer_properties= fs_info.sort_buffer_properties();
-    EXPECT_EQ(0U, buffer_properties.first);
-    EXPECT_EQ(0U, buffer_properties.second);
     EXPECT_TRUE(NULL == fs_info.get_sort_keys());
   }
 
@@ -42,15 +39,8 @@ class FileSortBufferTest : public ::testing::Test
 TEST_F(FileSortBufferTest, FileSortBuffer)
 {
   const char letters[10]= "abcdefghi";
-  std::pair<uint, uint> buffer_properties= fs_info.sort_buffer_properties();
-  EXPECT_EQ(0U, buffer_properties.first);
-  EXPECT_EQ(0U, buffer_properties.second);
 
   uchar **sort_keys= fs_info.alloc_sort_buffer(10, sizeof(char));
-  buffer_properties= fs_info.sort_buffer_properties();
-  EXPECT_EQ(10U, buffer_properties.first);
-  EXPECT_EQ(sizeof(char), buffer_properties.second);
-
   uchar **null_sort_keys= NULL;
   EXPECT_NE(null_sort_keys, sort_keys);
   EXPECT_NE(null_sort_keys, fs_info.get_sort_keys());