FHIR Chat · R3 Draft · implementers

Stream: implementers

Topic: R3 Draft

Grahame Grieve (Dec 01 2016 at 03:06):

I have applied all the FHIR-Infrastructure tasks that I'm going to prior to the R3 draft publication next week (stable version for January connectathon and also IG ballots). This is a heads up so that if you care, you can check the outstanding task list for FHIR-I and let me know if there's anything in there that you think we should get to in the next couple of days

Grahame Grieve (Dec 01 2016 at 03:09):

actually, GF#11088 is something we should look at @Josh Mandel @Chris Moesel @Ewout Kramer @James Agnew

Josh Mandel (Dec 01 2016 at 12:46):

I'm having trouble adding a follow-up in gforge, but here's the issue: if a composite parameter simply provides references to the two parameters that it composes, we would end up in a situation where invalid combinations of paths were indexed. For example on Observation, we might combine a top-level code with a component level valueString and index that combination, which would be nonsense.

Chris Moesel (Dec 01 2016 at 13:46):

Josh's point was one of my concerns -- he just voiced it better. I think that #2 of the proposed resolution addresses this though, right? It would require the author to indicate the exact combinations of paths that are valid -- so the author wouldn't group a top-level code with a component level valueString.

That said, I think the actual example in the proposed resolution is incorrect. (1) First "expresssion" has three "s", (2) Is it valid to have the value of the "expression" be an array of expressions (maybe it is, I don't know), (3) I expect the second expression grouping is intended to be of the Observation.component.code and Observation.component.valueQuantity, right?

James Agnew (Dec 01 2016 at 17:52):

I like this idea, making composites be properly computable would be great.

It does seem like the proposal addresses Josh's concern, since it's pointing to paths instead of parameter names. I guess it would exclude components entirely though, based on how it's written....

...though reading it again I see what Chris means. Presumably that's a typo and it addresses that too.

Josh Mandel (Dec 01 2016 at 18:13):

Yes, the proposal was designed to address this concern! My response here was to @Grahame Grieve's comment at the bottom (~"what's [wrong] with the current solution")

Grahame Grieve (Dec 01 2016 at 18:36):

so I'm fine with breaking up the observation value-[x] composite search. But What I don't undesrtand is why the proposed change has anything to do with the comments here

Grahame Grieve (Dec 01 2016 at 18:37):

you can still end up with invalid combinations of paths - if indeed you're right that the particular combination you identify is incorrect (I'd have to think about it). But the change doesn't change whether you can have that combination, or whether you can find out what it says it means

Grahame Grieve (Dec 01 2016 at 18:37):

and I don't see how it makes the composites any more computable than they are now

Grahame Grieve (Dec 01 2016 at 18:38):

back to you @Josh Mandel @Chris Moesel @James Agnew

James Agnew (Dec 01 2016 at 18:49):

But the change doesn't change whether you can have that combination

Doesn't it? The current way of declaring composites is that a composite is a composite of two search params, and search params are of course backed by paths. So currently,

code-value-quantity =
code & value-quantity =
(Observation.code|Observation.component.code) & (Observation.valueQuantity|Observation.component.valueQuantity)

The proposed change would allow to express

code-value-quantity =
(Observation.code & Observation.valueQuantity) | (Observation.component.code & Observation.component.valueQuantity)

..which is surely the intent

Grahame Grieve (Dec 01 2016 at 18:53):

interesting. It surely is the intent, but what it means is that is not actually a composite parameter

Josh Mandel (Dec 01 2016 at 18:53):

Having support for this feature (explicitly listing paths for composites) doesn't magically make everything work; somebody needs to actually write down the correct combinations and publish them as part of the spec. +1 to what @James Agnew said.

Grahame Grieve (Dec 01 2016 at 19:00):

so the issues are:
- the observation composite search parameters do not sensibly conform to the current definition of a composite search parameter
- the proposed change to the search parameter doesn't conform to the current definition of a composite search parameter (it conforms to the way it's being used in Observation)
- the proposal is wrong, in that if we're going to define composites that way the proposal anticipates, the components need a bunch of other data as well as just the expression - at least type | expression | xpathUsage and possible comparator

James Agnew (Dec 01 2016 at 19:05):

why do we need the other data? Isn't it enough to just specify that the components are defined as "1..* pairs of resource paths"

Grahame Grieve (Dec 01 2016 at 19:07):

