Skip to content

chore: move changes from ee #2956

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

chore: move changes from ee #2956

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 16 additions & 15 deletions packages/common/universaldb/src/transaction.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::{future::Future, ops::Deref, pin::Pin, sync::Arc};

use anyhow::{Context, Result};
use futures_util::StreamExt;

Check failure on line 4 in packages/common/universaldb/src/transaction.rs

View workflow job for this annotation

GitHub Actions / Check 

unused import: `futures_util::StreamExt`

Check warning on line 4 in packages/common/universaldb/src/transaction.rs

View workflow job for this annotation

GitHub Actions / Test 

unused import: `futures_util::StreamExt`

use crate::{
driver::TransactionDriver,
Expand Down Expand Up @@ -105,6 +105,11 @@
self.driver.clear(&self.subspace.pack(key));
}

pub fn delete_subspace(&self, subspace: &Subspace) {
self.informal()
.clear_subspace_range(&self.subspace.join(&subspace));
Comment on lines +109 to +110
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There appears to be a type mismatch between the parameter types in this PR. The new delete_subspace method is using Subspace from the utils module, but the clear_subspace_range method on line 240 has been modified to accept tuple::Subspace instead of Subspace.

This creates an inconsistency where self.subspace.join(&subspace) returns a Subspace type, but clear_subspace_range now expects a tuple::Subspace.

To resolve this, either:

  1. Change clear_subspace_range to continue accepting Subspace type, or
  2. Modify the delete_subspace implementation to convert to tuple::Subspace before passing it

Maintaining type consistency will prevent potential runtime errors.

Suggested change
self.informal()
.clear_subspace_range(&self.subspace.join(&subspace));
self.informal()
.clear_subspace_range(&tuple::Subspace::from(self.subspace.join(&subspace)));

Spotted by Diamond

Fix in Graphite

Is this helpful? React 👍 or 👎 to let us know.

}

pub fn delete_key_subspace<T: TuplePack>(&self, key: &T) {
self.informal()
.clear_subspace_range(&self.subspace.subspace(&self.subspace.pack(key)));
Expand Down Expand Up @@ -176,15 +181,15 @@
self.driver.get_ranges_keyvalues(opt, isolation_level)
}

pub fn read_entries<'a, T: FormalKey + for<'de> TupleUnpack<'de>>(
&'a self,
opt: RangeOption<'a>,
isolation_level: IsolationLevel,
) -> impl futures_util::Stream<Item = Result<(T, T::Value)>> {
self.driver
.get_ranges_keyvalues(opt, isolation_level)
.map(|res| self.read_entry(&res?))
}
// TODO: Fix types
// pub fn read_entries<'a, T: FormalKey + for<'de> TupleUnpack<'de>>(
// &'a self,
// opt: RangeOption<'a>,
// isolation_level: IsolationLevel,
// ) -> impl futures_util::Stream<Item = Result<(T, T::Value)>> {
// self.read_range(opt, isolation_level)
// .map(|res| self.read_entry(&res?))
// }

// ==== TODO: Remove. all of these should only be used via `tx.informal()` ====
pub fn get<'a>(
Expand Down Expand Up @@ -232,7 +237,7 @@
self.driver.clear_range(begin, end)
}

pub fn clear_subspace_range(&self, subspace: &Subspace) {
pub fn clear_subspace_range(&self, subspace: &tuple::Subspace) {
let (begin, end) = subspace.range();
self.driver.clear_range(&begin, &end);
}
Expand Down Expand Up @@ -316,15 +321,11 @@
}

/// Clear all keys in a subspace range
pub fn clear_subspace_range(&self, subspace: &Subspace) {
pub fn clear_subspace_range(&self, subspace: &tuple::Subspace) {
let (begin, end) = subspace.range();
self.inner.driver.clear_range(&begin, &end);
}

// pub fn commit(self: Box<Self>) -> Pin<Box<dyn Future<Output = Result<()>> + Send>> {
// self.inner.driver.commit()
// }

pub fn cancel(&self) {
self.inner.driver.cancel()
}
Expand Down
3 changes: 3 additions & 0 deletions packages/common/universaldb/src/utils/keys.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,4 +122,7 @@ define_keys! {
(94, SERVERLESS, "serverless"),
(95, DESIRED_SLOTS, "desired_slots"),
(96, BY_VARIANT, "by_variant"),
(97, ACL, "acl"),
(98, TOKEN, "token"),
(99, SECRET, "secret"),
}
6 changes: 6 additions & 0 deletions packages/common/universaldb/src/utils/subspace.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@ impl Subspace {
}
}

pub fn join(&self, s: &Subspace) -> Self {
Self {
inner: tuple::Subspace::from_bytes([self.inner.bytes(), s.bytes()].concat()),
}
}

/// Returns the key encoding the specified Tuple with the prefix of this Subspace
/// prepended.
pub fn pack<T: TuplePack>(&self, t: &T) -> Vec<u8> {
Expand Down
Loading