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

[FEATURE] Serialize DateOnly as a date instead of serializing its properties individually #680

Closed
nalka0 opened this issue Jun 20, 2024 · 8 comments · Fixed by #734
Closed
Labels
enhancement New feature or request

Comments

@nalka0
Copy link

nalka0 commented Jun 20, 2024

Is your feature request related to a problem?

DateOnly properties are treated as any other complex type and its properties are serialised individually. It prevents using features like date ranges when searching.

What solution would you like?

DateOnly properties be treated as DateTime properties, except maybe they'd use a different format (like yyyy-mm-dd)

What alternatives have you considered?

Customizing the serialization behavior myself, but I didn't find a way to do so. Also tried applying [DateAttribute] to my property but didn't seem to change anything

Do you have any additional context?

Currently DateOnly properties are serialized this way :

"dateOnlyProperty": {
    "year": 2024,
    "month": 4,
    "day": 15,
    "dayOfWeek": 1,
    "dayOfYear": 106,
    "dayNumber": 738990
},

PS : TimeOnly may have the same kind of issues

@nalka0 nalka0 added enhancement New feature or request untriaged labels Jun 20, 2024
@Xtansia Xtansia removed the untriaged label Jun 20, 2024
@Xtansia
Copy link
Collaborator

Xtansia commented Jun 20, 2024

Thanks for reporting this @nalka0! Would you be interested in contributing a fix for this? It should just involve adding a JsonFormatter similar to the DateTime & co ones then registering like here. It will require adding the minimum necessary .NET version (I think 6? as I believe it's not in .netstandard2.0) as a target framework and then using #ifs to conditionally add in the formatter depending on target .NET version.

@nalka0
Copy link
Author

nalka0 commented Jun 22, 2024

@Xtansia Honnestly, I don't feel familiar enough with either OpenSearch nor this nuget package to feel like sending a pull request 😅
I also am not so familiar with the .Net version management you described but can confirm DateOnly has been introduced in .Net 6 and isn't in .Net standard

@Xtansia
Copy link
Collaborator

Xtansia commented Jun 24, 2024

@nalka0 That's completely understandable and no problem :)

@joefeser
Copy link
Contributor

I am going to attempt a PR for this one

joefeser added a commit to joefeser/opensearch-net that referenced this issue Jul 30, 2024
joefeser added a commit to joefeser/opensearch-net that referenced this issue Jul 30, 2024
joefeser added a commit to joefeser/opensearch-net that referenced this issue Jul 30, 2024
Xtansia added a commit that referenced this issue Jul 31, 2024
* feat(): add .net6 and .net8 to core project and get it to compile #680

Signed-off-by: Joe Feser <[email protected]>

* feat(): add support for DateOnly and TimeOnly from .net 6 #680

Signed-off-by: Joe Feser <[email protected]>

* PR Comments

Signed-off-by: Joe Feser <[email protected]>

* PR Comments

Signed-off-by: Joe Feser <[email protected]>

* modified the added section of changelog.md

Signed-off-by: Joe Feser <[email protected]>

* Fix compilation/indentation error

Signed-off-by: Thomas Farr <[email protected]>

* Fix code conventions unit test

Signed-off-by: Thomas Farr <[email protected]>

* Fix remaining `#if`s

Signed-off-by: Thomas Farr <[email protected]>

* Fix changelog ordering

Signed-off-by: Thomas Farr <[email protected]>

* Fix CI errors

Signed-off-by: Thomas Farr <[email protected]>

---------

Signed-off-by: Joe Feser <[email protected]>
Signed-off-by: Thomas Farr <[email protected]>
Co-authored-by: Thomas Farr <[email protected]>
Xtansia pushed a commit to Xtansia/opensearch-net that referenced this issue Jul 31, 2024
…pensearch-project#734)

* feat(): add .net6 and .net8 to core project and get it to compile opensearch-project#680

Signed-off-by: Joe Feser <[email protected]>

* feat(): add support for DateOnly and TimeOnly from .net 6 opensearch-project#680

Signed-off-by: Joe Feser <[email protected]>

* PR Comments

Signed-off-by: Joe Feser <[email protected]>

* PR Comments

Signed-off-by: Joe Feser <[email protected]>

* modified the added section of changelog.md

Signed-off-by: Joe Feser <[email protected]>

* Fix compilation/indentation error

Signed-off-by: Thomas Farr <[email protected]>

* Fix code conventions unit test

Signed-off-by: Thomas Farr <[email protected]>

* Fix remaining `#if`s

Signed-off-by: Thomas Farr <[email protected]>

* Fix changelog ordering

Signed-off-by: Thomas Farr <[email protected]>

* Fix CI errors

Signed-off-by: Thomas Farr <[email protected]>

---------

Signed-off-by: Joe Feser <[email protected]>
Signed-off-by: Thomas Farr <[email protected]>
Co-authored-by: Thomas Farr <[email protected]>
(cherry picked from commit f3bae74)
Xtansia added a commit that referenced this issue Jul 31, 2024
* feat(): add .net6 and .net8 to core project and get it to compile #680

Signed-off-by: Joe Feser <[email protected]>

* feat(): add support for DateOnly and TimeOnly from .net 6 #680

Signed-off-by: Joe Feser <[email protected]>

* PR Comments

Signed-off-by: Joe Feser <[email protected]>

* PR Comments

Signed-off-by: Joe Feser <[email protected]>

* modified the added section of changelog.md

Signed-off-by: Joe Feser <[email protected]>

* Fix compilation/indentation error

Signed-off-by: Thomas Farr <[email protected]>

* Fix code conventions unit test

Signed-off-by: Thomas Farr <[email protected]>

* Fix remaining `#if`s

Signed-off-by: Thomas Farr <[email protected]>

* Fix changelog ordering

Signed-off-by: Thomas Farr <[email protected]>

* Fix CI errors

Signed-off-by: Thomas Farr <[email protected]>

---------

Signed-off-by: Joe Feser <[email protected]>
Signed-off-by: Thomas Farr <[email protected]>
Co-authored-by: Thomas Farr <[email protected]>
(cherry picked from commit f3bae74)

Co-authored-by: Joe Feser <[email protected]>
@nalka0
Copy link
Author

nalka0 commented Aug 8, 2024

I guess this issue could be reopened (can't do it myself) based on the problems I described on the related PR #734 (comment)

@joefeser
Copy link
Contributor

joefeser commented Aug 8, 2024

@nalka0 If you can reach out to me (see contact info), I can work with you to ensure everything works as expected.

@nalka0
Copy link
Author

nalka0 commented Aug 8, 2024

@nalka0 If you can reach out to me (see contact info), I can work with you to ensure everything works as expected.

I'm sorry, what contact info are you talking about ? I don't see any 😅

@joefeser
Copy link
Contributor

joefeser commented Aug 8, 2024

Sorry I realized I took my email off my profile. @nalka0 joseph dot Feser at the google email service.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants