Bug #31310238: SUPPORT SORTING MULTIPLE TABLES [patch 3/5, get row IDs] [noclose]

When sorting a join of multiple tables by row ID, make filesort and underlying
iterators call prepare_for_position() correctly. Otherwise, some storage
engines will not give us proper row IDs when calling position().

Change-Id: I7892a0a0a5f806aed0dd8063bc6d6b39f616bb39

Author: Steinar H. Gunderson

sql/bka_iterator.cc

@@ -103,6 +103,8 @@ bool BKAIterator::Init() {
       return true;
     }
   }
+  PrepareForRequestRowId(m_outer_input_tables.tables(),
+                         m_outer_input_tables.tables_to_get_rowid_for());
 
   BeginNewBatch();
   m_end_of_outer_rows = false;

sql/composite_iterators.cc

@@ -1324,6 +1324,12 @@ bool WeedoutIterator::Init() {
       !m_sj->tmp_table->file->inited) {
     m_sj->tmp_table->file->ha_index_init(0, false);
   }
+  for (SJ_TMP_TABLE_TAB *tab = m_sj->tabs; tab != m_sj->tabs_end; ++tab) {
+    TABLE *table = tab->qep_tab->table();
+    if (m_tables_to_get_rowid_for & table->pos_in_table_list->map()) {
+      table->prepare_for_position();
+    }
+  }
   return m_source->Init();
 }
 

sql/filesort.cc

