Prevent exception for weird query range (#797)

Signed-off-by: JaySon-Huang <[email protected]> Co-authored-by: ti-srebot <[email protected]>
pingcap · Jun 18, 2020 · 85bf28a · 85bf28a
1 parent e2535fc
commit 85bf28a
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 3 deletions.
diff --git a/dbms/src/Storages/DeltaMerge/tests/gtest_dm_storage_delta_merge.cpp b/dbms/src/Storages/DeltaMerge/tests/gtest_dm_storage_delta_merge.cpp
@@ -19,6 +19,7 @@
 #include <Storages/StorageDeltaMerge.h>
 #include <Storages/StorageDeltaMergeHelpers.h>
 #include <Storages/Transaction/RegionRangeKeys.h>
+#include <Storages/Transaction/TiKVRange.h>
 #include <Storages/Transaction/TiKVRecordFormat.h>
 #include <gtest/gtest.h>
 
@@ -233,10 +234,29 @@ TEST(StorageDeltaMerge_internal_test, OverlapQueryRanges)
     region.range_in_table = std::make_pair(300, 400);
     regions.emplace_back(region);
     region.range_in_table = std::make_pair(425, 475);
+    regions.emplace_back(region);
 
+    // Overlaped ranges throw exception
     ASSERT_ANY_THROW(auto ranges = ::DB::getQueryRanges(regions));
 }
 
+TEST(StorageDeltaMerge_internal_test, WeirdRange)
+{
+    // [100, 200), [200, MAX), [MAX, MAX)
+    MvccQueryInfo::RegionsQueryInfo regions;
+    RegionQueryInfo                 region;
+    region.range_in_table = DB::HandleRange<DB::HandleID>{100, 200};
+    regions.emplace_back(region);
+    region.range_in_table = DB::HandleRange<DB::HandleID>{DB::TiKVRange::Handle::max, DB::TiKVRange::Handle::max};
+    regions.emplace_back(region);
+    region.range_in_table = DB::HandleRange<DB::HandleID>{200, TiKVRange::Handle::max};
+    regions.emplace_back(region);
+
+    auto ranges = ::DB::getQueryRanges(regions);
+    ASSERT_EQ(ranges.size(), 1UL);
+    ASSERT_RANGE_EQ(ranges[0], DB::DM::HandleRange(100, DB::DM::HandleRange::MAX));
+}
+
 } // namespace tests
 } // namespace DM
 } // namespace DB
diff --git a/dbms/src/Storages/StorageDeltaMerge.cpp b/dbms/src/Storages/StorageDeltaMerge.cpp
@@ -693,8 +693,7 @@ BlockInputStreams StorageDeltaMerge::read( //
             } // else learner read from ch-client, keep num_streams
         }
 
-        HandleRanges ranges = getQueryRanges(mvcc_query_info.regions_query_info);
-
+        String str_query_ranges;
         if (log->trace())
         {
             std::stringstream ss;
@@ -712,10 +711,18 @@ BlockInputStreams StorageDeltaMerge::read( //
                     ss << region.region_id << "[" << range.first.toString() << "," << range.second.toString() << "),";
                 }
             }
+            str_query_ranges = ss.str();
+            LOG_TRACE(log, "reading ranges: orig, " << str_query_ranges);
+        }
+
+        HandleRanges ranges = getQueryRanges(mvcc_query_info.regions_query_info);
+
+        if (log->trace())
+        {
             std::stringstream ss_merged_range;
             for (const auto & range : ranges)
                 ss_merged_range << range.toString() << ",";
-            LOG_TRACE(log, "reading ranges: orig, " << ss.str() << " merged, " << ss_merged_range.str());
+            LOG_TRACE(log, "reading ranges: orig, " << str_query_ranges << " merged, " << ss_merged_range.str());
         }
 
         /// Get Rough set filter from query

diff --git a/dbms/src/Storages/StorageDeltaMergeHelpers.h b/dbms/src/Storages/StorageDeltaMergeHelpers.h
@@ -11,6 +11,11 @@
 namespace DB
 {
 
+namespace ErrorCodes
+{
+extern const int LOGICAL_ERROR;
+}
+
 namespace
 {
 inline DB::HandleID getRangeEndID(const DB::TiKVHandle::Handle<HandleID> & end)
@@ -79,6 +84,17 @@ inline DM::HandleRanges getQueryRanges(const DB::MvccQueryInfo::RegionsQueryInfo
         const size_t region_idx = sort_index[i];
         const auto & handle_range = handle_ranges[region_idx];
 
+        if (handle_range.first.type == DB::TiKVHandle::HandleIDType::MAX)
+        {
+            // Ignore [Max, Max)
+            if (handle_range.second.type == DB::TiKVHandle::HandleIDType::MAX)
+                continue;
+            else
+                throw Exception(
+                    "Can not merge invalid region range: [" + handle_range.first.toString() + "," + handle_range.second.toString() + ")",
+                    ErrorCodes::LOGICAL_ERROR);
+        }
+
         if (i == 0)
         {
             current.start = handle_range.first.handle_id;