StevenRasmussen / EFCore.SqlServer.NodaTime

NodaTime support for EF Core
MIT License
105 stars 18 forks source link

Support for Period #3

Open snebjorn opened 4 years ago

snebjorn commented 4 years ago

From the NodaTime docs

A duration represents a fixed length of elapsed time along the time line that occupies the same amount of time regardless of when it is applied. In contrast, Period represents a period of time in calendrical terms (years, months, days, and so on) that may vary in elapsed time when applied.

In general, use Duration to represent durations applied to global types like Instant and ZonedDateTime; use Period to represent a period applied to local types like LocalDateTime.

I understand that as: Period should be used with the Local* types and Duration with the Zoned* types.

So Period support would be nice :)

StevenRasmussen commented 4 years ago

So Period support would be nice :)

Agreed! A contribution here would be great ;)

snebjorn commented 4 years ago

No promises, but I'll have a look :)

benmccallum commented 3 years ago

Would this just be a matter of copying Duration's implementation and tweaking it a bit?

StevenRasmussen commented 3 years ago

TBH - I haven't worked with periods much... and looking into it again I think my hesitation in implementing it was finding the best way to store it in SQL in such a way that it wouldn't lose any information and could also be queried with operators other than ==. Looking at the period documentation, periods contain the different date parts, ie month, weeks, days, hours etc. But the kicker here is that these values can be negative.

We could "Normalize" the value before storing but then we lose the ability to store negative values. For example, a period of 1 day, -1 hours would be normalized to 23 hours.

It's also not clear what datatype to pick: a sql time can only store up to 24 hours. Storing as a datetime2 probably makes the most sense but still doesn't fit perfectly. According to the documentation the date value must be between 0001-01-01 and 9999-12-31. If our period is only 3 days then we are forced to add a year and month to it, ie: 0001-01-03:00.000, which looking at it you would think would represent 1 year, 1 month, and 3 days.

We could try storing it as some string, ie "YY:MM:WW:DD:HH:MM" etc (I know those aren't the exact format strings but you get the picture). But doing so makes it difficult to perform any sort of querying of this data since it would probably have to be converted to another type first to be able to use operators like < and >.

Anyway, lots of things to work out before implementing it :)

benmccallum commented 3 years ago

No worries, does indeed sound complicated and until there's enough demand can probably sit in the too hard basket :)

Not only negative values, but non-normalized values too, like 90minutes is valid and the equality check is each property being equa, so you'd want to maintain those in/out of the db as is, not matter how bizarre the values :P

It actually sounds like NodaTime doesn't support greater than, less than, etc. type "ordering" operators.

Period equality is implemented by comparing each property's values individually, without any normalization. (For example, a period of "24 hours" is not considered equal to a period of "1 day".) The static NormalizingEqualityComparer comparer provides an equality comparer which performs normalization before comparisons.

There is no natural ordering for periods, but CreateComparer(LocalDateTime) can be used to create a comparer which orders periods according to a reference date, by adding each period to that date and comparing the results.

If you look at a type like Duration --> click Operators (side menu), you can see it has tonnes more than Period does. So that got me thinking the others aren't supported and that does indeed look to be the case. Sample.

So if the compiler won't even allow < > <= >=, then we'd be ok to just choose a serialized format that we cando == or != with like you mentioned.

The trickier part then becomes mappings for +, - and the properties and functions like, like col.Weeks == x or col.HasDateComponent, etc.

StevenRasmussen commented 3 years ago

Ooohhhh.... interesting. I didn't see that < > <= >= won't even work... that does simplify things as far as storing the data. Our best option is probably a string at this point. That being said, I feel like this problem should already be solved somehow but I didn't really find anything when searching for something like 'store nodatime period in SQL'.

It looks like the ToString method on the period gives us a proper way of storing it (I had just come up with my own format before seeing that the ToString accomplished the same thing as mine).

This format allows us to implement the queries like x.Days == 12 => WHERE Period like '*D*'

I need to investigate further to see if it covers all the scenarios (methods & properties) that we'd want. Thoughts?

The + and - operators definitely pose a challenge if we need to do them in SQL. We'd essentially have to parse the strings in SQL into their respective parts, add/subtract each of the individual parts, then stitch them back together into the resulting string. Perhaps we deem this not supported initially?

benmccallum commented 3 years ago

I don't personally have a need for this immediately or can think of any future need at the moment. That's to say:

Using NodaTime's string format definitely makes sense, explicitly using PeriodPattern.Roundtrip.Format/Parse as it says "round-trips its values: a parse/format cycle will produce an identical period, including units.", rather than NormalizingIso. Docs. Sample.

Interesting stuff in any case!

HerrNiklasRaab commented 3 years ago

We could "Normalize" the value before storing but then we lose the ability to store negative values. For example, a period of 1 day, -1 hours would be normalized to 23 hours.

Wouldn't normalizing destroy the purpose of why it exists in the first place? Check out this.

StevenRasmussen commented 3 years ago

@HerrNiklasRaab - agreed. We would most likely use the PeriodPattern.Rountrip.Format/Parse methods as detailed by @benmccallum if we were to implement this.