Bug#35813134: Optimize table value constructors in the hypergraph optimizer

Run table value constructors through the hypergraph optimizer instead
of using the hard coded path for them in the old optimizer.

This fixes bug#31949057 (TABLE VALUE CONSTRUCTOR FOLLOWED BY ORDER BY
IS NOT SORTED CORRECTLY) in the hypergraph optimizer. The bug is still
present in the old optimizer.

The patch changes JOIN::optimize() so that it passes table value
constructor query blocks to FindBestQueryPlan() in the hypergraph
optimizer. Such query blocks go through the fast path for single-table
queries. The fast path was updated so that it also accepted table-less
queries.

When adding a SORT access path for ORDER BY, we now also check if the
query is table-less, and add a STREAM access path before the sorting
if it is. This is needed because Filesort requires a record buffer,
and we need a temporary table for that when there are no other tables.

Change-Id: Ia1a0ed5ce6711c23e7a3825f602cdb3aa75478b8

Author: kahatlen

mysql-test/include/table_value_constructor.inc

@@ -0,0 +1,18 @@
+--source include/have_hypergraph.inc
+--source include/table_value_constructor.inc
+
+--echo #
+--echo # Bug#31949057: TABLE VALUE CONSTRUCTOR FOLLOWED BY ORDER BY
+--echo #               IS NOT SORTED CORRECTLY
+--echo #
+
+VALUES ROW(1,9), ROW(2,4) ORDER BY column_1;
+
+(VALUES ROW(1,2), ROW(3,4), ROW(-1,2) ORDER BY column_0) ORDER BY column_0;
+
+VALUES ROW(1, 1), ROW(10, 2), ROW(2, 3), ROW(3, 4), ROW(5, 5)
+ORDER BY 1 LIMIT 2 OFFSET 1;
+
+VALUES ROW(0, 1), ROW(4, 6), ROW(7, 5), ROW(5, 7), ROW(9, 8),
+       ROW(7, 2), ROW(7, 6), ROW(4, 5), ROW(3, 2), ROW(9, 7)
+ORDER BY 1, 2;

mysql-test/r/table_value_constructor_hypergraph.result

@@ -99,11 +99,15 @@ eval $query;
 --skip_if_hypergraph
 eval EXPLAIN FORMAT = tree $query;
 
+# This query returns wrong results with the old optimizer. The
+# hypergraph optimizer returns correct results.
 let $query = ( ( (VALUES ROW(1,1), ROW(2,2), ROW(3,3), ROW(4,4), ROW(-1,-1))
                  ORDER BY column_0 LIMIT 4)
                LIMIT 3)
              ORDER BY 1;
+--skip_if_hypergraph  # Hypergraph returns correct results.
 eval $query;
+--skip_if_hypergraph  # Different plan (actually sorts result before LIMIT).
 --replace_regex $elide_costs
 eval EXPLAIN FORMAT = tree $query;
 
@@ -187,6 +191,7 @@ ORDER BY -a LIMIT 2;
 --echo # Check explicit table and table value constructor
 --echo #
 (TABLE r ORDER BY a LIMIT 5) ORDER BY -a LIMIT 4;
+--skip_if_hypergraph  # Wrong result with the old optimizer. Correct with hypergraph.
 VALUES ROW(1,2),ROW(3,4),ROW(-1,2) ORDER BY 1;
 (VALUES ROW(1,2),ROW(3,4),ROW(-1,2) ORDER BY 1) ORDER BY 1 LIMIT 2;
 

mysql-test/t/table_value_constructor.test

@@ -10762,16 +10762,10 @@ Item_values_column::Item_values_column(THD *thd, Item *ref) : super(thd, ref) {
   fixed = true;
 }
 
