-
Notifications
You must be signed in to change notification settings - Fork 121
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ExpandValueSet is called aggressively #840
Comments
I think it's important to call out here that
Then you can 1) always use the optimization of lazy expansion, and 2) ensure that you're always using terminology semantics for the |
Having said that, there is, by design, no way to construct a ValueSet with the codes that it contains. So let me ask the question, why not have first-class value set definitions for all the lists of codes you want to support? |
Because we can implicitly convert a
The issue I raised here is related to the fact that In our SDK, we implemented an implicit conversion between This lets us implement the We currently don't implement |
Consider this CQL:
When this function is called with a value set, e.g.:
The operand in the ELM to the "code in value set" call is an ExpandValueSet.
The value set does not in fact need to be expanded, because this is valid:
The expression of "example" is an InValueSet.
Eagerly expanding the value set ahead of time introduces a performance penalty for this pattern.
It's possible to declare the parameter as a System.ValueSet which will avoid the ExpandValueSet operand, but because of the automatic expansion of value sets with this usage, these two signatures become ambiguous:
CQL-to-ELM picks the second overload in this case. That's probably correct, but unintuitive.
We use the pattern of passing value sets to functions taking a List of Code often. It's a more flexible signature since it lets us pass either a value set or a manually constructed list of codes.
This is a significant performance penalty when run on the CQF ruler as prematurely expanding value sets which can be large (50k+ rows) results in linear checking of a list of codes instead of an optimized hash lookup of valueset membership.
The text was updated successfully, but these errors were encountered: