FHIR Chat · repeat('someString') · fhirpath

Stream: fhirpath

Topic: repeat('someString')

---

Paul Lynch (Mar 22 2022 at 21:46):

repeat takes an expression, which could be a constant. What should the behavior be in that case? If you have a collection "a", and call a.repeat('blue'), would you expect to get one blue for each item in 'a', or one blue in total? (I am not suggesting this is a useful thing to do, but trying to nail down what the correct behavior would be.)

Josh Mandel (Mar 23 2022 at 00:03):

The definition says:

as long as the projection yields new items

So if you call a.repeat('blue'), let's say initially a refers to a collection like[1,2,3]. The way I read it, the processing would be:

Begin Round 1 with inputs = [1, 2, 3]

• Evaluate 'blue' for the input item 1
  • --> evaluated items is [1]
  • --> Round 1 output is ['blue']
• Evaluate 'blue' for the input item 2
  • --> evaluated items is [1, 2]
  • --> output is ['blue'] (tried adding a 2nd time but it's not unique!)
• Evaluate 'blue' for the input item 3
  • --> evaluated items is [1, 2, 3]
  • --> output is ['blue'](tried adding a 3nd time but it's not unique!)

Round 1 complete, and the (outputs - evaluated) is not empty, so... begin Round 2 with inputs = ['blue'] (from Round 1's outputs)

• Evaluate 'blue' for the input item 'blue'
  • --> evaluated items is [1, 2, 3, 'blue']
  • --> output is ['blue'](tried adding a 4th time but it's not unique!)

Round 2 complete and (outputs - evaluated) is empty, so ... we're done!

Josh Mandel (Mar 23 2022 at 00:06):

Ooh, I see BTW why you're asking -- the https://hl7.github.io/fhirpath.js crashes when you ask it to evaluate Patient.name.repeat('test') ;-)

Paul Lynch (Mar 23 2022 at 19:21):

Indeed it does. I came across this when I accidentally wrote Questionnaire.repeat('item') instead of Questionnaire.repeat(item).

Paul Lynch (Mar 23 2022 at 19:28):

It is not clear to me from the documentation that the check for existing items occurs after each item or after each round, and I am not sure which is the better approach because it is hard to image a use case for doing this. Anyway, as Bryn agrees with your interpretation, I will file a tracker item to add a clarification. Thanks for the very clear example.

Paul Lynch (Mar 23 2022 at 19:38):

FHIR-36588

Brian Postlethwaite (Mar 24 2022 at 22:03):

p.s. That kills the dotnet fhirpath engine too. It never checks if the new node already existed (using =) so execution never stops.
This is where that code is in the donet SDK, will log it.
https://github.com/FirelyTeam/firely-net-common/blob/ba31e20d00dd0f2c5e0d56ceefa4fff4d686b309/src/Hl7.FhirPath/FhirPath/Expressions/SymbolTableInit.cs#L347

Brian Postlethwaite (Mar 24 2022 at 23:25):

https://github.com/FirelyTeam/firely-net-sdk/issues/2018

Grahame Grieve (Mar 31 2022 at 00:31):

that also did it for the Java engine as well. Fixed next release

Paul Lynch (Apr 01 2022 at 15:39):

@Josh Mandel We made a change to add the uniqueness check in your algorithm above, but I am having some doubts about that. I think if you write Questionnaire.repeat(extension), you just want all of the extensions, and don't care whether they are unique or not.

Josh Mandel (Apr 01 2022 at 17:19):

Thinking about a FHIR Questionnaire as a tree of nodes.... you're saying you want to see the same node more than once? Or just that if semantically equivalent nodes appear at different positions in the tree, you'd want to see all of those?

Paul Lynch (Apr 01 2022 at 17:19):

The latter. I don't think repeat could find the exact same node in a tree more than once.

Paul Lynch (Apr 01 2022 at 21:42):

However, the specification does say it stops when the projection does not yield "new items (as determined by the = (Equals) (=) operator).", so I don't see how my example of Questionnaire.repeat(extension) could be expected to return a complete list of the extensions on the root of the Questionnaire. In other words, I am withdrawing my question about the algorithm.

Paul Lynch (Apr 01 2022 at 23:30):

The requirement to do an Equals comparison means doing deep equals for Questionnaire items, and the complexity of the algorithm is O(n**2). On one test Questionnaire with 772 questions, it was faster to call descendants() than to call repeat(item). For a short questionnaire with only 10 items, descendants() and repeat(item) were about the same, which is disappointing, because I've been thinking repeat(item) should be much faster. It is also a bit frustrating since in this case the items should all be unique due to their linkIds, and the uniqueness check is just a waste of time. We are looking at possibly building a hash to speed up comparisons, at least when there are a large of number of items.

Brian Postlethwaite (Apr 02 2022 at 07:42):

And if you're doing that on questionnaireresponse. Item, you could legitimately have dups, but this might remove them. I'm not sure if that's sensible, but if that's what the response has, it wouldn't return it.

Paul Lynch (Apr 04 2022 at 14:56):

Maybe we should have repeat(...) and repeatUnique(...)?

Paul Lynch (Apr 04 2022 at 15:00):

I am not sure there is really a need for the uniqueness check, except as a way to avoid endless loops like repeat('someString'). If someone wanted uniqueness, they could always call repeat(something).distinct().

Josh Mandel (Apr 04 2022 at 15:53):

I like that concept, and engines could just have (effectively) a "stack depth limit" or "iteration limit" that could prevent actual endless loops in unbounded or pathological cases.

Grahame Grieve (Apr 04 2022 at 21:23):

but this would be a breaking change to normative content.

Grahame Grieve (Apr 04 2022 at 21:23):

so you'd really have to propose adding a .repeatNonUnique(xxx)

Josh Mandel (Apr 04 2022 at 22:53):

And to propose deprecating the old one

Brian Postlethwaite (Apr 05 2022 at 01:20):

I'll propose some tests with expected results to conoare/discuss.
(for unit testing the implementations with)

Grahame Grieve (Apr 05 2022 at 01:54):

why deprecate ?

Josh Mandel (Apr 05 2022 at 03:46):

If (from the discussion above) all known use cases are accounted for by other invocations and performance with the old definition is likely to be an issue.

Grahame Grieve (Apr 05 2022 at 03:55):

but it still works, so why force people to change?

Josh Mandel (Apr 05 2022 at 04:28):

If it works reliably, no reason. If it's a performance pitfall that people will hit when they least expect, that's a reason.

Grahame Grieve (Apr 05 2022 at 04:34):

it does work reliably and it's the right thing to do in many cases. Paul found one case where it's slower than an alternative, but that's no grounds to deprecate it

Paul Lynch (Apr 05 2022 at 20:11):

Created FHIR-36708 for "repeatNonUnique".

---

Last updated: Apr 12 2022 at 19:14 UTC