how do you know what the components are then?

Grahame Grieve (Dec 01 2016 at 19:08):

token$quanitity - but there's no way for a client to find that out. Nor is there a way for a server to find out how to index them because you need xPathUsage

Grahame Grieve (Dec 01 2016 at 19:08):

the current arrangement provides all that information.

James Agnew (Dec 01 2016 at 19:09):

Oh, you mean how does the client/server figure out the "search type" of the components.. Yeah, I guess that's a problem.

Josh Mandel (Dec 01 2016 at 19:10):

I think we need to specify which parameters are being composed, yes. And then separately provide a list of which path tuples should be indexed.

Josh Mandel (Dec 01 2016 at 19:10):

In other words, we keep what's defined today and *add* what's in the GForge proposal.

Josh Mandel (Dec 01 2016 at 19:12):

We can even have automated checks to ensure that the path tuples fall within the cartesian product of the individual component paths.

In other words if I have a composite parameter like a$b, where a has paths of path1, path2, and b has paths of path3, path4, then my a$b composite's paths must be a subset of (path1, path3), (path1, path4), (path2, path3), (path2, path4).

Grahame Grieve (Dec 01 2016 at 19:12):

I don't think that you can do that. either you define the composite in terms of other search parameters, or you define the composite directly. Please let's not mix the models

Josh Mandel (Dec 01 2016 at 19:12):

If you want to repeat the types while defining the composite, that's fine by me too — it's just a bit of denormalization.

Josh Mandel (Dec 01 2016 at 19:13):

But either way, it'd be nice to automatically check that composite definitions are "consistent with" their components' definitions.

Josh Mandel (Dec 01 2016 at 19:13):

As part of an automated QA process (not a burden that gets passed onto users)

Grahame Grieve (Dec 01 2016 at 19:14):

I don't know how such a check could be written.

Grahame Grieve (Dec 01 2016 at 19:16):

and I'm - now that we've had this discussion - against the way composites are defined in terms of other search parameters. That's really just lazy short cut on my part for supporting spreadsheets. We should define them properly, and just define them directly as a thing that has x parts.

Grahame Grieve (Dec 01 2016 at 19:16):

but even if we do, the observation search parameters with their nested grouping are a challenge

Grahame Grieve (Dec 01 2016 at 19:17):

that the proposal doesn't really address

James Agnew (Dec 01 2016 at 19:17):

"has x parts" or "has 2 parts"? i think that distinction has pretty big implications for how you write indexing code

Grahame Grieve (Dec 01 2016 at 19:17):

well, in the case of these observation search parameters, has 2 parts

James Agnew (Dec 01 2016 at 19:18):

but in general? is it valid to define a composite of 3 parts?

Grahame Grieve (Dec 01 2016 at 19:18):

currently yes, though there's no examples of that

James Agnew (Dec 01 2016 at 19:19):

I think there is one actually.. It broke the HAPI build for a while. At the time I asked and iirc the response was that it's not allowable to have 3-part composites so I excluded it in HAPI's build.

Lemme find it.

Grahame Grieve (Dec 01 2016 at 19:21):

Sequence.coordinate

James Agnew (Dec 01 2016 at 19:22):

yeah, that's the one.

so that's ok? HAPI has no support for that currently

Grahame Grieve (Dec 01 2016 at 19:22):

well, it's currently legal, yes

James Agnew (Dec 01 2016 at 19:24):

do you support it in your server?

the whole way I implement support for composites wouldn't work for composites of arbitrary length... I mean I can see the use case for wanting them, it's just not clear to me how you'd write a performant server implementation

Grahame Grieve (Dec 01 2016 at 19:25):

yeah it works for me. I just decompose them to their parts on the fly. For me, they're just a syntax trick to get grouping into the query.

Grahame Grieve (Dec 01 2016 at 19:25):

that's why my implementation depends on the fact that they refer to other search parameters. and also, my implementation is going to produce non-sensical results on the Observation composite

Grahame Grieve (Dec 01 2016 at 19:26):

because I am implementing exactly it's current definition

James Agnew (Dec 01 2016 at 19:32):

it sounds like your implementation probably works the same way as mine...

so you probably suffer from the same issue that I do, where an observation
{ systolic = 120, diastolic = 80 }
would be matched by systolic | lt110 even though that is not the intent

That's the part that is hard to solve (in a relational database at least) for N components

