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

Invalid OffsetDateTime comparison accounting for offset differences #536

Closed
novoj opened this issue Apr 24, 2024 · 2 comments · Fixed by #539
Closed

Invalid OffsetDateTime comparison accounting for offset differences #536

novoj opened this issue Apr 24, 2024 · 2 comments · Fixed by #539
Assignees
Labels
bug Something isn't working
Milestone

Comments

@novoj
Copy link
Collaborator

novoj commented Apr 24, 2024

There is probably some bug in sending the OffsetDateTime over the wire:

image

The data in the end looks like that:

image

And this doesn't work well for the equality comparison although there were correct data at the start of the query.

@novoj novoj added the bug Something isn't working label Apr 24, 2024
@novoj novoj added this to the Alpha milestone Apr 24, 2024
@novoj
Copy link
Collaborator Author

novoj commented Apr 24, 2024

May be this is not a problem of a client, but the Java Time API itself and the way we use it in evitaDB. See following explanation from chatGPT which we verified manually by our tests. For values:

2024-04-24T11:07:11.677467736Z
2024-04-24T13:07:11.677467736+02:00

Which are logically equal:


When you're using OffsetDateTime objects in Java, the compareTo method compares both the instant in time and the offset. So even if two OffsetDateTime objects represent the same moment in time (the same instant), if their offsets are different, they are not considered equal by the compareTo method. This is why you see a -1 instead of 0 when comparing your two dates.

In the case of the timestamps you provided:

  • 2024-04-24T11:07:11.677467736Z has an offset of UTC (Z or +00:00).
  • 2024-04-24T13:07:11.677467736+02:00 has an offset of +02:00.

Although they represent the same absolute point in time, their offsets differ. According to the OffsetDateTime.compareTo() specification:

Compares this OffsetDateTime with another ensuring that the date-time is ordered before the date-time with a later offset.

This means that an OffsetDateTime with a lesser offset is considered "earlier" than one with a greater offset when their instants are the same, hence the -1 result indicating the first is earlier.

If your goal is to compare only the instants (ignoring the offsets), you should convert these OffsetDateTime objects to Instant as shown previously:

import java.time.OffsetDateTime;
import java.time.Instant;

OffsetDateTime dateTime1 = OffsetDateTime.parse("2024-04-24T11:07:11.677467736Z");
OffsetDateTime dateTime2 = OffsetDateTime.parse("2024-04-24T13:07:11.677467736+02:00");

Instant instant1 = dateTime1.toInstant();
Instant instant2 = dateTime2.toInstant();

int result = instant1.compareTo(instant2);  // This should return 0, indicating equality.

This conversion to Instant ensures that the comparison is based solely on the universal timeline, disregarding any timezone or offset differences.

@novoj novoj assigned novoj and unassigned lukashornych Apr 24, 2024
@novoj
Copy link
Collaborator Author

novoj commented Apr 24, 2024

I think it's not entirely possible to pass the original form of the date time over different protocols from different clients and this may be also the case of evitaLab (waiting for confirmation from @lukashornych).

@novoj novoj modified the milestones: Alpha, Beta Apr 26, 2024
@novoj novoj changed the title Invalid OffsetDateTime conversion on the way from the client to the server Invalid OffsetDateTime comparison accounting for offset differences Apr 26, 2024
novoj added a commit that referenced this issue Apr 26, 2024
…es into account

Equal date times:

- `2024-04-24T11:07:11.677467736Z`
- `2024-04-24T13:07:11.677467736+02:00`

were considered different. Also, the filter index didn't take string collations into account, so less than & greater than didn't work properly for strings containing national characters.

The fix changed the format of the stored data, so I had to include a backward compatible deserializer so that existing data is loaded correctly. When the data is saved again, it has the correct new format. The old format should be automatically removed by compaction.
novoj added a commit that referenced this issue Apr 26, 2024
…version-on-the-way-from-the-client-to-the-server

fix (#536): Invalid OffsetDateTime comparison taking offset difference into an account
novoj added a commit that referenced this issue Apr 29, 2024
…es into account

The comparator was not updated in "changes" delta layer which lead to invalid sequence for collated strings. This corrupted also stored data so we have to work around the non-monotonic rows of values on start.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants