System Quantity Not Allowing Nulls

[JSRankins]

https://github.com/cqframework/cql-execution/blob/master/src/datatypes/quantity.js

  constructor(value, unit) {
    this.value = value;
    this.unit = unit;
    if (this.value == null || isNaN(this.value)) {
      throw new Error('Cannot create a quantity with an undefined value');
    } else if (!isValidDecimal(this.value)) {
      throw new Error('Cannot create a quantity with an invalid decimal value');
    }

The above Constructor throws an error when a null Quantity is provided. This presents an issue with CumulativeMedicationDuration library when timing.repeat.period in the let statement for MedicationRequestPeriod and MedicationDispensePeriod has no value. From a development perspective, I can understand why nulls are not desired. However, this presents an issue for testing systems when users do not provide timing for the related medication. @brynrhodes or @cmoesel, do you believe this to be an issue with the engine or the CQL?

[JSRankins]

@brynrhodes or @cmoesel, depending upon your answer to the above question, it looks like there are other quantity issues with those functions too - at least how they are working with version 2.2.0.

[cmoesel]

Hi @JSRankins -- the current spec says this about Quantity:

The value element of a Quantity must be present.

.. but it does not say what should happen if a Quantity value is null. Considering that a null value is a violation of a MUST in CQL, we throw an error rather than swallowing it and propagating the violation further. If authors write CQL expecting that value will never be null, then their logic could return unexpected results if we give it null. In addition, operators on Quantity may not be sufficiently defined to indicate how null values should work at all (since they are illegal anyway).

I'd note that the types.xsd schema also requires Quantity value:

<xs:element name="value" type="Decimal" minOccurs="1" maxOccurs="1"/>

but... oddly enough, the logical specification marks it as 0..1:

0..1 --> value

So... there is some inconsistency there.

We could probably update cql-execution to do something else, but as I noted above, I'm not 100% convinced that we should. Personally, I think the CQL logic should be updated to guard against this -- but I also recognize that this is an easy mistake for authors to stumble into, and that is concerning. I wonder if CQL-to-ELM could detect situations where authors build a quantity using a value that could be null, and then warn or error on it?

Also note that, at least for FHIR, if a FHIR.Quantity value is null, FHIRHelpers returns null for the whole Quantity, which I think is a reasonable approach.

I'd like to hear what @brynrhodes thinks about this.