From d11584e42e75d1a785286e2ecc5c8445042f42ac Mon Sep 17 00:00:00 2001 From: Yu Zhang Date: Wed, 31 Jan 2024 11:12:52 -0800 Subject: [PATCH] Be consistent in key range overlap check (#12315) Summary: We should be consistent in how we check key range overlap in memtables and in sst files. While all the sst file key range overlap check compares the user key without timestamp, for example: https://github.com/facebook/rocksdb/blob/377eee77f8da3f5d232cf014db0c4ca232352883/db/version_set.cc#L129-L130 This key range overlap check for memtable is comparing the whole user key. Currently it happen to achieve the same effect because this function is only called by `ExternalSstFileIngestionJob` and `DBImpl::CompactRange`, which takes a user key without timestamp as the range end, pad a max or min timestamp to it depending on whether the end is exclusive. So use `Compartor::Compare` here is working too, but we should update it to `Comparator::CompareWithoutTimestamp` to be consistent with all the other file key range overlapping check functions. Pull Request resolved: https://github.com/facebook/rocksdb/pull/12315 Test Plan: existing tests Reviewed By: ltamasi Differential Revision: D53273456 Pulled By: jowlyzhang fbshipit-source-id: c094ae1f0c195d52542124c4fb03fdca14241e85 --- db/column_family.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/db/column_family.cc b/db/column_family.cc index 5ab992e13f5..94ca034ef96 100644 --- a/db/column_family.cc +++ b/db/column_family.cc @@ -1223,7 +1223,8 @@ Status ColumnFamilyData::RangesOverlapWithMemtables( if (status.ok()) { if (memtable_iter->Valid() && - ucmp->Compare(seek_result.user_key, ranges[i].limit) <= 0) { + ucmp->CompareWithoutTimestamp(seek_result.user_key, + ranges[i].limit) <= 0) { *overlap = true; } else if (range_del_agg.IsRangeOverlapped(ranges[i].start, ranges[i].limit)) {