-/* purecov: begin deadcode */
-
-bool Item_values_column::eq(const Item *item, bool binary_cmp) const {
-  assert(false);
-  const Item *it = item->real_item();
-  return m_value_ref && m_value_ref->eq(it, binary_cmp);
+bool Item_values_column::eq(const Item *item, bool) const {
+  return item->type() == VALUES_COLUMN_ITEM && item_name.eq(item->item_name);
 }
 
-/* purecov: end */
-
 double Item_values_column::val_real() {
   assert(fixed);
   const double tmp = m_value_ref->val_real();

mysql-test/t/table_value_constructor_hypergraph.test

@@ -40,6 +40,7 @@
 
 class Filesort_info;
 class Item;
+class Item_values_column;
 class JOIN;
 class Sort_result;
 class THD;
@@ -500,7 +501,7 @@ class TableValueConstructorIterator final : public RowIterator {
   TableValueConstructorIterator(
       THD *thd, ha_rows *examined_rows,
       const mem_root_deque<mem_root_deque<Item *> *> &row_value_list,
-      mem_root_deque<Item *> *join_fields);
+      Mem_root_array<Item_values_column *> *output_refs);
 
   bool Init() override;
   int Read() override;
@@ -521,7 +522,8 @@ class TableValueConstructorIterator final : public RowIterator {
   /// References to the row we currently want to output. When multiple rows must
   /// be output, this contains Item_values_column objects. In this case, each
   /// call to Read() will replace its current reference with the next row.
-  mem_root_deque<Item *> *const m_output_refs;
+  /// It is nullptr if there is only one row.
+  Mem_root_array<Item_values_column *> *m_output_refs;
 };
 
 /**

mysql-test/t/unary_query_term.test

@@ -905,11 +905,11 @@ int ZeroRowsAggregatedIterator::Read() {
 TableValueConstructorIterator::TableValueConstructorIterator(
     THD *thd, ha_rows *examined_rows,
     const mem_root_deque<mem_root_deque<Item *> *> &row_value_list,
-    mem_root_deque<Item *> *join_fields)
+    Mem_root_array<Item_values_column *> *output_refs)
     : RowIterator(thd),
       m_examined_rows(examined_rows),
       m_row_value_list(row_value_list),
-      m_output_refs(join_fields) {
+      m_output_refs(output_refs) {
   assert(examined_rows != nullptr);
 }
 
@@ -924,12 +924,10 @@ int TableValueConstructorIterator::Read() {
   // If the TVC has a single row, we don't create Item_values_column reference
   // objects during resolving. We will instead use the single row directly from
   // Query_block::item_list, such that we don't have to change references here.
-  if (m_row_value_list.size() != 1) {
-    auto output_refs_it = VisibleFields(*m_output_refs).begin();
+  if (m_output_refs != nullptr) {
+    Item_values_column **output_refs_it = m_output_refs->begin();
     for (const Item *value : **m_row_it) {
-      Item_values_column *ref =
-          down_cast<Item_values_column *>(*output_refs_it);
-      ++output_refs_it;
+      Item_values_column *ref = *output_refs_it++;
 
       // Ideally we would not be casting away constness here. However, as the
       // evaluation of Item objects during execution is not const (i.e. none of

sql/item.cc

@@ -23,19 +23,28 @@
 #include "sql/join_optimizer/access_path.h"
 
 #include <algorithm>
+#include <cmath>
+#include <memory>
 #include <vector>
 
+#include "mem_root_deque.h"
 #include "my_base.h"
+#include "my_dbug.h"
+#include "mysql/components/services/bits/psi_bits.h"
+#include "prealloced_array.h"
+#include "sql/field.h"
 #include "sql/filesort.h"
+#include "sql/handler.h"
 #include "sql/item_cmpfunc.h"
 #include "sql/item_func.h"
-#include "sql/item_sum.h"
+#include "sql/item_subselect.h"
 #include "sql/iterators/basic_row_iterators.h"
 #include "sql/iterators/bka_iterator.h"
 #include "sql/iterators/composite_iterators.h"
 #include "sql/iterators/delete_rows_iterator.h"
 #include "sql/iterators/hash_join_iterator.h"
 #include "sql/iterators/ref_row_iterators.h"
+#include "sql/iterators/row_iterator.h"
 #include "sql/iterators/sorting_iterator.h"
 #include "sql/iterators/timing_iterator.h"
 #include "sql/iterators/window_iterators.h"
@@ -46,6 +55,7 @@
 #include "sql/join_optimizer/relational_expression.h"
 #include "sql/join_optimizer/walk_access_paths.h"
 #include "sql/mem_root_array.h"
+#include "sql/pack_rows.h"
 #include "sql/range_optimizer/geometry_index_range_scan.h"
 #include "sql/range_optimizer/group_index_skip_scan.h"
 #include "sql/range_optimizer/group_index_skip_scan_plan.h"
@@ -56,9 +66,18 @@
 #include "sql/range_optimizer/range_optimizer.h"
 #include "sql/range_optimizer/reverse_index_range_scan.h"
 #include "sql/range_optimizer/rowid_ordered_retrieval.h"
+#include "sql/sql_array.h"
+#include "sql/sql_const.h"
+#include "sql/sql_executor.h"
+#include "sql/sql_lex.h"
+#include "sql/sql_list.h"
+#include "sql/sql_opt_exec_shared.h"
 #include "sql/sql_optimizer.h"
 #include "sql/sql_update.h"
+#include "sql/system_variables.h"
 #include "sql/table.h"
+#include "sql/visible_fields.h"
+#include "template_utils.h"
 
 using pack_rows::TableCollection;
 using std::all_of;
@@ -123,6 +142,38 @@ AccessPath *NewUpdateRowsAccessPath(THD *thd, AccessPath *child,
   return path;
 }
 
+static Mem_root_array<Item_values_column *> *GetTableValueConstructorOutputRefs(
+    MEM_ROOT *mem_root, const JOIN *join) {
+  // If the table value constructor has a single row, the values are contained
+  // directly in join->fields, and there are no Item_values_column output refs.
+  if (join->query_block->row_value_list->size() == 1) {
+    return nullptr;
+  }
+
+  auto columns = new (mem_root) Mem_root_array<Item_values_column *>(mem_root);
+  if (columns == nullptr) return nullptr;
+
+  for (Item *column : VisibleFields(*join->fields)) {
+    if (columns->push_back(down_cast<Item_values_column *>(column))) {
+      return nullptr;
+    }
+  }
+
+  return columns;
+}
+
+AccessPath *NewTableValueConstructorAccessPath(const THD *thd,
+                                               const JOIN *join) {
+  AccessPath *path = new (thd->mem_root) AccessPath;
+  path->type = AccessPath::TABLE_VALUE_CONSTRUCTOR;
+  // The iterator keeps track of which row it is at in examined_rows,
+  // so we always need to give it the pointer.
+  path->count_examined_rows = true;
+  path->table_value_constructor().output_refs =
+      GetTableValueConstructorOutputRefs(thd->mem_root, join);
+  return path;
+}
+
 static AccessPath *FindSingleAccessPathOfType(AccessPath *path,
                                               AccessPath::Type type) {
   AccessPath *found_path = nullptr;
@@ -721,7 +772,7 @@ unique_ptr_destroy_only<RowIterator> CreateIteratorFromAccessPath(
         Query_block *query_block = join->query_block;
         iterator = NewIterator<TableValueConstructorIterator>(
             thd, mem_root, examined_rows, *query_block->row_value_list,
-            query_block->join->fields);
+            path->table_value_constructor().output_refs);
         break;
       }
       case AccessPath::FAKE_SINGLE_ROW:

sql/iterators/basic_row_iterators.h

@@ -25,33 +25,35 @@
 
 #include <assert.h>
 #include <stdint.h>
-
-#include <string>
+#include <cstddef>
 #include <type_traits>
+#include <utility>
 #include <vector>
 
-#include "sql/iterators/row_iterator.h"
+#include "my_alloc.h"
+#include "my_base.h"
+#include "my_table_map.h"
+#include "sql/item.h"
+// IWYU suggests removing row_iterator.h, but then the inlined short form of
+// CreateIteratorFromAccessPath() fails to compile. So use a pragma to keep it.
+#include "sql/iterators/row_iterator.h"  // IWYU pragma: keep
 #include "sql/join_optimizer/interesting_orders_defs.h"
 #include "sql/join_optimizer/materialize_path_parameters.h"
 #include "sql/join_optimizer/node_map.h"
 #include "sql/join_optimizer/overflow_bitset.h"
-#include "sql/join_optimizer/relational_expression.h"
 #include "sql/join_type.h"
 #include "sql/mem_root_array.h"
 #include "sql/olap.h"
-#include "sql/sql_array.h"
 #include "sql/sql_class.h"
 #include "sql/table.h"
 
-template <class T>
-class Bounds_checked_array;
-class Common_table_expr;
+class Cost_model_server;
 class Filesort;
 class HashJoinCondition;
-class Item;
 class Item_func_match;
 class JOIN;
 class KEY;
+class Query_expression;
 class QEP_TAB;
 class QUICK_RANGE;
 class SJ_TMP_TABLE;
@@ -63,10 +65,8 @@ struct GroupIndexSkipScanParameters;
 struct IndexSkipScanParameters;
 struct Index_lookup;
 struct KEY_PART;
-struct ORDER;
 struct POSITION;
 struct RelationalExpression;
-struct TABLE;
 
 /**
   A specification that two specific relational expressions
@@ -1059,7 +1059,7 @@ struct AccessPath {
     } unqualified_count;
 
     struct {
-      // No members (implicit from the JOIN).
+      Mem_root_array<Item_values_column *> *output_refs;
     } table_value_constructor;
     struct {
       // No members.
@@ -1424,14 +1424,8 @@ inline AccessPath *NewUnqualifiedCountAccessPath(THD *thd) {
   return path;
 }
 
-inline AccessPath *NewTableValueConstructorAccessPath(THD *thd) {
-  AccessPath *path = new (thd->mem_root) AccessPath;
-  path->type = AccessPath::TABLE_VALUE_CONSTRUCTOR;
-  // The iterator keeps track of which row it is at in examined_rows,
-  // so we always need to give it the pointer.
-  path->count_examined_rows = true;
-  return path;
-}
+AccessPath *NewTableValueConstructorAccessPath(const THD *thd,
+                                               const JOIN *join);
 
 inline AccessPath *NewNestedLoopSemiJoinWithDuplicateRemovalAccessPath(
     THD *thd, AccessPath *outer, AccessPath *inner, const TABLE *table,