@@ -143,7 +143,8 @@ struct Mem_compare_queue_key {
 
 static ha_rows read_all_rows(
     THD *thd, Sort_param *param, const Prealloced_array<TABLE *, 4> &tables,
-    Filesort_info *fs_info, IO_CACHE *chunk_file, IO_CACHE *tempfile,
+    table_map tables_to_get_rowid_for, Filesort_info *fs_info,
+    IO_CACHE *chunk_file, IO_CACHE *tempfile,
     Bounded_queue<uchar *, uchar *, Sort_param, Mem_compare_queue_key> *pq,
     RowIterator *source_iterator, ha_rows *found_rows, size_t *longest_key,
     size_t *longest_addon);
@@ -355,6 +356,10 @@ static void trace_filesort_information(Opt_trace_context *trace,
   @param      thd            Current thread
   @param      filesort       How to sort the table
   @param      source_iterator Where to read the rows to be sorted from.
+  @param      tables_to_get_rowid_for
+                             Which tables we are responsible for getting row IDs
+                             for. Tables in this set that are not also in
+                             "tables" are ignored.
   @param      num_rows_estimate How many rows source_iterator is expected
                              to produce. Only used for whether we intend
                              to use the priority queue optimization or not;
@@ -374,8 +379,9 @@ static void trace_filesort_information(Opt_trace_context *trace,
 */
 
 bool filesort(THD *thd, Filesort *filesort, RowIterator *source_iterator,
-              ha_rows num_rows_estimate, Filesort_info *fs_info,
-              Sort_result *sort_result, ha_rows *found_rows) {
+              table_map tables_to_get_rowid_for, ha_rows num_rows_estimate,
+              Filesort_info *fs_info, Sort_result *sort_result,
+              ha_rows *found_rows) {
   int error;
   ulong memory_available = thd->variables.sortbuff_size;
   ha_rows num_rows_found = HA_POS_ERROR;
@@ -411,6 +417,15 @@ bool filesort(THD *thd, Filesort *filesort, RowIterator *source_iterator,
   my_b_clear(&chunk_file);
   error = 1;
 
+  if (!param->using_addon_fields()) {
+    for (TABLE *table : filesort->tables) {
+      if (table->pos_in_table_list == nullptr ||
+          (tables_to_get_rowid_for & table->pos_in_table_list->map())) {
+        table->prepare_for_position();
+      }
+    }
+  }
+
   // Make sure the source iterator is initialized before init_for_filesort(),
   // since table->file (and in particular, ref_length) may not be initialized
   // before that.
@@ -512,9 +527,9 @@ bool filesort(THD *thd, Filesort *filesort, RowIterator *source_iterator,
   {
     Opt_trace_array ota(trace, "filesort_execution");
     num_rows_found = read_all_rows(
-        thd, param, filesort->tables, fs_info, &chunk_file, &tempfile,
-        param->using_pq ? &pq : nullptr, source_iterator, found_rows,
-        &longest_key, &longest_addons);
+        thd, param, filesort->tables, tables_to_get_rowid_for, fs_info,
+        &chunk_file, &tempfile, param->using_pq ? &pq : nullptr,
+        source_iterator, found_rows, &longest_key, &longest_addons);
     if (num_rows_found == HA_POS_ERROR) goto err;
   }
 
@@ -875,6 +890,10 @@ static bool alloc_and_make_sortkey(Sort_param *param, Filesort_info *fs_info,
   @param thd               Thread handle
   @param param             Sorting parameter
   @param tables            List of all tables being sorted.
+  @param tables_to_get_rowid_for
+                           Which tables we are responsible for getting row IDs
+                           for. Tables in this set that are not also in "tables"
+                           are ignored.
   @param fs_info           Struct containing sort buffer etc.
   @param chunk_file        File to write Merge_chunks describing sorted segments
                            in tempfile.
@@ -930,7 +949,8 @@ static bool alloc_and_make_sortkey(Sort_param *param, Filesort_info *fs_info,
 
 static ha_rows read_all_rows(
     THD *thd, Sort_param *param, const Prealloced_array<TABLE *, 4> &tables,
-    Filesort_info *fs_info, IO_CACHE *chunk_file, IO_CACHE *tempfile,
+    table_map tables_to_get_rowid_for, Filesort_info *fs_info,
+    IO_CACHE *chunk_file, IO_CACHE *tempfile,
     Bounded_queue<uchar *, uchar *, Sort_param, Mem_compare_queue_key> *pq,
     RowIterator *source_iterator, ha_rows *found_rows, size_t *longest_key,
     size_t *longest_addons) {
@@ -985,7 +1005,10 @@ static ha_rows read_all_rows(
     // Note where we are, for the case where we are not using addon fields.
     if (!param->using_addon_fields()) {
       for (TABLE *table : tables) {
-        table->file->position(table->record[0]);
+        if (table->pos_in_table_list == nullptr ||
+            (table->pos_in_table_list->map() & tables_to_get_rowid_for)) {
+          table->file->position(table->record[0]);
+        }
       }
     }
     DBUG_EXECUTE_IF("debug_filesort", {

sql/filesort.h

@@ -29,6 +29,7 @@
 #include "my_base.h" /* ha_rows */
 #include "my_dbug.h"
 #include "my_inttypes.h"
+#include "my_table_map.h"
 #include "prealloced_array.h"
 #include "sql/sort_param.h"
 
@@ -96,8 +97,9 @@ class Filesort {
 };
 
 bool filesort(THD *thd, Filesort *filesort, RowIterator *source_iterator,
-              ha_rows num_rows_estimate, Filesort_info *fs_info,
-              Sort_result *sort_result, ha_rows *found_rows);
+              table_map tables_to_get_rowid_for, ha_rows num_rows_estimate,
+              Filesort_info *fs_info, Sort_result *sort_result,
+              ha_rows *found_rows);
 void filesort_free_buffers(TABLE *table, bool full);
 void change_double_for_sort(double nr, uchar *to);
 

sql/hash_join_iterator.cc

@@ -206,6 +206,11 @@ bool HashJoinIterator::Init() {
   m_probe_chunk_current_row = 0;
   m_current_chunk = -1;
 
+  PrepareForRequestRowId(m_probe_input_tables.tables(),
+                         m_tables_to_get_rowid_for);
+  PrepareForRequestRowId(m_build_input_tables.tables(),
+                         m_tables_to_get_rowid_for);
+
   // Build the hash table
   if (BuildHashTable()) {
     DBUG_ASSERT(thd()->is_error() ||
@@ -301,6 +306,16 @@ void RequestRowId(const Prealloced_array<hash_join_buffer::Table, 4> &tables,
   }
 }
 
+void PrepareForRequestRowId(
+    const Prealloced_array<hash_join_buffer::Table, 4> &tables,
+    table_map tables_to_get_rowid_for) {
+  for (const hash_join_buffer::Table &it : tables) {
+    if (tables_to_get_rowid_for & it.table->pos_in_table_list->map()) {
+      it.table->prepare_for_position();
+    }
+  }
+}
+
 // Write all the remaining rows from the given iterator out to chunk files
 // on disk. If the function returns true, an unrecoverable error occurred
 // (IO error etc.).

sql/hash_join_iterator.h

@@ -652,4 +652,8 @@ class HashJoinIterator final : public RowIterator {
 void RequestRowId(const Prealloced_array<hash_join_buffer::Table, 4> &tables,
                   table_map tables_to_get_rowid_for);
 
+void PrepareForRequestRowId(
+    const Prealloced_array<hash_join_buffer::Table, 4> &tables,
+    table_map tables_to_get_rowid_for);
+
 #endif  // SQL_HASH_JOIN_ITERATOR_H_

sql/join_optimizer/access_path.cc

@@ -38,6 +38,32 @@
 
 using std::vector;
 
+AccessPath *NewSortAccessPath(THD *thd, AccessPath *child, Filesort *filesort,
+                              bool count_examined_rows) {
+  AccessPath *path = new (thd->mem_root) AccessPath;
+  path->type = AccessPath::SORT;
+  path->count_examined_rows = count_examined_rows;
+  path->sort().child = child;
+  path->sort().filesort = filesort;
+
+  if (filesort->using_addon_fields()) {
+    path->sort().tables_to_get_rowid_for = 0;
+  } else {
+    if (filesort->tables.size() == 1 &&
+        filesort->tables[0]->pos_in_table_list == nullptr) {
+      // This can happen if we sort a single temporary table
+      // which is not in the table list (e.g., one that was
+      // specifically created for us). Filesort has special-casing
+      // to always get the row ID in this case.
+      path->sort().tables_to_get_rowid_for = 0;
+    } else {
+      FindTablesToGetRowidFor(path);
+    }
+  }
+
+  return path;
+}
+
 template <class Func>
 void WalkAccessPaths(AccessPath *path, bool cross_query_blocks, Func &&func) {
   if (func(path)) {
@@ -542,8 +568,9 @@ unique_ptr_destroy_only<RowIterator> CreateIteratorFromAccessPath(
               ? HA_POS_ERROR
               : lrint(path->sort().child->num_output_rows);
       Filesort *filesort = path->sort().filesort;
-      iterator = NewIterator<SortingIterator>(thd, filesort, move(child),
-                                              num_rows_estimate, examined_rows);
+      iterator = NewIterator<SortingIterator>(
+          thd, filesort, move(child), num_rows_estimate,
+          path->sort().tables_to_get_rowid_for, examined_rows);
       if (filesort->m_remove_duplicates) {
         filesort->tables[0]->duplicate_removal_iterator =
             down_cast<SortingIterator *>(iterator->real_iterator());
@@ -794,6 +821,12 @@ void FindTablesToGetRowidFor(AccessPath *path) {
       path->weedout().tables_to_get_rowid_for =
           GetUsedTables(path) & ~handled_by_others;
       break;
+    case AccessPath::SORT:
+      WalkAccessPaths(path, /*cross_query_blocks=*/false,
+                      add_tables_handled_by_others);
+      path->sort().tables_to_get_rowid_for =
+          GetUsedTables(path) & ~handled_by_others;
+      break;
     default:
       abort();
   }

sql/join_optimizer/access_path.h

@@ -592,6 +592,7 @@ struct AccessPath {
     struct {
       AccessPath *child;
       Filesort *filesort;
+      table_map tables_to_get_rowid_for;
     } sort;
     struct {
       AccessPath *child;
@@ -894,16 +895,10 @@ inline AccessPath *NewFilterAccessPath(THD *thd, AccessPath *child,
   return path;
 }
 
-inline AccessPath *NewSortAccessPath(THD *thd, AccessPath *child,
-                                     Filesort *filesort,
-                                     bool count_examined_rows) {
-  AccessPath *path = new (thd->mem_root) AccessPath;
-  path->type = AccessPath::SORT;
-  path->count_examined_rows = count_examined_rows;
-  path->sort().child = child;
-  path->sort().filesort = filesort;
-  return path;
-}
+// Not inline, because it needs access to filesort internals
+// (which are forward-declared in this file).
+AccessPath *NewSortAccessPath(THD *thd, AccessPath *child, Filesort *filesort,
+                              bool count_examined_rows);
 
 inline AccessPath *NewAggregateAccessPath(THD *thd, AccessPath *child,
                                           Temp_table_param *temp_table_param,
@@ -1138,7 +1133,7 @@ inline AccessPath *NewInvalidatorAccessPath(THD *thd, AccessPath *child,
   return path;
 }
 
-void FindTablesToGetRowidFor(AccessPath *weedout_path);
+void FindTablesToGetRowidFor(AccessPath *path);
 unique_ptr_destroy_only<RowIterator> CreateIteratorFromAccessPath(
     THD *thd, AccessPath *path, JOIN *join, bool eligible_for_batch_mode);
 void SetCostOnTableAccessPath(const Cost_model_server &cost_model,

sql/sorting_iterator.cc

@@ -397,11 +397,13 @@ void SortBufferIndirectIterator::SetNullRowFlag(bool is_null_row) {
 SortingIterator::SortingIterator(THD *thd, Filesort *filesort,
                                  unique_ptr_destroy_only<RowIterator> source,
                                  ha_rows num_rows_estimate,
+                                 table_map tables_to_get_rowid_for,
                                  ha_rows *examined_rows)
     : RowIterator(thd),
       m_filesort(filesort),
       m_source_iterator(move(source)),
       m_num_rows_estimate(num_rows_estimate),
+      m_tables_to_get_rowid_for(tables_to_get_rowid_for),
       m_examined_rows(examined_rows) {}
 
 SortingIterator::~SortingIterator() {
@@ -510,9 +512,9 @@ int SortingIterator::DoSort() {
                             MYF(MY_WME | MY_ZEROFILL));
 
   ha_rows found_rows;
-  bool error =
-      filesort(thd(), m_filesort, m_source_iterator.get(), m_num_rows_estimate,
-               &m_fs_info, &m_sort_result, &found_rows);
+  bool error = filesort(thd(), m_filesort, m_source_iterator.get(),
+                        m_tables_to_get_rowid_for, m_num_rows_estimate,
+                        &m_fs_info, &m_sort_result, &found_rows);
   for (TABLE *table : m_filesort->tables) {
     table->set_keyread(false);  // Restore if we used indexes
   }

sql/sorting_iterator.h

@@ -29,6 +29,7 @@
 
 #include "my_alloc.h"
 #include "my_base.h"
+#include "my_table_map.h"
 #include "sql/basic_row_iterators.h"
 #include "sql/row_iterator.h"
 #include "sql/sql_sort.h"
@@ -64,7 +65,8 @@ class SortingIterator final : public RowIterator {
   // RAM, we never use the priority queue.
   SortingIterator(THD *thd, Filesort *filesort,
                   unique_ptr_destroy_only<RowIterator> source,
-                  ha_rows num_rows_estimate, ha_rows *examined_rows);
+                  ha_rows num_rows_estimate, table_map tables_to_get_rowid_for,
+                  ha_rows *examined_rows);
   ~SortingIterator() override;
 
   // Calls Init() on the source iterator, then does the actual sort.
@@ -128,6 +130,7 @@ class SortingIterator final : public RowIterator {
   Sort_result m_sort_result;
 
   const ha_rows m_num_rows_estimate;
+  const table_map m_tables_to_get_rowid_for;
   ha_rows *m_examined_rows;
 
   // Holds one out of all RowIterator implementations we need so that it is