Skip to content
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.

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

Add support for System.TimeSpan #15

Open
eiriktsarpalis opened this issue Oct 2, 2015 · 8 comments
Open

Add support for System.TimeSpan #15

eiriktsarpalis opened this issue Oct 2, 2015 · 8 comments

Comments

@eiriktsarpalis
Copy link
Member

While not a supported primitive by Azure, TimeSpan is a useful type to include in this library. It can be easily stored and queried using the Ticks representation.

@daniel-chambers
Copy link
Contributor

This is very doable. However, if something like #14 was implemented would that make the need for this redundant (ie you could configure a custom serializer to ticksify your timespans at will)? I'm a little concerned that if we hardcode TimeSpan to ticks, others may wish it written in different formats for whatever reason, especially since there is no usual behaviour defined by the standard Azure API.

@eiriktsarpalis
Copy link
Member Author

True, however it would make it impossible to generate queries that compare TimeSpans using that approach. The nice property of ticks representation is that it preserves ordering, unlike string representations.

@eiriktsarpalis
Copy link
Member Author

Obviously anyone opting for an alternative representation could override by attaching the [<Serialized>] attribute.

@daniel-chambers
Copy link
Contributor

That's true, though I'd argue the ordering thing is kinda moot since ordering only really matters on the PartitionKey and RowKey properties, which must be strings, so you'd need to zero pad your tick numbers for sorting, etc. Also, if you want reverse sorting, you need to serialize Int64.MaxValue - ticks, since Table Storage only supports top.

I don't agree that you wouldn't be able to compare Timespans with the serialization approach, but I'll take that discussion over to #14. :)

All this being said, TimeSpan is a pretty basic .NET type, so standard out of the box support using ticks seems reasonable, and should be pretty easy to implement. If people don't like it, as you say, they can use the custom serializers from #14 to change the default behaviour.

@eiriktsarpalis
Copy link
Member Author

There's no need to zero pad ticks, you can simply take advantage of the GenerateFilterConditionForLong method.

@daniel-chambers
Copy link
Contributor

I don't think you can use that against PK and RK because they must be string-typed. That method generates something like this PartitionKey lt 2L which when I try it, just causes an error.

Anyway, people can't use long/etc with PK/RK right now, so they won't be able to use TimeSpan either, which avoids the problem. That's why IEntityIdentifiable exists, to allow people to generate calculated string-based PK/RK values.

@eiriktsarpalis
Copy link
Member Author

Right, my assumption here was that you could only use string type for PK and RK annotated properties. Is this not the case?

@daniel-chambers
Copy link
Contributor

That's my understanding, yes.

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

No branches or pull requests

2 participants