-
Notifications
You must be signed in to change notification settings - Fork 91
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
feat: add support to filter by Instant
, ZonedDateTime
, OffsetDateTime
, ... in LINQ
#605
base: master
Are you sure you want to change the base?
feat: add support to filter by Instant
, ZonedDateTime
, OffsetDateTime
, ... in LINQ
#605
Conversation
…ove a useless function call for `DateTime` conversion
…decimal` in LINQ queries
…ffsetDateTime`, `OffsetDate`, `LocalDateTime`, `LocalDate` and .NET 6+ `DateOnly` in LINQ queries
…sure tick precision of `TimeSpan`
…g ones for `TimeSpan` precision
6645a51
to
b57a9d2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Sylvain2703,
First off, a big thank you for your pull request and the fantastic improvements you've made to this client! Your contribution is greatly appreciated, and it's wonderful to see such valuable enhancements. 👍
I do have one concern regarding the support for older .NET versions. Would it be possible to retain the <LangVersion>
as 8
in the project configuration? We aim to continue supporting .NET Core 3.1
, and keeping the language version to 8 would help ensure compatibility with this version of .NET Core.
Thanks again for your excellent work and looking forward to your input on this matter.
Best regards
Hi @bednar, Thank you very much for your feedback 😀 Regarding the C# version, I was surprised that this lib still uses C# 8 only... but I understood the reason when I saw the CI errors 😉 In our case, C# 12 requires the .NET 8.0 SDK but can compile for .NET Core 3.1 (or even .NET Framework 4.*). After checking the CI pipeline and trying to use the .NET 8 SDK, I realized that the tests cannot run because the .NET SDK Docker image only contains the runtime for the same version. Best regards |
f098a8e
to
34facc5
Compare
… install missing runtimes for testing
34facc5
to
427de6b
Compare
Instant
, ZonedDateTime
, OffsetDateTime
, ... in LINQ
Hi @Sylvain2703, Apologies for my late response. Thank you for thoroughly checking the CI pipeline and experimenting with the .NET 8 SDK. Your insights into the issue with the .NET SDK Docker image and the inability to run tests due to version-specific runtime constraints are invaluable. Your proposed solution to install multiple .NET Core runtimes (3.1/5/6/7) on the .NET 8 SDK Docker image, as discussed in this StackOverflow answer, sounds like an excellent approach. It not only resolves our current issue but also simplifies future maintenance by ensuring compatibility across different .NET versions. This foresight is greatly appreciated and aligns well with our goals for the client's long-term usability and robustness. We are more than happy to incorporate these changes into the client. Your initiative in finding and suggesting a viable solution is a significant contribution to the project. Thank you for your dedication and valuable input. Best Regards |
Hi @bednar, Thank you for your answer! After experimenting a lot with CircleCI, I propose a slightly different but cleaner solution than the one mentioned previously. Instead of installing additional .NET runtimes on the .NET 8 SDK Docker image (which was causing dependencies incompatibilities with the base image), I opened PR #613 to:
After completing PR #613, this PR should be able to pass the CI pipeline 😀 Best regards |
Proposed Changes
Features
DateTimeLiteral
AST type (thanks to NodaTime'sInstant
).Instant
,ZonedDateTime
,OffsetDateTime
,OffsetDate
,LocalDateTime
andLocalDate
,DateOnly
(addingnet6.0
to InfluxDB.Client.Linq'sTargetFrameworks
),Duration
andPeriod
,Tick
precision forTimeSpan
(only microsecond precision was supported),sbyte
,short
anddecimal
.Refactor
Expression
(inVariableAggregator.CreateExpression()
).DateTime
conversion (inFluxRecord.GetTimeInDateTime()
).Tests
TimeSpan
precision.Checklist
dotnet test
completes successfully