Support the TimeOnly and DateOnly types added in .NET 6 #424
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
marcin-krystianc
approved these changes
Feb 19, 2024
Comment on lines
+612
to
+661
| #if NET6_0_OR_GREATER | ||
| public static void ConvertDateOnly(ReadOnlySpan<int> source, Span<DateOnly> destination) | ||
| { | ||
| for (int i = 0; i < destination.Length; ++i) | ||
| { | ||
| destination[i] = ToDateOnly(source[i]); | ||
| } | ||
| } | ||
|
|
||
| public static void ConvertDateOnly(ReadOnlySpan<int> source, ReadOnlySpan<short> defLevels, Span<DateOnly?> destination, short definedLevel) | ||
| { | ||
| for (int i = 0, src = 0; i < destination.Length; ++i) | ||
| { | ||
| destination[i] = defLevels[i] != definedLevel ? default(DateOnly?) : ToDateOnly(source[src++]); | ||
| } | ||
| } | ||
|
|
||
| public static void ConvertTimeOnlyMicros(ReadOnlySpan<long> source, Span<TimeOnly> destination) | ||
| { | ||
| for (int i = 0; i < destination.Length; ++i) | ||
| { | ||
| destination[i] = ToTimeOnlyMicros(source[i]); | ||
| } | ||
| } | ||
|
|
||
| public static void ConvertTimeOnlyMicros(ReadOnlySpan<long> source, ReadOnlySpan<short> defLevels, Span<TimeOnly?> destination, short definedLevel) | ||
| { | ||
| for (int i = 0, src = 0; i < destination.Length; ++i) | ||
| { | ||
| destination[i] = defLevels[i] != definedLevel ? default(TimeOnly?) : ToTimeOnlyMicros(source[src++]); | ||
| } | ||
| } | ||
|
|
||
| public static void ConvertTimeOnlyMillis(ReadOnlySpan<int> source, Span<TimeOnly> destination) | ||
| { | ||
| for (int i = 0; i < destination.Length; ++i) | ||
| { | ||
| destination[i] = ToTimeOnlyMillis(source[i]); | ||
| } | ||
| } | ||
|
|
||
| public static void ConvertTimeOnlyMillis(ReadOnlySpan<int> source, ReadOnlySpan<short> defLevels, Span<TimeOnly?> destination, short definedLevel) | ||
| { | ||
| for (int i = 0, src = 0; i < destination.Length; ++i) | ||
| { | ||
| destination[i] = defLevels[i] != definedLevel ? default(TimeOnly?) : ToTimeOnlyMillis(source[src++]); | ||
| } | ||
| } | ||
| #endif | ||
|
|
There was a problem hiding this comment.
I think all those methods could be internal, but given that other converters are already public we should probably maintain consistency.
There was a problem hiding this comment.
Hmm yeah the convert methods were all made public intentionally in #185. I assume the reason is that users might want to make use of these methods in their own custom converters. It does mean we have to try not to introduce breaking changes here though.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #423
This adds support for converting to and from the .NET
TimeOnlyandDateOnlytypes when using the .NET 6 target. For theTimeOnlytype, I've added support for millisecond and microsecond precision, matching the existingTimeSpanconversion behaviour.In addition, I've added two options to allow customizing the built-in
LogicalTypeFactoryclass so that these types can be used by default instead of theParquetSharp.DateandTimeSpantypes. I've made these settable properties so that they can be modified on the defaultLogicalTypeFactoryinstance to allow changing the process-wide behaviour. I'd usually tend towards using immutable objects, but in this case it seems worthwhile to allow configuring this without too much boilerplate. I'm open to other suggestions on how to handle this though.We've also previously used AppContext switches to allow customizing behaviour in csproj configuration files (see #288), and it could make sense to support that here too?