From 3467913d42fc90903cbbfd63ddde528cc3c5cf4c Mon Sep 17 00:00:00 2001
From: Thomas Goyne <tg@realm.io>
Date: Wed, 17 May 2023 10:31:23 -0700
Subject: [PATCH] Remove support for sorting TableViews which aren't up to date

This was never actually used (Results().snapshot().sort() produces a live
Results) and had a significant performance impact.
---
 src/realm/table_view.cpp      | 24 ++++++++++--------------
 test/object-store/results.cpp | 23 +++++++++++++++++++++++
 2 files changed, 33 insertions(+), 14 deletions(-)

diff --git a/src/realm/table_view.cpp b/src/realm/table_view.cpp
index 0c5e4c7cff6..3a1e43309ae 100644
--- a/src/realm/table_view.cpp
+++ b/src/realm/table_view.cpp
@@ -438,9 +438,15 @@ void TableView::sort(ColKey column, bool ascending)
 // Sort according to multiple columns, user specified order on each column
 void TableView::sort(SortDescriptor order)
 {
+    REALM_ASSERT(is_in_sync());
     m_descriptor_ordering.append_sort(std::move(order), SortDescriptor::MergeMode::prepend);
     m_descriptor_ordering.collect_dependencies(m_table.unchecked_ptr());
 
+    // Adding the new sort descriptor marked us as no longer in sync, but do_sort()
+    // will bring us back into sync
+    m_last_seen_versions.clear();
+    get_dependencies(m_last_seen_versions);
+
     do_sort(m_descriptor_ordering);
 }
 
@@ -455,6 +461,7 @@ void TableView::do_sync()
     // - Table::get_backlink_view()
     // Here we sync with the respective source.
     m_last_seen_versions.clear();
+    get_dependencies(m_last_seen_versions);
 
     if (m_collection_source) {
         m_key_values.clear();
@@ -493,8 +500,6 @@ void TableView::do_sync()
     }
 
     do_sort(m_descriptor_ordering);
-
-    get_dependencies(m_last_seen_versions);
 }
 
 void TableView::do_sort(const DescriptorOrdering& ordering)
@@ -504,22 +509,15 @@ void TableView::do_sort(const DescriptorOrdering& ordering)
     size_t sz = size();
     if (sz == 0)
         return;
+    REALM_ASSERT(is_in_sync());
 
     // Gather the current rows into a container we can use std algorithms on
-    size_t detached_ref_count = 0;
     BaseDescriptor::IndexPairs index_pairs;
     index_pairs.reserve(sz);
-    // always put any detached refs at the end of the sort
-    // FIXME: reconsider if this is the right thing to do
-    // FIXME: consider specialized implementations in derived classes
-    // (handling detached refs is not required in linkviews)
     for (size_t t = 0; t < sz; t++) {
         ObjKey key = get_key(t);
-        if (m_table->is_valid(key)) {
-            index_pairs.emplace_back(key, t);
-        }
-        else
-            ++detached_ref_count;
+        REALM_ASSERT_DEBUG(m_table->is_valid(key));
+        index_pairs.emplace_back(key, t);
     }
 
     const int num_descriptors = int(ordering.size());
@@ -541,8 +539,6 @@ void TableView::do_sort(const DescriptorOrdering& ordering)
     for (auto& pair : index_pairs) {
         m_key_values.add(pair.key_for_object);
     }
-    for (size_t t = 0; t < detached_ref_count; ++t)
-        m_key_values.add(null_key);
 }
 
 bool TableView::is_in_table_order() const
diff --git a/test/object-store/results.cpp b/test/object-store/results.cpp
index cfe6b8ef532..a0f454ac18f 100644
--- a/test/object-store/results.cpp
+++ b/test/object-store/results.cpp
@@ -3638,6 +3638,29 @@ TEST_CASE("results: snapshots") {
         REQUIRE_FALSE(snapshot.first()->is_valid());
         REQUIRE_FALSE(snapshot.last()->is_valid());
     }
+
+    SECTION("sorting a snapshot results in live Results") {
+        auto table = r->read_group().get_table("class_object");
+        write([=] {
+            table->create_object();
+        });
+
+        auto snapshot = Results(r, table).snapshot();
+        auto sorted = snapshot.sort({{"value", true}});
+        REQUIRE(snapshot.size() == 1);
+        REQUIRE(sorted.size() == 1);
+
+        write([=] {
+            table->clear();
+        });
+
+        REQUIRE(snapshot.size() == 1);
+        REQUIRE(sorted.size() == 0);
+
+        // Create a second sorted Results derived from the now-stale snapshot
+        sorted = snapshot.sort({{"value", true}});
+        REQUIRE(sorted.size() == 0);
+    }
 }
 
 TEST_CASE("results: distinct") {