Skip to content

Commit

Permalink
Impose FilterableValue in UniqueColumn::find (#2267)
Browse files Browse the repository at this point in the history
  • Loading branch information
Centril authored Feb 18, 2025
1 parent bf82614 commit 65ec786
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 7 deletions.
17 changes: 15 additions & 2 deletions crates/bindings/src/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,10 @@ impl<Tbl: Table, Col: Index + Column<Table = Tbl>> UniqueColumn<Tbl, Col::ColTyp
// whereas by-ref makes passing `!Copy` fields more performant.
// Can we do something smart with `std::borrow::Borrow`?
#[inline]
pub fn find(&self, col_val: impl Borrow<Col::ColType>) -> Option<Tbl::Row> {
pub fn find(&self, col_val: impl Borrow<Col::ColType>) -> Option<Tbl::Row>
where
for<'a> &'a Col::ColType: FilterableValue,
{
self._find(col_val.borrow())
}

Expand Down Expand Up @@ -407,6 +410,10 @@ impl<Tbl: Table, IndexType, Idx: Index> RangedIndex<Tbl, IndexType, Idx> {
}
}

mod private_filtrable_value {
pub trait Sealed {}
}

/// Types which can appear as an argument to an index filtering operation
/// for a column of type `Column`.
///
Expand All @@ -427,12 +434,18 @@ impl<Tbl: Table, IndexType, Idx: Index> RangedIndex<Tbl, IndexType, Idx> {
/// for any pair of types `(Arg, Col)` which meet the above criteria
/// is desirable if `Arg` and `Col` have the same BSATN layout.
/// E.g. `&str: FilterableValue<Column = String>` is desirable.
pub trait FilterableValue: Serialize {
#[diagnostic::on_unimplemented(
message = "`{Self}` cannot appear as an argument to an index filtering operation",
label = "should be an integer type, `bool`, `String`, `&str`, `Identity`, `ConnectionId`, or `Hash`, not `{Self}`",
note = "The allowed set of types are limited to integers, bool, strings, `Identity`, `ConnectionId`, and `Hash`"
)]
pub trait FilterableValue: Serialize + private_filtrable_value::Sealed {
type Column;
}

macro_rules! impl_filterable_value {
(@one $arg:ty => $col:ty) => {
impl private_filtrable_value::Sealed for $arg {}
impl FilterableValue for $arg {
type Column = $col;
}
Expand Down
18 changes: 18 additions & 0 deletions crates/bindings/tests/ui/tables.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,22 @@ struct TypeParam<T> {
#[spacetimedb::table(name = const_param)]
struct ConstParam<const X: u8> {}

#[derive(spacetimedb::SpacetimeType)]
enum Alpha { Beta, Gamma }

#[spacetimedb::table(name = delta)]
struct Delta {
#[unique]
#[index(btree)]
compound_a: Alpha,
#[index(btree)]
compound_b: Alpha,
}

#[spacetimedb::reducer]
fn bad_filter_on_index(ctx: &spacetimedb::ReducerContext) {
ctx.db.delta().compound_a().find(Alpha::Beta);
ctx.db.delta().compound_b().filter(Alpha::Gamma);
}

fn main() {}
65 changes: 60 additions & 5 deletions crates/bindings/tests/ui/tables.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ error[E0277]: the column type `Test` does not implement `SpacetimeType`
&T
()
AlgebraicTypeRef
Alpha
Arc<T>
ArrayType
Box<T>
ColId
ColumnAttribute
and $N others
= note: required for `Test` to implement `TableColumn`

Expand All @@ -41,11 +41,11 @@ error[E0277]: the trait bound `Test: SpacetimeType` is not satisfied
&T
()
AlgebraicTypeRef
Alpha
Arc<T>
ArrayType
Box<T>
ColId
ColumnAttribute
and $N others

error[E0277]: the trait bound `Test: Deserialize<'de>` is not satisfied
Expand All @@ -62,10 +62,10 @@ error[E0277]: the trait bound `Test: Deserialize<'de>` is not satisfied
&'de str
()
AlgebraicTypeRef
Alpha
Arc<[T]>
ArrayType
Box<T>
Box<[T]>
and $N others
note: required by a bound in `spacetimedb::spacetimedb_lib::de::SeqProductAccess::next_element`
--> $WORKSPACE/crates/sats/src/de.rs
Expand All @@ -84,10 +84,10 @@ error[E0277]: the trait bound `Test: Deserialize<'_>` is not satisfied
&'de str
()
AlgebraicTypeRef
Alpha
Arc<[T]>
ArrayType
Box<T>
Box<[T]>
and $N others
note: required by a bound in `get_field_value`
--> $WORKSPACE/crates/sats/src/de.rs
Expand All @@ -105,14 +105,69 @@ error[E0277]: the trait bound `Test: Serialize` is not satisfied
&T
()
AlgebraicTypeRef
Alpha
Arc<T>
ArrayType
ArrayValue
Bound<T>
Box<T>
and $N others
note: required by a bound in `spacetimedb::spacetimedb_lib::ser::SerializeNamedProduct::serialize_element`
--> $WORKSPACE/crates/sats/src/ser.rs
|
| fn serialize_element<T: Serialize + ?Sized>(&mut self, name: Option<&str>, elem: &T) -> Result<(), Self::Error>;
| ^^^^^^^^^ required by this bound in `SerializeNamedProduct::serialize_element`

error[E0277]: `&'a Alpha` cannot appear as an argument to an index filtering operation
--> tests/ui/tables.rs:30:33
|
30 | ctx.db.delta().compound_a().find(Alpha::Beta);
| ^^^^ should be an integer type, `bool`, `String`, `&str`, `Identity`, `ConnectionId`, or `Hash`, not `&'a Alpha`
|
= help: the trait `for<'a> FilterableValue` is not implemented for `&'a Alpha`
= note: The allowed set of types are limited to integers, bool, strings, `Identity`, `ConnectionId`, and `Hash`
= help: the following other types implement trait `FilterableValue`:
&ConnectionId
&Identity
&bool
&ethnum::int::I256
&ethnum::uint::U256
&i128
&i16
&i32
and $N others
note: required by a bound in `UniqueColumn::<Tbl, <Col as spacetimedb::table::Column>::ColType, Col>::find`
--> src/table.rs
|
| pub fn find(&self, col_val: impl Borrow<Col::ColType>) -> Option<Tbl::Row>
| ---- required by a bound in this associated function
| where
| for<'a> &'a Col::ColType: FilterableValue,
| ^^^^^^^^^^^^^^^ required by this bound in `UniqueColumn::<Tbl, <Col as Column>::ColType, Col>::find`

error[E0277]: the trait bound `Alpha: IndexScanRangeBounds<(Alpha,), SingleBound>` is not satisfied
--> tests/ui/tables.rs:31:40
|
31 | ctx.db.delta().compound_b().filter(Alpha::Gamma);
| ------ ^^^^^^^^^^^^ the trait `FilterableValue` is not implemented for `Alpha`
| |
| required by a bound introduced by this call
|
= help: the following other types implement trait `FilterableValue`:
&ConnectionId
&Identity
&bool
&ethnum::int::I256
&ethnum::uint::U256
&i128
&i16
&i32
and $N others
= note: required for `Alpha` to implement `IndexScanRangeBounds<(Alpha,), SingleBound>`
note: required by a bound in `RangedIndex::<Tbl, IndexType, Idx>::filter`
--> src/table.rs
|
| pub fn filter<B, K>(&self, b: B) -> impl Iterator<Item = Tbl::Row>
| ------ required by a bound in this associated function
| where
| B: IndexScanRangeBounds<IndexType, K>,
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `RangedIndex::<Tbl, IndexType, Idx>::filter`

2 comments on commit 65ec786

@github-actions
Copy link

@github-actions github-actions bot commented on 65ec786 Feb 18, 2025

Choose a reason for hiding this comment

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

Criterion benchmark results

Error when comparing benchmarks: Couldn't find AWS credentials in environment, credentials file, or IAM role.

Caused by:
Couldn't find AWS credentials in environment, credentials file, or IAM role.

@github-actions
Copy link

@github-actions github-actions bot commented on 65ec786 Feb 18, 2025

Choose a reason for hiding this comment

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

Callgrind benchmark results Error when comparing benchmarks: Couldn't find AWS credentials in environment, credentials file, or IAM role.

Caused by:
Couldn't find AWS credentials in environment, credentials file, or IAM role.

Please sign in to comment.