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

PanacheMongoRepository does not deal well with instants #6566

Closed
TheVidAllMayThe opened this issue Jan 15, 2020 · 8 comments · Fixed by #6583 or #7159
Closed

PanacheMongoRepository does not deal well with instants #6566

TheVidAllMayThe opened this issue Jan 15, 2020 · 8 comments · Fixed by #6583 or #7159
Assignees
Labels
Milestone

Comments

@TheVidAllMayThe
Copy link

TheVidAllMayThe commented Jan 15, 2020

When using the PanacheMongoRepository to query the database using Instants the Inventory does not find any results unless the query is written in mongo syntax.

fun getByCreationInstantBefore(time: Instant) = list("endAt < ?1", time)

Should return the Commands present in the database with endAt < time but returns an empty list.

Steps to reproduce the behavior:

  1. Create a Database populated with entries with time
  2. Try to query by time like above

fun getByCreationInstantBefore(time: Instant) = list("{endAt : { \$lte : ISODate(?1) }}", time) works well

@TheVidAllMayThe TheVidAllMayThe added the kind/bug Something isn't working label Jan 15, 2020
@gsmet
Copy link
Member

gsmet commented Jan 15, 2020

@loicmathieu could you have a look?

@loicmathieu
Copy link
Contributor

@TheVidAllMayThe < will map to $lt if you want to map to $lte you need to use <=.

Can you try with

fun getByCreationInstantBefore(time: Instant) = list("endAt <= ?1", time)

You can active query logging to check that the correct query is created by MongoDB with Panache via

quarkus.log.category."io.quarkus.mongodb.panache.runtime".level=DEBUG

Please activate it and give us the query to help diagnose the issue.

@TheVidAllMayThe
Copy link
Author

That was my bad, I really wanted the behavior to be of "<", not "<=".

@loicmathieu when trying as you suggested I get this:

[io.qua.mon.pan.run.MongoOperations] (executor-thread-1) {'endAt':{'$lt':'2020-01-16T09:45:21.283Z'}} and the result is again an empty list.

I'm guessing it should be translated to {'endAt':{'$lt':ISODate('2020-01-16T09:45:21.283Z')}} instead

@loicmathieu loicmathieu self-assigned this Jan 16, 2020
@loicmathieu loicmathieu added this to the 1.2.0 milestone Jan 16, 2020
@loicmathieu
Copy link
Contributor

OK, so the issue is that Instant is not translated to ISODate.

As a workaround you may be able to do (not tested)

fun getByCreationInstantBefore(time: Instant) = list("endAt < ISODate(?1)", time)

Anyway, I'll work for a fix.
Thanks for reporting.

@TheVidAllMayThe TheVidAllMayThe changed the title PanacheMongoRepository does deal well with instants PanacheMongoRepository does not deal well with instants Jan 16, 2020
@TheVidAllMayThe
Copy link
Author

OK, so the issue is that Instant is not translated to ISODate.

As a workaround you may be able to do (not tested)

fun getByCreationInstantBefore(time: Instant) = list("endAt < ISODate(?1)", time)

Anyway, I'll work for a fix.
Thanks for reporting.

@loicmathieu Uunfortunately this does not work, as it results in:
[io.qua.mon.pan.run.MongoOperations] (executor-thread-1) {'endAt':{'$lt':null}}

The only way I've got it to work was by writing the query manually:
fun getByCreationInstantBefore(time: Instant) = list("{endAt : { \$lte : ISODate(?1) }}", time)

@TheVidAllMayThe
Copy link
Author

@loicmathieu I don't think this was properly fixed. In version 1.2.0.Final, when I do list("{endAt : { \$lte : ISODate(?1) }}", time) which works on 1.1.1.Final I get:

(QuarkusQuartzScheduler_Worker-1) {endAt : { $lt : ISODate('2020-01-29T09:17:32.521') }}
org.bson.json.JsonParseException: Invalid date format.

And same with list("endAt < ?1", time)

The problem seems to be on the missing Z at the end:
It should be '2020-01-29T09:17:32.521Z' instead of '2020-01-29T09:17:32.521'

I find it odd that the Z disappears now since it shows if you call the toString() method in any Instant.

So to get it working now I need to change the workaround to list("{endAt : { \$lte : ISODate(?1) }}", time.toString()) and I didn't have to call the toString method before since the Instant was correctly converted.

@loicmathieu
Copy link
Contributor

@TheVidAllMayThe I reopen the issue, will have a look next week

@loicmathieu loicmathieu reopened this Jan 29, 2020
@gsmet gsmet removed this from the 1.2.0.CR1 milestone Jan 29, 2020
@loicmathieu
Copy link
Contributor

@TheVidAllMayThe I works on this now and the date format is not OK for both LocalDateTime and Instant. It's not the fact that is misses the Z but it's the fact it generates format with too many nanoseconds fractions (6 and MongoDB only supports 3 of them).

See this generated query with all the supported formats

{'dateDate':{'$lt':ISODate('2022-11-08T11:26:59.200')},'localDate':{'$lt':ISODate('2020-02-13')},'localDateTime':{'$lt':ISODate('2020-02-13T11:26:59.200248')},'instant':{'$lt':ISODate('2020-02-13T10:26:59.200336')}}

I will fix this issue but it raises the question of adding Z at the end to express the fact that it's UTC dates. In fact it's true for Instant but not for LocalDateTime as they are local so expressed in the server current timezone. I can move them at UTC timezone to be consistent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants