FHIR / sql-on-fhir-v2

This project provides the source for the SQL on FHIR v2.0 Implementation Guide
https://build.fhir.org/ig/FHIR/sql-on-fhir-v2/
MIT License
92 stars 26 forks source link

Decimal for lowBoundary and highBoundary #238

Open Diferti opened 4 months ago

Diferti commented 4 months ago

In #229 I implemented lowBoundary and highBoundary for dateTime, date, and time.

How should I interpret lowBoundary and highBodunary for Decimal? The expectation here says lowBoundary for 1.0 should be 0.95 but why is it not 1.000000000000000000e+000000000?

JavaScript can't have numbers as big as the FHIR spec? What should we do? One option is just to make them as big as JavaScript can be.

awalley-ncqa commented 3 months ago

I am still confused, myself, on how to implement the low/high boundary functions in FHIRPath (and subsequently sof). Based on this definition, I am not sure how we get 0.95 as an expectation. I think that would mean we only have precision on a step of 0.05, right?

Also in the fhirpath spec that I shared it says that decimal should have a minimum of 8 digits of precision. I believe a double fits within this requirement, so we could (potentially) use a minimum requirement based on the 8 digits (?)

awalley-ncqa commented 3 months ago

I just found this implementation from the fhirpath.js zulip chat: https://chat.fhir.org/#narrow/stream/179298-fhirpath.2Ejs/topic/lowBoundary.20and.20highBoundary.20functions.20implementation/near/436916972

see: https://github.com/cqframework/clinical_quality_language/blob/master/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/LowBoundaryEvaluator.java

Diferti commented 3 months ago

Thank you. I will review it.

However, even 8 digits of precision JS can not hold in some cases, for example:

1.123456789123456 is 16 digits in total, precision is 15, it works fine 1.1234567891234567 is 17 digits in total, precision is 16, overflow happens 12345.123456789123456 is 20 digits in total, precision is 15, overflow happens, because floating point numbers in JS can hold only 16 digits in total. One digit represents 4-bits, hence 64-bit has 16 digits.

So, case when input value like 123456789123.1234, precision 4 is maximum if more then overflow will happen.

if consider FHIR datatypes maximum input value can be this: 999999999999999999.999999999999999999e999999999

As for step of 0.05, in my opinion this is an error in the tests, because it does not make sense and the expected number does not really change in length, that is it does not fulfill what the boundary function intended.

johngrimes commented 1 month ago

Here is a link to where I am trying to get a resolution on the intended behaviour for these functions: https://chat.fhir.org/#narrow/stream/179266-fhirpath/topic/Boundary.20functions

I think the SQL on FHIR spec is fine as is, as our link goes to the CI build of the FHIRPath spec.

The reference implementation may need to change to reflect any updates to the semantics.