RetrieveEvaluator doesnt pass on valueset version to DataProvider/RetrieveProvider

[Zylox]

The retrieve evaluator has access to the valueset version when it resolves the valuesetRef here:

clinical_quality_language/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/RetrieveEvaluator.java

Lines 46 to 47 in dfafdb9

	ValueSet vs = ValueSetRefEvaluator.toValueSet(state, (ValueSetRef) elm.getCodes());
	valueSet = vs.getId();

As that code shows, it then extracts ONLY the id when passed to the DataProvider below:

clinical_quality_language/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/RetrieveEvaluator.java

Lines 76 to 88 in dfafdb9

	result = dataProvider.retrieve(
	state.getCurrentContext(),
	(String) dataProvider.getContextPath(state.getCurrentContext(), dataType.getLocalPart()),
	state.getCurrentContextValue(),
	dataType.getLocalPart(),
	elm.getTemplateId(),
	elm.getCodeProperty(),
	codes,
	valueSet,
	elm.getDateProperty(),
	elm.getDateLowProperty(),
	elm.getDateHighProperty(),
	dateRange);

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:

clinical_quality_language/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/InValueSetEvaluator.java

Lines 30 to 52 in 542c919

	if (valueset instanceof ValueSet) {
	ValueSetInfo vsi = ValueSetInfo.fromValueSet((ValueSet) valueset);
	TerminologyProvider provider = state.getEnvironment().getTerminologyProvider();
	// perform operation
	if (code instanceof String) {
	if (provider.in(new Code().withCode((String) code), vsi)) {
	return true;
	}
	return false;
	} else if (code instanceof Code) {
	if (provider.in((Code) code, vsi)) {
	return true;
	}
	return false;
	} else if (code instanceof Concept) {
	for (Code codes : ((Concept) code).getCodes()) {
	if (codes == null) return null;
	if (provider.in(codes, vsi)) return true;
	}
	return false;
	}
	}

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.

[JPercival]

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.

[Zylox]

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