Bug #31588831: ASSERTION FAILURE IN NEWWEEDOUTACCESSPATHFORTABLES() AT SQL/SQL_EXECUTOR.CC

After Bug #31310238 (SUPPORT SORTING MULTIPLE TABLES), we would call
Filesort::using_addon_fields() fairly early, which decides whether
to use addon fields or not on the first call. This is problematic if
we sort a single table; we would need to know at that point whether
the table is under a weedout operation or not (because weedout forces
row IDs).

We already had logic in place to check this; however, ConnectJoins()
can add expand a weedout operation after-the-fact if there are non-hierarchical
weedouts present in the query. If so, we need to undo the addon field
decision, or we could be without row IDs for the table in question.

Change-Id: Iefd05afd768987565ec6a16b6e02287188b9e522

Author: Steinar H. Gunderson

Diff for: mysql-test/r/filesort.result

@@ -108,3 +108,34 @@ EXPLAIN
 
 DROP TABLE t1;
 SET sql_mode=DEFAULT;
+#
+# Bug #31588831: ASSERTION FAILURE IN NEWWEEDOUTACCESSPATHFORTABLES() AT SQL/SQL_EXECUTOR.CC
+#
+CREATE TABLE t1 (pk INTEGER, f2 INTEGER, PRIMARY KEY (pk));
+CREATE TABLE t2 (pk INTEGER, f2 INTEGER, PRIMARY KEY (pk));
+INSERT INTO t1 VALUES (1,1), (2,2);
+ANALYZE TABLE t1, t2;
+Table	Op	Msg_type	Msg_text
+test.t1	analyze	status	OK
+test.t2	analyze	status	OK
+EXPLAIN FORMAT=tree SELECT * FROM
+t1
+LEFT JOIN t1 AS table2 ON 119 IN (
+SELECT SUBQUERY1_t2.pk
+FROM t1 AS SUBQUERY1_t1, (t2 AS SUBQUERY1_t2 STRAIGHT_JOIN t1 ON TRUE)
+)
+ORDER BY t1.f2;
+EXPLAIN
+-> Remove duplicate (t1, table2) rows using temporary table (weedout)
+    -> Nested loop left join
+        -> Sort row IDs: t1.f2
+            -> Table scan on t1
+        -> Nested loop inner join
+            -> Nested loop inner join
+                -> Nested loop inner join
+                    -> Constant row from SUBQUERY1_t2
+                    -> Table scan on table2
+                -> Index scan on SUBQUERY1_t1 using PRIMARY
+            -> Index scan on t1 using PRIMARY
+
+DROP TABLE t1, t2;

Diff for: mysql-test/t/filesort.test

@@ -97,3 +97,26 @@ eval EXPLAIN FORMAT=tree $query;
 
 DROP TABLE t1;
 SET sql_mode=DEFAULT;
+
+--echo #
+--echo # Bug #31588831: ASSERTION FAILURE IN NEWWEEDOUTACCESSPATHFORTABLES() AT SQL/SQL_EXECUTOR.CC
+--echo #
+
+CREATE TABLE t1 (pk INTEGER, f2 INTEGER, PRIMARY KEY (pk));
+CREATE TABLE t2 (pk INTEGER, f2 INTEGER, PRIMARY KEY (pk));
+
+INSERT INTO t1 VALUES (1,1), (2,2);
+ANALYZE TABLE t1, t2;
+
+# Sets up a filesort on t1 which initially is not part of weedout,
+# but gets caught in one anyway later, and thus needs to be sorted by row ID.
+--replace_regex / *\(cost=\d+\.\d+\ rows=\d+\)//
+EXPLAIN FORMAT=tree SELECT * FROM
+  t1
+  LEFT JOIN t1 AS table2 ON 119 IN (
+    SELECT SUBQUERY1_t2.pk
+    FROM t1 AS SUBQUERY1_t1, (t2 AS SUBQUERY1_t2 STRAIGHT_JOIN t1 ON TRUE)
+  )
+  ORDER BY t1.f2;
+
+DROP TABLE t1, t2;

Diff for: sql/filesort.cc

@@ -215,6 +215,11 @@ void Sort_param::decide_addon_fields(Filesort *file_sort,
   }
 }
 
+void Sort_param::clear_addon_fields() {
+  m_addon_fields_status = Addon_fields_status::unknown_status;
+  addon_fields = nullptr;
+}
+
 void Sort_param::init_for_filesort(Filesort *file_sort,
                                    Bounds_checked_array<st_sort_field> sf_array,
                                    uint sortlen,
@@ -2263,6 +2268,8 @@ bool Filesort::using_addon_fields() {
   return m_sort_param.using_addon_fields();
 }
 
+void Filesort::clear_addon_fields() { m_sort_param.clear_addon_fields(); }
+
 /*
 ** functions to change a double or float to a sortable string
 ** The following should work for IEEE

Diff for: sql/filesort.h

@@ -89,6 +89,10 @@ class Filesort {
   /// order is properly set up.
   bool using_addon_fields();
 
+  /// Reset the decision made in using_addon_fields(). Only used in exceptional
+  /// circumstances (see NewWeedoutAccessPathForTables()).
+  void clear_addon_fields();
+
  private:
   /* Prepare ORDER BY list for sorting. */
   uint make_sortorder(ORDER *order, bool unwrap_rollup);

Diff for: sql/sort_param.h

@@ -328,6 +328,10 @@ class Sort_param {
                            const Prealloced_array<TABLE *, 4> &tables,
                            bool sort_positions);
 
+  /// Reset the decision made in decide_addon_fields(). Only used in exceptional
+  /// circumstances (see NewWeedoutAccessPathForTables()).
+  void clear_addon_fields();
+
   /**
     Initialize this struct for filesort() usage.
     @see description of record layout above

Diff for: sql/sql_executor.cc

@@ -1270,8 +1270,15 @@ static AccessPath *NewWeedoutAccessPathForTables(
       // See JOIN::add_sorting_to_table() for rationale.
       Filesort *filesort = qep_tabs[i].filesort;
       if (filesort != nullptr) {
-        DBUG_ASSERT(filesort->m_sort_param.m_addon_fields_status ==
-                    Addon_fields_status::unknown_status);
+        if (filesort->m_sort_param.m_addon_fields_status !=
+            Addon_fields_status::unknown_status) {
+          // This can happen in the exceptional case that there's an extra
+          // weedout added after-the-fact due to nonhierarchical weedouts
+          // (see FindSubstructure for details). Note that our caller will
+          // call FindTablesToGetRowidFor() if needed, which should overwrite
+          // the previous (now wrong) decision there.
+          filesort->clear_addon_fields();
+        }
         filesort->m_force_sort_positions = true;
       }
     }