Grahame Grieve (Dec 01 2016 at 19:34):

yes. that's the core problem here. But it's in the definitions, not our implementations. And the proposed solution is effectively a kludge to fix up the definitions being broken

James Agnew (Dec 01 2016 at 19:35):

No I know, I'm tangent-ing a bit to ponder the question of whether we should allow composites of N parts or not. If we change composites, we should examine this question based on implementability

Grahame Grieve (Dec 01 2016 at 19:37):

yes. so back to Sequence.coordinate - there doesn't seem to be a slicker way to do that

Chris Moesel (Dec 01 2016 at 21:14):

I'm trying to follow this, but I can't tell if we landed on something that everyone is happy with. FWIW, Josh's proposal makes sense to me, particularly w/ his clarification that the valid path combinations would have to actually be written by someone (they can't be automatically inferred), and they'd be in addition to what's already in place. While it does make some extra work (probably including adjustments to the spreadsheet template), it seems to me that it covers the bases better than anything else I've heard.

Chris Moesel (Dec 01 2016 at 21:19):

The only "simpler" solution I can think of is to not allow composite search parameters where one of the parts has multiple paths. Or to indicate that when that does happen, all possible permutations of paths between the composite parts *must* produce "unsurprising" results. So this Observation use case would not be allowed at all.

Chris Moesel (Dec 01 2016 at 21:22):

Actually, I just understood James's point about systolic/diastolic -- if those are both Observation components, then does the problem still exist even with Josh's solution? Or is it understood that if two parts of a composite share a common parent path, and an element in that common path is a list, that the parts need to apply to the same item?

James Agnew (Dec 01 2016 at 21:54):

I believe the systolic/diastolic thing *can* be resolved with Josh's solution. We just need to explicitly state in the definition that in the case of paths which descend into repeating structures, the index applies only to pairs within the same repetition. That seems like a fair thing to state.

The thing I'm worried about then though is that in order to implement that, i'm thinking it'll be hard to implement a system that respects that rule and performs well for an arbitrary number of paths in a single composite.

James Agnew (Dec 01 2016 at 21:56):

..at least that is the case for the sequence example. I mean I'd probably in an RDBMS just have a table dedicated to that one index.

Maybe that's not such a big deal though.... It's not like there are hundreds of composites.

Grahame Grieve (Dec 01 2016 at 22:07):

so, Josh's proposal solves the particular problem of the the observation search parameter, but it creates lots of other problems. His revised hybrid proposal just creates a mess. My proposal to redefine composte parameters solves the general problem, but doesn't resolve this particular parameter

Grahame Grieve (Dec 01 2016 at 22:08):

the problem is the pairing where different composites have different paths, and you can permutate them. in some cases, that might be what you want, while in others, it might not

Grahame Grieve (Dec 01 2016 at 22:17):

and, in fact, Josh's solution does not work either

Josh Mandel (Dec 01 2016 at 22:44):

What breaks in "the general case"?

Josh Mandel (Dec 01 2016 at 22:45):

(And why does "my" solution "not work either"?)

Grahame Grieve (Dec 01 2016 at 22:55):

well, your original solution broke the link to the definitions.

Grahame Grieve (Dec 01 2016 at 22:56):

and your solution doesn't work because while you separate nicely the root code/value and the component code/value, there's still an unresolved permutational explosion between the component codes and values

Josh Mandel (Dec 01 2016 at 22:57):

Do we have (or anticipate) examples where the set size is greater than 2 or 3?

Grahame Grieve (Dec 01 2016 at 22:57):

I don't know.

Grahame Grieve (Dec 01 2016 at 22:57):

really, the solution is for a single expression that gives a list of tuples back.

Grahame Grieve (Dec 01 2016 at 22:57):

I'm going to see if I can write one shortly

Josh Mandel (Dec 01 2016 at 23:00):

Thinking back to how I did this for the SMART server, I introduced a path from the resource root to "a composite param common root", and then relative paths to the components. So for example...

Josh Mandel (Dec 01 2016 at 23:01):

param: code$value-string:

1. Top-level pairs

• path to composite param common root: Observation
• path to code component: code
• path to value-string component: valueString

2. Component pairs

• path to composite param common root: Observation.component
• path to code component: code
• path to value-string component: valueString

Grahame Grieve (Dec 01 2016 at 23:05):

