Skip to content

Commit 9d3f5d3

Browse files
MichaelCuevasmeta-codesync[bot]
authored andcommitted
FilteredBackingStore: improve compareObjectsById when filters differ
Summary: # This diff Fixes a bug in compareObjectsById that causes two FilteredObjectIds to be considered different if their filters are not bytewise equal (despite being semantically equal). Only one line of code needs to change -- we check if filters are identical rather than checking if they're bytewise equal. Reviewed By: vilatto Differential Revision: D87029066 fbshipit-source-id: 2ee80a9cdc49d148a9ab2569f131b522034261a1
1 parent ab6bf9e commit 9d3f5d3

2 files changed

Lines changed: 9 additions & 6 deletions

File tree

eden/fs/store/FilteredBackingStore.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,8 @@ ObjectComparison FilteredBackingStore::compareObjectsById(
163163
typeTwo == FilteredObjectIdType::OBJECT_TYPE_TREE) {
164164
// If the filters are the same, then we can simply check whether the
165165
// underlying ObjectIds resolve to equal.
166-
if (filteredOne.filter() == filteredTwo.filter()) {
166+
if (filter_->areFiltersIdentical(
167+
filteredOne.filter(), filteredTwo.filter())) {
167168
return backingStore_->compareObjectsById(
168169
filteredOne.object(), filteredTwo.object());
169170
}

eden/fs/store/test/FilteredBackingStoreTest.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -826,13 +826,15 @@ TEST_F(FakeSubstringFilteredBackingStoreTest, testCompareTreeObjectsById) {
826826
filteredStore_->compareObjectsById(grandchildOID, grandchildOID2) ==
827827
ObjectComparison::Unknown);
828828

829-
// FIXME: These two trees have the same underlying tree (grandchildTreeId) and
830-
// semantically identical filters. They should be Identical, but this will
831-
// fail because compareObjectsById does a bytewise comparison instead of
832-
// calling areFiltersIdentical.
829+
// These two trees have the same underlying tree (grandchildTreeId) and
830+
// bytewise different (but semantically identical) filters. They should
831+
// be Identical despite the bytewise filter difference.
832+
auto grandchildFIDLegacy = FilteredObjectId::fromObjectId(grandchildOID);
833+
auto grandchildFIDV1 = FilteredObjectId::fromObjectId(grandchildOID1V1);
834+
EXPECT_NE(grandchildFIDLegacy.filter(), grandchildFIDV1.filter());
833835
EXPECT_EQ(
834836
filteredStore_->compareObjectsById(grandchildOID, grandchildOID1V1),
835-
ObjectComparison::Unknown);
837+
ObjectComparison::Identical);
836838
}
837839

838840
TEST_F(FakeSubstringFilteredBackingStoreTest, getGlobFiles) {

0 commit comments

Comments
 (0)