From b5a6b972f18fff5a45ce43e42c8f1cd0514531a9 Mon Sep 17 00:00:00 2001
From: Kirill Smelkov <kirr@nexedi.com>
Date: Thu, 11 Nov 2021 22:20:20 +0300
Subject: [PATCH] Provide loadBeforeEx

loadBeforeEx is like loadBefore, but simpler, provides better
information for object delete records and can be more efficiently
implemented by many storages: https://github.com/zopefoundation/ZODB/pull/323

On RelStorage loadBefore is currently implemented via 3 SQL queries:

  1) check whether object record exists at all
  2) retrieve object state
  3) retrieve serial of next object revision

Compared to that loadBeforeEx is implemented via only one SQL query "2"
from the above - "retrieve object state". It is exactly the same query
that loadBefore uses and after the patch loadBefore actually invokes
loadBeforeEx for step 2.

This change was outlined in

https://github.com/zopefoundation/ZODB/issues/318#issuecomment-657683419 and
https://github.com/zopefoundation/ZODB/issues/318#issuecomment-657685745

and as explained in the first link this patch is also semantically coupled with

https://github.com/zodb/relstorage/pull/484

This patch passes tests with both ZODB5 and with ZODB5+https://github.com/zopefoundation/ZODB/pull/323:

- when ran with ZODB5 it verifies that loadBefore implementation does
  not become broken.

- when ran with ZODB5+https://github.com/zopefoundation/ZODB/pull/323 it
  verifies that loadBeforeEx implementation is correct.

For tests to pass with
ZODB5+https://github.com/zopefoundation/ZODB/pull/323 we also need
https://github.com/zopefoundation/zc.zlibstorage/pull/11 because without
that fix zc.zlibstorage does not decompress data on loadBeforeEx.
---
 src/relstorage/storage/load.py      | 32 +++++++++++++++++++++++++----
 src/relstorage/tests/reltestbase.py |  1 +
 2 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/src/relstorage/storage/load.py b/src/relstorage/storage/load.py
index 845a939e..3dad1b7f 100644
--- a/src/relstorage/storage/load.py
+++ b/src/relstorage/storage/load.py
@@ -200,6 +200,30 @@ def loadSerial(self, oid, serial):
 
         raise self.__pke(oid, tid_int=tid_int, state=state)
 
+    @stale_aware
+    @storage_method
+    @metricmethod_sampled
+    def loadBeforeEx(self, oid, tid): # -> (state, serial)
+        """
+        Return most recent revision of oid before tid committed.
+        (see IStorageLoadBeforeEx)
+        """
+        oid_int = bytes8_to_int64(oid)
+        cursor = self.load_connection.cursor
+        state, serial_tid = self._loadBeforeEx(cursor, oid_int, tid)
+        serial = int64_to_8bytes(serial_tid)
+        return state, serial
+
+    def _loadBeforeEx(self, cursor, oid_int, tid): # -> (state, serial_tid)
+        # serves loadBeforeEx and loadBefore
+        state, start_tid = self.adapter.mover.load_before(
+            cursor, oid_int, bytes8_to_int64(tid))
+
+        if start_tid is None:
+            assert state is None
+            start_tid = 0
+
+        return state, start_tid
 
     @stale_aware
     @storage_method
@@ -225,6 +249,8 @@ def loadBefore(self, oid, tid):
 
         # TODO: This makes three separate queries, and also bypasses the cache.
         # We should be able to fix at least the multiple queries.
+        # ( this is "fixed" on recent ZODB which calls loadBeforeEx instead of
+        #   loadBefore after https://github.com/zopefoundation/ZODB/pull/323 )
 
         # In the past, we would use the store connection (only if it was already open)
         # to "allow leading dato from later transactions for conflict resolution".
@@ -242,10 +268,8 @@ def loadBefore(self, oid, tid):
         if not self.adapter.mover.exists(cursor, oid_int):
             raise self.__pke(oid, exists=False)
 
-        state, start_tid = self.adapter.mover.load_before(
-            cursor, oid_int, bytes8_to_int64(tid))
-
-        if start_tid is None:
+        state, start_tid = self._loadBeforeEx(cursor, oid_int, tid)
+        if start_tid == 0:
             return None
 
         if state is None:
diff --git a/src/relstorage/tests/reltestbase.py b/src/relstorage/tests/reltestbase.py
index 659ad159..1ccefd34 100644
--- a/src/relstorage/tests/reltestbase.py
+++ b/src/relstorage/tests/reltestbase.py
@@ -164,6 +164,7 @@ def func(*args, **kwargs):
             return func
 
         for name in (
+                'loadBeforeEx',
                 'loadBefore',
                 'load',
                 'store',