clinical_quality_language icon indicating copy to clipboard operation
clinical_quality_language copied to clipboard

RetrieveEvaluator doesnt pass on valueset version to DataProvider/RetrieveProvider

Open Zylox opened this issue 1 year ago • 2 comments

The retrieve evaluator has access to the valueset version when it resolves the valuesetRef here: https://github.com/cqframework/clinical_quality_language/blob/dfafdb933f23ab54ce750c4ed1d6d1bf910e9bed/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/RetrieveEvaluator.java#L46-L47

As that code shows, it then extracts ONLY the id when passed to the DataProvider below: https://github.com/cqframework/clinical_quality_language/blob/dfafdb933f23ab54ce750c4ed1d6d1bf910e9bed/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/RetrieveEvaluator.java#L76-L88

This leads to a situation where it is ambiguous what version of the valueset is to be used by the retreive/data provider.

It is NOT ambigious when referenced as part of an expression: https://github.com/cqframework/clinical_quality_language/blob/542c919aa271fcb393be2b803814355763ab7633/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/InValueSetEvaluator.java#L30-L52

It seems the simplest solution would be to pass the ValueSet object in place of the valuesetId string, the retreiveprovider can strip it down to just the id if that is desired.

Zylox avatar Dec 03 '24 18:12 Zylox

Yes. This is a change we'd like to the DataProvider API.

My current thinking on this is to convert the long list of method arguments into a RetrieveParams object:

class RetrieveParams {
   get/set context,
   get/set contextPath,
   etc..
}

List<Object> retrieve(RetrieveParams params);

Same for the terminology where applicable. That will make it easier to expand and/or modify the parameters going forward, and to add a batch/multi retrieve overload:

List<List<Object>> retrieve(List<RetrieveParams> params);

Probably wouldn't literally use a list of lists. That's just conceptual. But the point is that we could batch data requests to the underlying platform for fewer round-trips.

JPercival avatar Dec 19 '24 16:12 JPercival

Being able to batch those could really benefit my use case's performance, I like that a lot.

Zylox avatar Dec 19 '24 17:12 Zylox