MDEV-33875: ORDER BY DESC causes ROWID Filter slowdown

Rowid Filter cannot be used with reverse-ordered scans, for the
same reason as IndexConditionPushdown cannot be.

test_if_skip_sort_order() already has logic to disable ICP when
setting up a reverse-ordered scan. Added logic to also disable
Rowid Filter in this case, factored out the code into
prepare_for_reverse_ordered_access(), and added a comment describing
the cause of this limitation.

Author: spetrunia

mysql-test/main/rowid_filter_innodb.result

@@ -3947,3 +3947,68 @@ count(*)
 SET optimizer_switch=@save_optimizer_switch;
 DROP TABLE t0, t1;
 # End of 10.4 tests
+#
+# MDEV-33875: ORDER BY DESC causes ROWID Filter slowdown
+#
+create table t1  (
+pk int primary key auto_increment,
+a int,
+b int,
+f1 varchar(200),
+f2 varchar(200),
+f3 varchar(200),
+f4 varchar(200),
+f5 varchar(200),
+key(a, pk),
+key(b)
+) engine=innodb;
+insert into t1 (a,b,f1, f2, f3, f4) select
+seq, seq,
+repeat('1-', 100),
+repeat('2-', 100),
+repeat('3-', 100),
+repeat('4-', 100)
+from
+seq_1_to_5000;
+insert into t1 (a,b,f1, f2, f3, f4)select
+30100, 30100,
+'abcd','abcd','abcd','abcd'
+from
+seq_1_to_250;
+insert into t1 (a,b,f1) values ( 110, 100, 12345);
+analyze table t1;
+Table	Op	Msg_type	Msg_text
+test.t1	analyze	status	Engine-independent statistics collected
+test.t1	analyze	status	OK
+# The following must NOT use Rowid Filter:
+analyze format=json select * from t1
+where
+a =30100 and b in (30100,30101,30102)
+order by
+pk desc;
+ANALYZE
+{
+  "query_block": {
+    "select_id": 1,
+    "r_loops": 1,
+    "r_total_time_ms": "REPLACED",
+    "table": {
+      "table_name": "t1",
+      "access_type": "ref",
+      "possible_keys": ["a", "b"],
+      "key": "a",
+      "key_length": "5",
+      "used_key_parts": ["a"],
+      "ref": ["const"],
+      "r_loops": 1,
+      "rows": 250,
+      "r_rows": 250,
+      "r_table_time_ms": "REPLACED",
+      "r_other_time_ms": "REPLACED",
+      "filtered": 4.799086094,
+      "r_filtered": 100,
+      "attached_condition": "t1.a <=> 30100 and t1.b in (30100,30101,30102)"
+    }
+  }
+}
+drop table t1;

mysql-test/main/rowid_filter_innodb.test

@@ -751,3 +751,47 @@ SET optimizer_switch=@save_optimizer_switch;
 DROP TABLE t0, t1;
 
 --echo # End of 10.4 tests
+
+--echo #
+--echo # MDEV-33875: ORDER BY DESC causes ROWID Filter slowdown
+--echo #
+create table t1  (
+  pk int primary key auto_increment,
+  a int,
+  b int,
+  f1 varchar(200),
+  f2 varchar(200),
+  f3 varchar(200),
+  f4 varchar(200),
+  f5 varchar(200),
+  key(a, pk),
+  key(b)
+) engine=innodb;
+
+insert into t1 (a,b,f1, f2, f3, f4) select
+  seq, seq,
+  repeat('1-', 100),
+  repeat('2-', 100),
+  repeat('3-', 100),
+  repeat('4-', 100)
+from
+  seq_1_to_5000;
+
+insert into t1 (a,b,f1, f2, f3, f4)select
+  30100, 30100,
+  'abcd','abcd','abcd','abcd'
+from
+  seq_1_to_250;
+insert into t1 (a,b,f1) values ( 110, 100, 12345);
+analyze table t1;
+
+--echo # The following must NOT use Rowid Filter:
+--source include/analyze-format.inc
+analyze format=json select * from t1
+where
+  a =30100 and b in (30100,30101,30102)
+order by
+  pk desc;
+
+drop table t1;
+

sql/sql_select.cc

@@ -23957,6 +23957,90 @@ void compute_part_of_sort_key_for_equals(JOIN *join, TABLE *table,
 }
 
 
