read_batch set include_deleted to false by default when reading a ver…

…sion (#1419)

Fixes #1385 
Similar to a normal read, `batch_read` now sets `include_deleted` to
false when looking for an index key via `find_index_key_for_version_id`
- removed `test_batch_read_metadata_multi` as `self._versions.items()`
has versions that have already been deleted, the test tries to lookup
the deleted versions and tries to asserts that they are found
- removed `test_batch_read_snapshot` as the `VersionStoreComparison`
creates a test sequence where it first deletes a symbol, then with
`test_batch_read_snapshot` checks that the versions in the snapshot
match the versions read with `batch_read` which is not the expected
behaviour because deleted versions will not be returned by batch read.
- similarly removed `test_batch_read_tombstoned_version_via_snapshot`
as, with `batch_read(symbol, as_of=[version])` , we shouldn't expect to
read a snapshotted version that has been deleted.


#### Checklist

<details>
  <summary>
   Checklist for code changes...
  </summary>
 
- [ ] Have you updated the relevant docstrings, documentation and
copyright notice?
- [ ] Is this contribution tested against [all ArcticDB's
features](../docs/mkdocs/docs/technical/contributing.md)?
- [ ] Do all exceptions introduced raise appropriate [error
messages](https://docs.arcticdb.io/error_messages/)?
 - [ ] Are API changes highlighted in the PR description?
- [ ] Is the PR labelled as enhancement or bug so it appears in
autogenerated release notes?
</details>

<!--
Thanks for contributing a Pull Request to ArcticDB! Please ensure you
have taken a look at:
- ArcticDB's Code of Conduct:
https://github.com/man-group/ArcticDB/blob/master/CODE_OF_CONDUCT.md
- ArcticDB's Contribution Licensing:
https://github.com/man-group/ArcticDB/blob/master/docs/mkdocs/docs/technical/contributing.md#contribution-licensing
-->

cpp/arcticdb/version/version_map_batch_methods.cpp

@@ -81,7 +81,8 @@ void StreamVersionData::do_react(const pipelines::SnapshotVersionQuery &snapshot
 
 std::optional<AtomKey> get_specific_version_from_entry(
     const std::shared_ptr<VersionMapEntry>& version_map_entry,
-    const pipelines::SpecificVersionQuery& specific_version
+    const pipelines::SpecificVersionQuery& specific_version,
+    bool include_deleted = false
     ) {
     auto signed_version_id = specific_version.version_id_;
     VersionId version_id;
@@ -101,7 +102,7 @@ std::optional<AtomKey> get_specific_version_from_entry(
             return std::nullopt;
         }
     }
-    return find_index_key_for_version_id(version_id, version_map_entry);
+    return find_index_key_for_version_id(version_id, version_map_entry, include_deleted);
 }
 
 std::optional<AtomKey> get_version_map_entry_by_timestamp(

python/tests/hypothesis/arcticdb/test_hypothesis_version_store.py

@@ -210,33 +210,10 @@ def _check_batch_read(self, syms: List[str], vers: Optional[List[int]]):
             else:
                 assert_frame_equal(data[sym].data, expected.data)
 
-    def _check_batch_read_metadata_multi(self, syms: List[str], vers: List[int]):
-        meta = self._lib.batch_read_metadata_multi(syms, as_ofs=vers)
-
-        for sym, versions in meta.items():
-            expected_vers = self._versions[sym]
-            for v in range(len(versions)):
-                expected = expected_vers[v]
-                result = versions[v]
-                assert result.symbol == sym
-                assert result.version == v
-                assert result.metadata == expected.metadata
-
     @invariant()
     def test_batch_read_latest(self):
         self._check_batch_read(list(self._visible_symbols), None)
 
-    @invariant()
-    def test_batch_read_metadata_multi(self):
-        syms = []
-        vers = []
-        for sym, versions in self._versions.items():
-            for ver in range(len(versions)):
-                syms.append(sym)
-                vers.append(ver)
-
-        self._check_batch_read_metadata_multi(syms, vers)
-
     @invariant()
     def test_list_versions_latest_only(self):
         expected = {sym: self._get_latest_undeleted_version(sym)[0] for sym in self._visible_symbols}
@@ -306,12 +283,6 @@ def test_list_versions_snapshot(self, latest_only=False):
     def test_list_versions_latest_only_snapshot(self):
         self.test_list_versions_snapshot(latest_only=True)
 
-    @rule(snap=snaps)
-    def test_batch_read_snapshot(self, snap: str):
-        snap = self._visible_snapshots.get(snap)
-        assume(snap)
-        self._check_batch_read(list(snap.sym_vers), list(snap.sym_vers.values()))
-
 
 @pytest.mark.parametrize("lib_type", ["lmdb_version_store_delayed_deletes_v1", "lmdb_version_store_delayed_deletes_v2"])
 @SLOW_TESTS_MARK

python/tests/integration/arcticdb/version_store/test_basic_version_store.py

@@ -28,7 +28,7 @@
     InternalException,
     UserInputException,
 )
-from arcticdb import QueryBuilder
+from arcticdb import QueryBuilder, ReadRequest
 from arcticdb.flattener import Flattener
 from arcticdb.version_store import NativeVersionStore
 from arcticdb.version_store._custom_normalizers import CustomNormalizer, register_normalizer
@@ -1331,18 +1331,6 @@ def test_batch_operations(object_version_store_prune_previous):
         equals(result["sym3"].data, np.arange(10))
 
 
-def test_batch_read_tombstoned_version_via_snapshot(basic_store):  # AN-285
-    lib = basic_store
-    lib.write("a", 0)
-    lib.snapshot("s")
-    lib.write("a", 1, prune_previous_version=True)
-
-    actual = lib.batch_read(["a"], as_ofs=[0])  # Other version query types are not implemented
-    assert actual["a"].data == 0
-    meta = lib.batch_read_metadata(["a"], as_ofs=[0])
-    assert meta["a"].version == 0
-
-
 def test_batch_write(basic_store_tombstone_and_sync_passive):
     lmdb_version_store = basic_store_tombstone_and_sync_passive
     multi_data = {"sym1": np.arange(8), "sym2": np.arange(9), "sym3": np.arange(10)}
@@ -2236,6 +2224,17 @@ def test_batch_read_version_doesnt_exist(basic_store):
     with pytest.raises(NoDataFoundException):
         _ = basic_store.batch_read([sym1, sym2], as_ofs=[0, 1])
 
+def test_read_batch_deleted_version_doesnt_exist(basic_store):
+    sym1 = 'mysymbol'
+    basic_store.write(sym1, 0)
+
+    basic_store.delete(sym1)
+    basic_store.write(sym1, 1)
+    with pytest.raises(NoSuchVersionException):
+        basic_store.read(sym1, as_of=0)
+
+    with pytest.raises(NoSuchVersionException):
+        basic_store.batch_read([sym1],  as_ofs=[0])
 
 def test_index_keys_start_end_index(basic_store, sym):
     idx = pd.date_range("2022-01-01", periods=100, freq="D")