Bug #30110851: SUBQUERY INVOLVES COUNT() AGGREGATE FUNCTION PERFORMANCE REGRESSION

When determining whether to recalculate a materialized derived table for each
execution, the uncacheability of the _parent_ query block was used instead of
the rematerialize flag on the table (which is currently only true for
JSON_TABLE). This could cause unneeded rematerializations, especially when making
queries against non-merged views, leading to reduced performance.

Do note that there are situations where we do the opposite (materialize where a
derived table or CTE is only used once, so it should be streamed); a TODO has
been added.

(count() has nothing to do with it; the bug title is misleading.)

Change-Id: I2b0f20d2323b4f089f67bd0d0c29361156c97190

Author: Steinar H. Gunderson

mysql-test/r/derived_correlated.result

@@ -2057,10 +2057,12 @@ EXPLAIN
         -> Table scan on t1  (cost=0.45 rows=2)
     -> Table scan on dt3
         -> Materialize (invalidate on row from t1)
-            -> Stream results
-                -> Limit: 1 row(s)
-                    -> Stream results
-                        -> Table scan on t2  (cost=0.45 rows=2)
+            -> Table scan on dt2
+                -> Materialize
+                    -> Limit: 1 row(s)
+                        -> Table scan on dt
+                            -> Materialize
+                                -> Table scan on t2  (cost=0.45 rows=2)
 
 Warnings:
 Note	1276	Field or reference 'test.t1.a' of SELECT #4 was resolved in SELECT #1
@@ -3365,3 +3367,24 @@ id	c
 20	20,20
 30	NULL
 DROP TABLE t1, t2;
+#
+# Bug #30110851: SUBQUERY INVOLVES COUNT() AGGREGATE FUNCTION PERFORMANCE REGRESSION
+#
+CREATE TABLE t1 ( f1 INTEGER NOT NULL, f2 INTEGER NOT NULL );
+CREATE TABLE t2 ( f1 INTEGER NOT NULL, f2 INTEGER NOT NULL );
+CREATE ALGORITHM=TEMPTABLE VIEW v1 AS SELECT * FROM t1;
+CREATE ALGORITHM=TEMPTABLE VIEW v2 AS SELECT ( SELECT f2 FROM v1 WHERE v1.f1 = t2.f1 ) AS f3 FROM t2;
+EXPLAIN FORMAT=TREE SELECT * FROM v2 WHERE f3 = 3;
+EXPLAIN
+-> Index lookup on v2 using <auto_key0> (f3=3)
+    -> Materialize
+        -> Table scan on t2  (cost=0.35 rows=1)
+        -> Select #3 (subquery in projection; dependent)
+            -> Index lookup on v1 using <auto_key0> (f1=t2.f1)
+                -> Materialize
+                    -> Table scan on t1  (cost=0.35 rows=1)
+
+Warnings:
+Note	1276	Field or reference 'test.t2.f1' of SELECT #3 was resolved in SELECT #2
+DROP TABLE t1, t2;
+DROP VIEW v1, v2;

mysql-test/t/derived_correlated.test

@@ -930,3 +930,24 @@ SELECT * FROM t1 JOIN LATERAL (
 ) d0 ON (1);
 
 DROP TABLE t1, t2;
+
+--echo #
+--echo # Bug #30110851: SUBQUERY INVOLVES COUNT() AGGREGATE FUNCTION PERFORMANCE REGRESSION
+--echo #
+
+CREATE TABLE t1 ( f1 INTEGER NOT NULL, f2 INTEGER NOT NULL );
+CREATE TABLE t2 ( f1 INTEGER NOT NULL, f2 INTEGER NOT NULL );
+CREATE ALGORITHM=TEMPTABLE VIEW v1 AS SELECT * FROM t1;
+CREATE ALGORITHM=TEMPTABLE VIEW v2 AS SELECT ( SELECT f2 FROM v1 WHERE v1.f1 = t2.f1 ) AS f3 FROM t2;
+
+#
+# Both materializations here should be marked as non-rematerialize
+# (ie., not “Temporary table”). In particular, the materialization
+# in the SELECT clause should be reused for each iteration, even though
+# the index lookup against it is outer-correlated, and it is part of
+# a query block which is also itself outer-correlated.
+#
+EXPLAIN FORMAT=TREE SELECT * FROM v2 WHERE f3 = 3;
+
+DROP TABLE t1, t2;
+DROP VIEW v1, v2;