+/*
+  @brief
+    This is called when switching table access to produce records
+    in reverse order.
+
+  @detail
+    - Disable "Range checked for each record" (Is this strictly necessary
+      here?)
+    - Disable Index Condition Pushdown and Rowid Filtering.
+
+  IndexConditionPushdownAndReverseScans, RowidFilteringAndReverseScans:
+  Suppose we're computing
+
+    select * from t1
+    where
+      key1 between 10 and 20 and extra_condition
+    order by key1 desc
+
+  here the range access uses a reverse-ordered scan on (1 <= key1 <= 10) and
+  extra_condition is checked by either ICP or Rowid Filtering.
+
+  Also suppose that extra_condition happens to be false for rows of t1 that
+  do not satisfy the "10 <= key1 <= 20" condition.
+
+  For forward ordered range scan, the SQL layer will make these calls:
+
+    h->read_range_first(RANGE(10 <= key1 <= 20));
+    while (h->read_range_next()) { ... }
+
+  The storage engine sees the end endpoint of "key1<=20" and can stop scanning
+  as soon as it encounters a row with key1>20.
+
+  For backward-ordered range scan, the SQL layer will make these calls:
+
+    h->index_read_map(key1=20, HA_READ_PREFIX_LAST_OR_PREV);
+    while (h->index_prev()) {
+      if (cmp_key(h->record, "key1=10" )<0)
+        break; // end of range
+      ...
+    }
+
+  Note that the check whether we've walked beyond the key=10 endpoint is
+  made at the SQL layer. The storage engine has no information about the left
+  endpoint of the interval we're scanning.  If all rows before that endpoint
+  do not satisfy ICP condition or do not pass the Rowid Filter, the storage
+  engine will enumerate the records until the table start.
+
+  In MySQL, the API is extended with set_end_range() call so that the storage
+  engine "knows" when to stop scanning.
+*/
+
+static void prepare_for_reverse_ordered_access(JOIN_TAB *tab)
+{
+  /* Cancel "Range checked for each record" */
+  if (tab->use_quick == 2)
+  {
+    tab->use_quick= 1;
+    tab->read_first_record= join_init_read_record;
+  }
+  /*
+    Cancel Pushed Index Condition, as it doesn't work for reverse scans.
+  */
+  if (tab->select && tab->select->pre_idx_push_select_cond)
+  {
+    tab->set_cond(tab->select->pre_idx_push_select_cond);
+     tab->table->file->cancel_pushed_idx_cond();
+  }
+  /*
+    The same with Rowid Filter: it doesn't work with reverse scans so cancel
+    it, too.
+  */
+  {
+    /*
+      Rowid Filter is initialized at a later stage. It is not pushed to
+      the storage engine yet:
+    */
+    DBUG_ASSERT(!tab->table->file->pushed_rowid_filter);
+    tab->range_rowid_filter_info= NULL;
+    delete tab->rowid_filter;
+    tab->rowid_filter= NULL;
+  }
+}
+
+
 /**
   Test if we can skip the ORDER BY by using an index.
 
@@ -24409,23 +24493,11 @@ test_if_skip_sort_order(JOIN_TAB *tab,ORDER *order,ha_rows select_limit,
           tab->limit= 0;
           goto use_filesort;           // Reverse sort failed -> filesort
         }
-        /*
-          Cancel Pushed Index Condition, as it doesn't work for reverse scans.
-        */
-        if (tab->select && tab->select->pre_idx_push_select_cond)
-	{
-          tab->set_cond(tab->select->pre_idx_push_select_cond);
-           tab->table->file->cancel_pushed_idx_cond();
-        }
+        prepare_for_reverse_ordered_access(tab);
+
         if (select->quick == save_quick)
           save_quick= 0;                // make_reverse() consumed it
         select->set_quick(tmp);
-        /* Cancel "Range checked for each record" */
-        if (tab->use_quick == 2)
-        {
-          tab->use_quick= 1;
-          tab->read_first_record= join_init_read_record;
-        }
       }
       else if (tab->type != JT_NEXT && tab->type != JT_REF_OR_NULL &&
                tab->ref.key >= 0 && tab->ref.key_parts <= used_key_parts)
@@ -24438,20 +24510,7 @@ test_if_skip_sort_order(JOIN_TAB *tab,ORDER *order,ha_rows select_limit,
         */
         tab->read_first_record= join_read_last_key;
         tab->read_record.read_record_func= join_read_prev_same;
-        /* Cancel "Range checked for each record" */
-        if (tab->use_quick == 2)
-        {
-          tab->use_quick= 1;
-          tab->read_first_record= join_init_read_record;
-        }
-        /*
-          Cancel Pushed Index Condition, as it doesn't work for reverse scans.
-        */
-        if (tab->select && tab->select->pre_idx_push_select_cond)
-	{
-          tab->set_cond(tab->select->pre_idx_push_select_cond);
-           tab->table->file->cancel_pushed_idx_cond();
-        }
+        prepare_for_reverse_ordered_access(tab);
       }
     }
     else if (select && select->quick)