clinical_quality_language icon indicating copy to clipboard operation
clinical_quality_language copied to clipboard

ToList is not implemented according to the spec

Open EvanMachusak opened this issue 3 years ago • 2 comments

According to the ELM spec,

https://cql.hl7.org/04-logicalspecification.html#tolist

The operator is effectively shorthand for "if operand is null then { } else { operand }".

However: https://github.com/cqframework/clinical_quality_language/blob/d10c4ef801a8e160510b2a868c044dea5bdb85c1/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/execution/ToListEvaluator.java

When operand is already a list, the ruler simply returns the operand. That logic is on line 12.

This is important as the cql-to-elm translator assumes this behavior. Consider this CQL:


private codesystem "Test": 'https://example.org/fhir/codesystem/test-cs'
private code "Code 1": '1' from "Test"
private code "Code 2": '2' from "Test"
private code "Code 3": '3' from "Test"

define "Test Codes": { "Code 1", "Code 2", "Code 3" }
define "Retrieve with list of codes": [Observation : "Test Codes"]

The translator is generating this ELM for the codes property of the Retrieve:

      "expression" : {
         "type" : "Retrieve",
         "codes" : {
           "type" : "ToList",
           "operand" : {
             "type" : "ExpressionRef",
             "resultTypeSpecifier" : {
               "type" : "ListTypeSpecifier",
               "elementType" : {
                 "type" : "NamedTypeSpecifier",
                 "name" : "{urn:hl7-org:elm-types:r1}Code"
               }
             },
             "locator" : "24:54-24:65",
             "name" : "Test Codes"
           }
         },

Because the ruler ignores the ToList for expressions that are already lists, it codes would end up being expressed as a List<Code>.

If you were to follow the letter of the ELM spec, codes ends up expressed as a List<List<Code>>.

This confounds engines that expect the terminology element of the retrieve statement to be a List<Code> (or a value set, which is implicitly convertible to a List<Code>).

EvanMachusak avatar Mar 03 '23 13:03 EvanMachusak

IMO, the implementation makes sense -- if a list is passed to ToList, it should return the list as-is rather than increase the dimensions of the list. Which means that my recommendation would be to update the specification. I'd consider this a "technical correction". @brynrhodes - what say you?

cmoesel avatar Mar 14 '23 20:03 cmoesel

I can see it either way.

I don't mind whether this is an implementation change or a spec change.

We could tack this into the ELM spec:

When applied to a value that is already a List, this expression returns the existing List.

EvanMachusak avatar Mar 25 '23 09:03 EvanMachusak