fsprojects / FSharp.Azure.Storage

F# API for using Microsoft Azure Table Storage service
MIT License
75 stars 16 forks source link

Add support for System.TimeSpan #15

Open eiriktsarpalis opened 9 years ago

eiriktsarpalis commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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

daniel-chambers commented 9 years ago

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 commented 9 years ago

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

daniel-chambers commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

That's my understanding, yes.