yes. I prefer to do all that in the fhirpath expression

Josh Mandel (Dec 01 2016 at 23:05):

All in a single expression?

James Agnew (Dec 01 2016 at 23:05):

i am so intrigued by what you have in mind.. :)

Josh Mandel (Dec 01 2016 at 23:05):

I would have through you'd need a few.

Grahame Grieve (Dec 01 2016 at 23:08):

no FHIRPath selects are flattened. So you can't do that.

Grahame Grieve (Dec 01 2016 at 23:10):

but you can do :

SearchParameter
   type: composite
   expression: {main expression}
   component
      reference
      sub-expression
   component
      reference
     sub-expression

Grahame Grieve (Dec 01 2016 at 23:13):

so for the observation example:

  code-value-quantity
  expression=Observation | Observation.component

  component
     reference = code
     expression = code
  component
     reference = value-quantity
     expression = value.as(Quantity)

Grahame Grieve (Dec 01 2016 at 23:14):

though you could just as well do:

SearchParameter
   type: composite
   expression: {main expression}
   component
      type | usage | comparators
      sub-expression
   component
     type | usage | comparators
     sub-expression

Grahame Grieve (Dec 01 2016 at 23:15):

and inline them rather than having them be a reference to another parameter that has to exist independently. I do not know which I prefer. @James Agnew or @Brian Postlethwaite or @Ewout Kramer might have a preference

James Agnew (Dec 01 2016 at 23:20):

isn't that just a different syntax for exactly what Josh proposed that you hated? you've got the search parameter names that are being composited, plus an expression for the paths, and presumably a rule that they have to match

James Agnew (Dec 01 2016 at 23:21):

..and for the record I prefer the first, and I like it.

Grahame Grieve (Dec 01 2016 at 23:21):

what I disliked greatly about the original proposal was that
- the metadata was lost
- the expressions didn't work right

Grahame Grieve (Dec 01 2016 at 23:21):

I'll tell you want I'll do: I'll update SearchParameter for this right now, and we can look at the changes

Grahame Grieve (Dec 01 2016 at 23:22):

based on the first option

Grahame Grieve (Dec 02 2016 at 00:08):

ok. committed (svn 10444) for you all to have a look at once it's built. The question is, how do I show this in the resource specific pages?

Grahame Grieve (Dec 02 2016 at 00:11):

and James.... at least for my implementation, I will probably have to keep the one index multiple times now - once for it as a standalone parameter, and once for each composite it's involved in, since the composite ones are tried in groups

Grahame Grieve (Dec 02 2016 at 02:08):

update is visible now.

Brian Postlethwaite (Dec 02 2016 at 05:19):

I haven't implemented composites in my search indexing as of yet.
The proposal of a parent expression to get to the correct component, then sub-expressions to retrieve the actual values is good.
The other Idea I had was to modify fhirpath to be able to return tuples, which would permt a fhirpath only definition of the index, and have no dependance on the other indexes.
component.select(code.coding.first().select( system + '#' + code) , value.value.)

Brian Postlethwaite (Dec 02 2016 at 05:20):

By the way, my custom questionnaire data extract logic does exactly the main expression, then sub-expressions for extracting content.

Grahame Grieve (Dec 04 2016 at 19:48):

well, no one has commented on my draft... I guess it will be published as is

James Agnew (Dec 05 2016 at 12:17):

Sorry... it's been the weekend you know :)

Will give it a read today.

Josh Mandel (Dec 05 2016 at 20:11):

https://github.com/hl7-fhir/fhir-svn/commit/68419bcb2a6d7db5c593bf3137d754b4c921217f implements Grahame's change for #11088 but I can't quite follow

Josh Mandel (Dec 05 2016 at 20:12):

Does this implement one of the approaches we discussed @Grahame Grieve ? Viewing the rendered spec I only see things like:

pasted image

at http://build.fhir.org/searchparameter-registry.html

Josh Mandel (Dec 05 2016 at 20:14):

Also the search param registry at http://build.fhir.org/searchparameter-registry.html doesn't list canonical URLs or links to the search parameter resource instances.

Josh Mandel (Dec 05 2016 at 20:17):

And following search param links in http://build.fhir.org/capabilitystatement-terminology-server.json.html for example doesn't quite work. A link like http://hl7.org/fhir/SearchParameter/ValueSet-version just redirects to http://hl7.org/fhir/valueset.html.

