From 40f59f124f453d27da30b4f023bf9a46fe58d3be Mon Sep 17 00:00:00 2001
From: Jussi Saurio <jussi.saurio@gmail.com>
Date: Sat, 8 Feb 2025 11:12:16 +0200
Subject: [PATCH 1/4] Fix comment: new -> old

---
 core/mvcc/database/mod.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/core/mvcc/database/mod.rs b/core/mvcc/database/mod.rs
index 4ae85f195..4cdd4f65e 100644
--- a/core/mvcc/database/mod.rs
+++ b/core/mvcc/database/mod.rs
@@ -534,7 +534,7 @@ impl<Clock: LogicalClock, T: Sync + Send + Clone + Debug + 'static> MvStore<Cloc
                     }
                     if let Some(TxTimestampOrID::TxID(id)) = row_version.end {
                         if id == tx_id {
-                            // New version is valid UNTIL committing transaction's end timestamp
+                            // Old version is valid UNTIL committing transaction's end timestamp
                             // See diagram on page 299: https://www.cs.cmu.edu/~15721-f24/papers/Hekaton.pdf
                             row_version.end = Some(TxTimestampOrID::Timestamp(end_ts));
                             self.insert_version_raw(

From a0460ae2d53dab12a39ce1ddcead8f0a8e3d4901 Mon Sep 17 00:00:00 2001
From: Jussi Saurio <jussi.saurio@gmail.com>
Date: Sat, 8 Feb 2025 11:18:05 +0200
Subject: [PATCH 2/4] Comment about tx visibility when deleting a row

---
 core/mvcc/database/mod.rs | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/core/mvcc/database/mod.rs b/core/mvcc/database/mod.rs
index 4cdd4f65e..b2a683cc9 100644
--- a/core/mvcc/database/mod.rs
+++ b/core/mvcc/database/mod.rs
@@ -318,8 +318,9 @@ impl<Clock: LogicalClock, T: Sync + Send + Clone + Debug + 'static> MvStore<Cloc
                     .ok_or(DatabaseError::NoSuchTransactionID(tx_id))?;
                 let tx = tx.value().read().unwrap();
                 assert_eq!(tx.state, TransactionState::Active);
-                let version_is_visible_to_current_tx = is_version_visible(&self.txs, &tx, rv);
-                if !version_is_visible_to_current_tx {
+                // A transaction cannot delete a version that it cannot see,
+                // nor can it conflict with it.
+                if !is_version_visible(&self.txs, &tx, rv) {
                     continue;
                 }
                 if is_write_write_conflict(&self.txs, &tx, rv) {

From c23d97f33ce540be9b0688f36fdc29dd21f09a09 Mon Sep 17 00:00:00 2001
From: Jussi Saurio <jussi.saurio@gmail.com>
Date: Sat, 8 Feb 2025 14:28:04 +0200
Subject: [PATCH 3/4] refactor: is_version_visible() ->
 RowVersion::is_visible_to()

---
 core/mvcc/database/mod.rs   | 28 ++++++++++++++++------------
 core/mvcc/database/tests.rs |  2 +-
 2 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/core/mvcc/database/mod.rs b/core/mvcc/database/mod.rs
index b2a683cc9..51b3ee3eb 100644
--- a/core/mvcc/database/mod.rs
+++ b/core/mvcc/database/mod.rs
@@ -320,7 +320,7 @@ impl<Clock: LogicalClock, T: Sync + Send + Clone + Debug + 'static> MvStore<Cloc
                 assert_eq!(tx.state, TransactionState::Active);
                 // A transaction cannot delete a version that it cannot see,
                 // nor can it conflict with it.
-                if !is_version_visible(&self.txs, &tx, rv) {
+                if !rv.is_visible_to(&tx, &self.txs) {
                     continue;
                 }
                 if is_write_write_conflict(&self.txs, &tx, rv) {
@@ -367,11 +367,13 @@ impl<Clock: LogicalClock, T: Sync + Send + Clone + Debug + 'static> MvStore<Cloc
         assert_eq!(tx.state, TransactionState::Active);
         if let Some(row_versions) = self.rows.get(&id) {
             let row_versions = row_versions.value().read().unwrap();
-            for rv in row_versions.iter().rev() {
-                if is_version_visible(&self.txs, &tx, rv) {
-                    tx.insert_to_read_set(id);
-                    return Ok(Some(rv.row.clone()));
-                }
+            for rv in row_versions
+                .iter()
+                .rev()
+                .filter(|rv| rv.is_visible_to(&tx, &self.txs))
+            {
+                tx.insert_to_read_set(id);
+                return Ok(Some(rv.row.clone()));
             }
         }
         Ok(None)
@@ -764,12 +766,14 @@ pub(crate) fn is_write_write_conflict<T>(
     }
 }
 
-pub(crate) fn is_version_visible<T>(
-    txs: &SkipMap<TxID, RwLock<Transaction>>,
-    tx: &Transaction,
-    rv: &RowVersion<T>,
-) -> bool {
-    is_begin_visible(txs, tx, rv) && is_end_visible(txs, tx, rv)
+impl<T> RowVersion<T> {
+    pub fn is_visible_to(
+        &self,
+        tx: &Transaction,
+        txs: &SkipMap<TxID, RwLock<Transaction>>,
+    ) -> bool {
+        is_begin_visible(txs, tx, self) && is_end_visible(txs, tx, self)
+    }
 }
 
 fn is_begin_visible<T>(
diff --git a/core/mvcc/database/tests.rs b/core/mvcc/database/tests.rs
index b317a15d2..f3dcabca9 100644
--- a/core/mvcc/database/tests.rs
+++ b/core/mvcc/database/tests.rs
@@ -699,7 +699,7 @@ fn test_snapshot_isolation_tx_visible1() {
             },
         };
         tracing::debug!("Testing visibility of {row_version:?}");
-        is_version_visible(&txs, &current_tx, &row_version)
+        row_version.is_visible_to(&current_tx, &txs)
     };
 
     // begin visible:   transaction committed with ts < current_tx.begin_ts

From 3826c540ae1b45e25ee43fcb939c33b503b35c97 Mon Sep 17 00:00:00 2001
From: Jussi Saurio <jussi.saurio@gmail.com>
Date: Sat, 8 Feb 2025 14:47:41 +0200
Subject: [PATCH 4/4] thank you clippy, actually a nice suggestion

---
 core/mvcc/database/mod.rs | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/core/mvcc/database/mod.rs b/core/mvcc/database/mod.rs
index 51b3ee3eb..ca0425ecb 100644
--- a/core/mvcc/database/mod.rs
+++ b/core/mvcc/database/mod.rs
@@ -367,10 +367,11 @@ impl<Clock: LogicalClock, T: Sync + Send + Clone + Debug + 'static> MvStore<Cloc
         assert_eq!(tx.state, TransactionState::Active);
         if let Some(row_versions) = self.rows.get(&id) {
             let row_versions = row_versions.value().read().unwrap();
-            for rv in row_versions
+            if let Some(rv) = row_versions
                 .iter()
                 .rev()
                 .filter(|rv| rv.is_visible_to(&tx, &self.txs))
+                .next()
             {
                 tx.insert_to_read_set(id);
                 return Ok(Some(rv.row.clone()));