Skip to content

Conversation

rafaelGuerreiro
Copy link
Contributor

@rafaelGuerreiro rafaelGuerreiro commented May 2, 2025

Description of Changes

Making Timestamp and TimeDuration implement FilterableValue.

This resolves #2650

API and ABI breaking changes

API change is additive in this case, I'm not marking this as unstable because the FilterableValue implementation is already stabilized.

Expected complexity level and risk

Complexity 1.

Testing

@CLAassistant
Copy link

CLAassistant commented May 2, 2025

CLA assistant check
All committers have signed the CLA.

@rafaelGuerreiro rafaelGuerreiro changed the title Make Timestamp and TimeDuration Filterable Make Timestamp and TimeDuration implement FilterableValue May 2, 2025
@rafaelGuerreiro
Copy link
Contributor Author

This PR is currently marked as draft because I'll work on the C# backend, as well as the client SDK changes, if necessary.

@@ -89,6 +89,22 @@ pub enum TestF {
Baz(String),
}

#[spacetimedb::table(
name = test_g,
index(name = created_at_index, btree(columns = [name, created_at])),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add a test case for this index as well

@@ -320,13 +320,16 @@ public static void insert_one_connection_id(ReducerContext ctx, ConnectionId a)
[SpacetimeDB.Table(Name = "one_timestamp", Public = true)]
public partial struct OneTimestamp
{
[SpacetimeDB.Index.BTree]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Replicate the test in module-test-cs instead of sdk-test-cs

@gefjon
Copy link
Contributor

gefjon commented May 5, 2025

This one is going to be much more work to get merged than your previous PR. It's going to need:

@egormanga
Copy link

egormanga commented Aug 7, 2025

May I ask why isn't this prioritized still?

The backend module change should be separate from the client ones, if it takes so much to implement properly across SDKs. All it takes is merging my one-liner PR.

This is currently a major blocker for me, requiring a crate override.

Upd. Oh right, #3008. Wasn't mentioned here. Probably better to close this PR as a duplicate, too.

@bfops
Copy link
Collaborator

bfops commented Aug 8, 2025

Since this PR hasn't been updated in a while and is missing the client SDK pieces, closing this in favor of #3008.

@bfops bfops closed this Aug 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make Timestamp implement FilterableValue
5 participants