sql/composite_iterators.cc

@@ -813,7 +813,7 @@ bool MaterializeIterator::Init() {
   }
 
   if (m_unit != nullptr) {
-    m_unit->clear_corr_ctes();
+    m_unit->clear_correlated_query_blocks();
   }
 
   // If we are removing duplicates by way of a hash field

sql/item_subselect.cc

@@ -3046,8 +3046,7 @@ bool subselect_indexsubquery_engine::exec(THD *) {
       subquery doesn't go through unit::execute() or JOIN::reset(), we have to
       do manual clearing:
     */
-    item->unit->clear_corr_ctes();
-    tab->join()->clear_corr_derived_tmp_tables();
+    item->unit->clear_correlated_query_blocks();
     if (!table->materialized) {
       THD *const thd = table->in_use;
       bool err = tl->create_materialized_table(thd);

sql/parse_tree_nodes.h

@@ -343,7 +343,7 @@ class PT_common_table_expr : public Parse_tree_node {
   */
   Common_table_expr m_postparse;
 
-  friend bool SELECT_LEX_UNIT::clear_corr_ctes();
+  friend bool SELECT_LEX_UNIT::clear_correlated_query_blocks();
 };
 
 /**
@@ -419,7 +419,7 @@ class PT_with_clause : public Parse_tree_node {
   */
   const TABLE_LIST *m_most_inner_in_parsing;
 
-  friend bool SELECT_LEX_UNIT::clear_corr_ctes();
+  friend bool SELECT_LEX_UNIT::clear_correlated_query_blocks();
 };
 
 class PT_select_item_list : public PT_item_list {

sql/sql_executor.cc

@@ -1766,26 +1766,6 @@ unique_ptr_destroy_only<RowIterator> GetTableIterator(
     ConvertItemsToCopy(unit->get_field_list(),
                        qep_tab->table()->visible_field_ptr(), tmp_table_param,
                        subjoin);
-
-    bool rematerialize = qep_tab->rematerialize;
-    if (qep_tab->join()->select_lex->uncacheable &&
-        qep_tab->table_ref->common_table_expr() == nullptr) {
-      // If the query is uncacheable, we need to rematerialize it each and
-      // every time it's read. In particular, this can happen for
-      // outer-correlated derived tables.
-      //
-      // For (outer-correlated) CTEs, we don't need this check, as we already
-      // explicitly clear CTEs when we start executing the query block where
-      // it is defined (clear_corr_ctes(), called whenever we start a query
-      // block or materialize a table, takes care of this). In fact,
-      // rematerializing every time is actively harmful, as it would risk
-      // clearing out a temporary table that an outer query block is still
-      // scanning. We don't want to set it for a LATERAL derived table either,
-      // as we only want to rematerialize it when the previous tables' rows
-      // change.
-      rematerialize = true;
-    }
-
     bool copy_fields_and_items_in_materialize = true;
     if (unit->is_simple()) {
       // See if AggregateIterator already does this for us.
@@ -1805,14 +1785,21 @@ unique_ptr_destroy_only<RowIterator> GetTableIterator(
           thd, unit->release_query_blocks_to_materialize(), qep_tab->table(),
           move(qep_tab->iterator), qep_tab->table_ref->common_table_expr(),
           unit, /*subjoin=*/nullptr,
-          /*ref_slice=*/-1, rematerialize, tmp_table_param->end_write_records);
+          /*ref_slice=*/-1, qep_tab->rematerialize,
+          tmp_table_param->end_write_records);
     } else if (qep_tab->table_ref->common_table_expr() == nullptr &&
-               rematerialize && qep_tab->using_table_scan()) {
+               qep_tab->rematerialize && qep_tab->using_table_scan()) {
       // We don't actually need the materialization for anything (we would
       // just reading the rows straight out from the table, never to be used
       // again), so we can just stream records directly over to the next
       // iterator. This saves both CPU time and memory (for the temporary
       // table).
+      //
+      // NOTE: Currently, qep_tab->rematerialize is true only for JSON_TABLE.
+      // We could extend this to other situations, such as the leftmost
+      // table of the join (assuming nested loop only). The test for CTEs is
+      // also conservative; if the CTEs is defined within this join and used
+      // only once, we could still stream without losing performance.
       table_iterator = NewIterator<StreamingIterator>(
           thd, unit->release_root_iterator(), &subjoin->tmp_table_param,
           qep_tab->table(), copy_fields_and_items_in_materialize);
@@ -1821,11 +1808,11 @@ unique_ptr_destroy_only<RowIterator> GetTableIterator(
           thd, unit->release_root_iterator(), tmp_table_param, qep_tab->table(),
           move(qep_tab->iterator), qep_tab->table_ref->common_table_expr(),
           select_number, unit, /*subjoin=*/nullptr,
-          /*ref_slice=*/-1, copy_fields_and_items_in_materialize, rematerialize,
-          tmp_table_param->end_write_records);
+          /*ref_slice=*/-1, copy_fields_and_items_in_materialize,
+          qep_tab->rematerialize, tmp_table_param->end_write_records);
     }
 
-    if (!rematerialize) {
+    if (!qep_tab->rematerialize) {
       MaterializeIterator *materialize =
           down_cast<MaterializeIterator *>(table_iterator->real_iterator());
       if (qep_tab->invalidators != nullptr) {

sql/sql_lex.h

@@ -907,7 +907,7 @@ class SELECT_LEX_UNIT {
 
   bool check_materialized_derived_query_blocks(THD *thd);
 
-  bool clear_corr_ctes();
+  bool clear_correlated_query_blocks();
 
   void fix_after_pullout(SELECT_LEX *parent_select, SELECT_LEX *removed_select);
 

sql/sql_union.cc

@@ -1086,9 +1086,14 @@ bool SELECT_LEX_UNIT::explain(THD *explain_thd, const THD *query_thd) {
 }
 
 /**
-   Empties all correlated CTEs defined in the unit's WITH clause.
-*/
-bool SELECT_LEX_UNIT::clear_corr_ctes() {
+  Empties all correlated query blocks defined within the query expression;
+  that is, correlated CTEs defined in the expression's WITH clause, and
+  correlated derived tables.
+ */
+bool SELECT_LEX_UNIT::clear_correlated_query_blocks() {
+  for (SELECT_LEX *sl = first_select(); sl; sl = sl->next_select()) {
+    sl->join->clear_corr_derived_tmp_tables();
+  }
   if (!m_with_clause) return false;
   for (auto el : m_with_clause->m_list->elements()) {
     Common_table_expr &cte = el->m_postparse;
@@ -1402,7 +1407,7 @@ bool SELECT_LEX_UNIT::ExecuteIteratorQuery(THD *thd) {
   Opt_trace_array trace_steps(trace, "steps");
 
   if (is_executed()) {
-    if (clear_corr_ctes()) return true;
+    if (clear_correlated_query_blocks()) return true;
 
     // TODO(sgunders): Most of JOIN::reset() should be done in iterators.
     for (SELECT_LEX *sl = first_select(); sl; sl = sl->next_select()) {
@@ -1573,7 +1578,7 @@ bool SELECT_LEX_UNIT::execute(THD *thd) {
   }
 
   if (is_executed()) {
-    if (clear_corr_ctes()) return true;
+    if (clear_correlated_query_blocks()) return true;
     for (SELECT_LEX *sl = first_select(); sl; sl = sl->next_select()) {
       if (sl->join->is_executed()) {
         thd->lex->set_current_select(sl);