fix keep order in table scan is lost when remote read of PartitionTableScan(release-7.1) (#7526)

close #7519, close #7527

Author: Lloyd-Pottiger

dbms/src/Flash/Coprocessor/TiDBTableScan.cpp

@@ -29,8 +29,10 @@ TiDBTableScan::TiDBTableScan(
     // Only No-partition table need keep order when tablescan executor required keep order.
     // If keep_order is not set, keep order for safety.
     , keep_order(!is_partition_table_scan && (table_scan->tbl_scan().keep_order() || !table_scan->tbl_scan().has_keep_order()))
-    , is_fast_scan(table_scan->tbl_scan().is_fast_scan())
+    , is_fast_scan(is_partition_table_scan ? table_scan->partition_table_scan().is_fast_scan() : table_scan->tbl_scan().is_fast_scan())
 {
+    RUNTIME_CHECK_MSG(!keep_order || pushed_down_filters.empty(), "Bad TiDB table scan executor: push down filter is not empty when keep order is true");
+
     if (is_partition_table_scan)
     {
         if (table_scan->partition_table_scan().has_table_id())
@@ -79,6 +81,7 @@ void TiDBTableScan::constructTableScanForRemoteRead(tipb::TableScan * tipb_table
         for (auto id : partition_table_scan.primary_prefix_column_ids())
             tipb_table_scan->add_primary_prefix_column_ids(id);
         tipb_table_scan->set_is_fast_scan(partition_table_scan.is_fast_scan());
+        tipb_table_scan->set_keep_order(false);
     }
     else
     {

dbms/src/Storages/DeltaMerge/DeltaMergeStore.cpp

@@ -1020,7 +1020,10 @@ static ReadMode getReadModeImpl(const Context & db_context, bool is_fast_scan, b
 ReadMode DeltaMergeStore::getReadMode(const Context & db_context, bool is_fast_scan, bool keep_order, const PushDownFilterPtr & filter)
 {
     auto read_mode = getReadModeImpl(db_context, is_fast_scan, keep_order);
-    RUNTIME_CHECK_MSG(!filter || !filter->before_where || read_mode == ReadMode::Bitmap, "Push down filters needs bitmap");
+    RUNTIME_CHECK_MSG(!filter || !filter->before_where || read_mode == ReadMode::Bitmap,
+                      "Push down filters needs bitmap, push down filters is empty: {}, read mode: {}",
+                      filter == nullptr || filter->before_where == nullptr,
+                      magic_enum::enum_name(read_mode));
     return read_mode;
 }
 
@@ -1050,11 +1053,13 @@ BlockInputStreams DeltaMergeStore::read(const Context & db_context,
     SegmentReadTasks tasks = getReadTasksByRanges(*dm_context, sorted_ranges, num_streams, read_segments, /*try_split_task =*/!enable_read_thread);
     auto log_tracing_id = getLogTracingId(*dm_context);
     auto tracing_logger = log->getChild(log_tracing_id);
-    LOG_DEBUG(tracing_logger,
-              "Read create segment snapshot done, keep_order={} dt_enable_read_thread={} enable_read_thread={}",
-              keep_order,
-              db_context.getSettingsRef().dt_enable_read_thread,
-              enable_read_thread);
+    LOG_INFO(tracing_logger,
+             "Read create segment snapshot done, keep_order={} dt_enable_read_thread={} enable_read_thread={} is_fast_scan={} is_push_down_filter_empty={}",
+             keep_order,
+             db_context.getSettingsRef().dt_enable_read_thread,
+             enable_read_thread,
+             is_fast_scan,
+             filter == nullptr || filter->before_where == nullptr);
 
     auto after_segment_read = [&](const DMContextPtr & dm_context_, const SegmentPtr & segment_) {
         // TODO: Update the tracing_id before checkSegmentUpdate?

tests/fullstack-test/issues/issue_7519.test

@@ -0,0 +1,88 @@
+# Copyright 2023 PingCAP, Ltd.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# Preparation.
+=> DBGInvoke __init_fail_point()
+
+mysql> drop table if exists test.t
+mysql> create table test.t (x int, a varchar(50), y int, t time) partition by range (x) (partition p0 values less than (5), partition p1 values less than (10));
+
+mysql> insert into test.t values (1, 'a', 1, '700:11:11.1234'), (2, 'b', 2, '711:12:12.1234');
+mysql> insert into test.t select * from test.t;
+mysql> insert into test.t select * from test.t;
+mysql> insert into test.t select * from test.t;
+mysql> insert into test.t select * from test.t;
+mysql> insert into test.t select * from test.t;
+mysql> insert into test.t select * from test.t;
+mysql> insert into test.t select * from test.t;
+mysql> insert into test.t select * from test.t;
+mysql> insert into test.t select * from test.t;
+mysql> insert into test.t select * from test.t;
+mysql> insert into test.t select * from test.t;
+mysql> insert into test.t select * from test.t;
+mysql> insert into test.t select * from test.t;
+mysql> insert into test.t values (8, 'c', 8, '500:21:21.1234');
+
+mysql> alter table test.t set tiflash replica 1;
+func> wait_table test t
+mysql> analyze table test.t;
+
+mysql> set tidb_partition_prune_mode=dynamic; set tidb_enforce_mpp=1; select count(*) from test.t;
++----------+
+| count(*) |
++----------+
+|    16385 |
++----------+
+
+mysql> set tidb_partition_prune_mode=dynamic; set tidb_enforce_mpp=1; select * from test.t where x >= 5 and x < 10;
++------+------+------+-----------+
+| x    | a    | y    | t         |
++------+------+------+-----------+
+|    8 | c    |    8 | 500:21:21 |
++------+------+------+-----------+
+
+mysql> set tidb_partition_prune_mode=dynamic; set tidb_enforce_mpp=1; select x, a, y, hour(t) from test.t where x >= 5 and x < 10;
++------+------+------+---------+
+| x    | a    | y    | hour(t) |
++------+------+------+---------+
+|    8 | c    |    8 |     500 |
++------+------+------+---------+
+
+=> DBGInvoke __enable_fail_point(force_remote_read_for_batch_cop)
+
+mysql> set tidb_partition_prune_mode=dynamic; set tidb_enforce_mpp=1; select count(*) from test.t;
++----------+
+| count(*) |
++----------+
+|    16385 |
++----------+
+
+mysql> set tidb_partition_prune_mode=dynamic; set tidb_enforce_mpp=1; select * from test.t where x >= 5 and x < 10;
++------+------+------+-----------+
+| x    | a    | y    | t         |
++------+------+------+-----------+
+|    8 | c    |    8 | 500:21:21 |
++------+------+------+-----------+
+
+mysql> set tidb_partition_prune_mode=dynamic; set tidb_enforce_mpp=1; select x, a, y, hour(t) from test.t where x >= 5 and x < 10;
++------+------+------+---------+
+| x    | a    | y    | hour(t) |
++------+------+------+---------+
+|    8 | c    |    8 |     500 |
++------+------+------+---------+
+
+=> DBGInvoke __disable_fail_point(force_remote_read_for_batch_cop)
+
+# Clean up.
+mysql> drop table if exists test.t;