search

openEHR / archie

OpenEHR library implementing ADL 2, AOM 2, BMM, RM 1.0.4 and many tools
Apache License 2.0
53 stars 25 forks source link

DateTimeParser - support > 6 milliseconds #257

Closed bjornna closed 3 years ago

bjornna commented 3 years ago

I faced a problem using EHRBase SDK doing queries against our openEHR CDR. Some DV_DATE_TIME have many milliseconds. I.e: 2019-04-30T12:36:09.3893812+02:00

This fails. I think it's because there is a limit on 6 digits for milliseconds in the DateTimeFormatter: https://github.com/openEHR/archie/blob/52eb5bd4d77a528972a05254119867248ee2f864/utils/src/main/java/com/nedap/archie/datetime/DateTimeFormatters.java#L40

.appendFraction(ChronoField.MICRO_OF_SECOND, 0, 6, false)

pieterbos commented 3 years ago

That is an awfully precise datetime for use in a medical record!

Increasing the number of digits is not going to work. The built in OffsetDateTime and LocalDateTime store microseconds in an integer format, and you are sending a time with a fraction smaller than a microsecond. To solve this, we’ll have to add a nanoseconds fraction, either as one fraction with zero to nine digits replacing the microseconds fraction, or as both an optional microseconds and an optional nanoseconds fraction. That is possible, by changing both the pattern used for parsing and the one used for formatting.

The limit is nanoseconds, so nine digits, and that is doable, and I can do that. If you need more than nine digits, so picoseconds, you’re going to have to write your own datetime plus parser implementation, or something that just throws away all information that is even more precise.

But first a question: do you really need support for more exact times than microseconds, and why? :)

bjornna commented 3 years ago

This is precision medicine 😋

Or more serious: It doesn't make sense to register data like this with such a precision level. The datetime was from a blood pressure.

From a technical perspective: We don't alter data from clients. If the data is valid we store it. The datetime string is AFAIK valid IS0-8601. If that holds true the libraries should not fail when such data is presented.

I tried to write some tests and code to fix the issue. But I was not able to make it work.

pieterbos commented 3 years ago

Ah I see. I created a pull request that adds support for nanosecond precision. So the limit is now 9 digits.

As said, supporting anything more precise than nanoseconds is not an option, unless we store just the string and do not provide datetime arithmetic at all, OR someone implements a more precise datetime library.

pieterbos commented 3 years ago

The fix has been released: Nanosecond support is now available in version 0.18.0, release notes at https://github.com/openEHR/archie/releases/tag/0.18.0 . Should be available at maven central within a couple of hours.

birgerhaarbrandt commented 3 years ago

Cool, we will try to include this in EHRbase SDK asap and then @bjornna it would be great if you can repeat your test! Thanks @pieterbos!