Grahame Grieve (Dec 05 2016 at 20:56):

the search parameter registry does not show the execution details, and not for composite parameters either

Grahame Grieve (Dec 05 2016 at 20:57):

I don't have space for canonical URLs in the search parameter registry, so they appear as fly-overs. And I didn't have time to render every search parameter resource so I could link to it from the registry. If you want to create a task about that, that'll make sure I get to it

Grahame Grieve (Dec 05 2016 at 20:59):

and all the canonical URL redirects are broken in the current build because the canonical URL points to the final location, not the current build

Josh Mandel (Dec 05 2016 at 20:59):

OK. We tried to review your resolution on today's FHIR-I call, but nobody could understand what the resolution was.

James Agnew (Dec 05 2016 at 21:00):

Oh shoot. I thought today was cancelled... Sorry, would have joined!

Josh Mandel (Dec 05 2016 at 21:00):

We wound up ending early.

Grahame Grieve (Dec 05 2016 at 21:16):

'nobody could understand what the resolution was' is obviously a problem.

Grahame Grieve (Dec 05 2016 at 21:17):

if we were not in freeze, I ould add a paragraph explaining. That'll have to come next time

James Agnew (Dec 05 2016 at 21:18):

This definitely doesn't seem like something crucial to make STU3 in the grand scheme of things.. It's a pretty niche feature.

Josh Mandel (Dec 05 2016 at 21:31):

Could you add a paragraph here, though?

Grahame Grieve (Dec 05 2016 at 21:34):

When a search parameter is a composite, the parts of which it is composed are specifed by other parameters, in SearchParameter.component.definition

Grahame Grieve (Dec 05 2016 at 21:34):

these provide the declaration of their functionality.

Grahame Grieve (Dec 05 2016 at 21:36):

when it comes to determining the correct values for the search on the composite field, a different arrangement specifies the values. It's not possible to simply execute the individual component search parameter because the point of a composite is to tie the values of the components together, not to permutate them with each other

Grahame Grieve (Dec 05 2016 at 21:38):

so for composite parameters, the expression is split into parts. The first expression (SearchParameter.expression) specifies the main expression that returns the list of indexable items. These are usually anonymous element types (e.g. Observation.component). Then, for each component, there's a sub-expression that specifies how to determine the correct value of the component for that indexable item. For composite parameters, the sub-expression must return only a single value

Grahame Grieve (Dec 05 2016 at 21:39):

does that help?

Josh Mandel (Dec 05 2016 at 21:39):

Yes! And is there an example (like for Observation:code-value-string)?

Grahame Grieve (Dec 05 2016 at 21:41):

sure. that's all in the spec. but only really available - because of my band-width limitations - in the file searchparameters.[xml/json] which is in the various definition downloads.

Grahame Grieve (Dec 05 2016 at 21:41):

so for your ease:

Grahame Grieve (Dec 05 2016 at 21:43):

      "resource": {
        "resourceType": "SearchParameter",
        "url": "http://hl7.org/fhir/SearchParameter/Observation-code-value-concept",
        "code": "code-value-concept",
        "base": [ "Observation" ],
        "type": "composite",
        "expression": "Observation | Observation.component",
        "xpathUsage": "normal",
        "component": [
          {
            "definition": {
              "reference": "http://hl7.org/fhir/SearchParameter/Observation-code"
            },
            "expression": "code"
          },
          {
            "definition": {
              "reference": "http://hl7.org/fhir/SearchParameter/Observation-value-concept"
            },
            "expression": "value.as(CodeableConcept)"
          }
        ]
      }

Josh Mandel (Dec 05 2016 at 21:47):

Ah, I was looking for searchparameters.json: do we link to it from somewhere? http://build.fhir.org/searchparameter-registry.html would be a good place.

Josh Mandel (Dec 05 2016 at 21:47):

I can add a tracker item if that helps.

Grahame Grieve (Dec 05 2016 at 21:48):

it's just included in the various definitions zips. I don't think we link to it directly anywhere

Grahame Grieve (Dec 05 2016 at 21:48):

should include it on searchparameter-examples.html

Lloyd McKenzie (Dec 05 2016 at 21:48):

@Grahame Grieve wrong topic?

Grahame Grieve (Dec 05 2016 at 21:50):

yes it was. fixed

Last updated: Apr 12 2022 at 19:14 UTC