Skip to content
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

Implicit base not picked up for the patient search parameter #1674

Closed
lmsurpre opened this issue Nov 6, 2020 · 10 comments
Closed

Implicit base not picked up for the patient search parameter #1674

lmsurpre opened this issue Nov 6, 2020 · 10 comments
Assignees
Labels
bug Something isn't working P3 Priority 3 - Nice To Have search

Comments

@lmsurpre
Copy link
Member

lmsurpre commented Nov 6, 2020

Describe the bug
I found a case that we missed when we implemented #300 for adding the implicit resource type of a reference: chained parameters.

Specifically, note this line from https://build.fhir.org/search.html#chaining

Because the Diagnostic Report subject can be one of a set of different resources, it's necessary to limit the search to a particular type:
GET [base]/DiagnosticReport?subject:Patient.name=peter

To me, that implies that if the type was NOT ambiguous, then the type modifier should be optional (just like in the normal case). However, my query for Observation?patient.gender=female returns the following issue:

'issue': [{'severity': 'fatal', 'code': 'invalid', 'details': {'text': "Search parameter: 'patient' must have resource type name modifier"}

Expected behavior
The type modifier is not required if the reference search parameter only targets a single type.

Additional context
We should open an HL7 JIRA ticket on this section of the spec as well, because it contradicts itself in a bad way:

For instance, given that the resource DiagnosticReport has a search parameter named subject, which is usually a reference to a Patient resource, and the Patient resource includes a parameter name which searches on patient name, then the search
 GET [base]/DiagnosticReport?subject.name=peter
is a request to return all the lab reports that have a subject whose name includes "peter".
Because the Diagnostic Report subject can be one of a set of different resources, it's necessary to limit the search to a particular type:
 GET [base]/DiagnosticReport?subject:Patient.name=peter

If it's "necessary to limit the search to a particular type" then the first query they give isn't actually valid, but they don't make this very clear.

@lmsurpre lmsurpre added the bug Something isn't working label Nov 6, 2020
@lmsurpre lmsurpre changed the title https://github.com/IBM/FHIR/issues/300 Implicit base not picked up in FHIR chained search Nov 6, 2020
@lmsurpre lmsurpre added the search label Nov 6, 2020
@michaelwschroeder
Copy link
Contributor

@lmsurpre I believe this is working as designed. When parsing a chained search parameter, we check the SearchParameter.target contents. If the target contains more than one resource type and a type modifier is not specified on the search parameter, then an error (as shown above) is returned.

In the example above, the Observation?patient.gender=female search correctly returns an error because the patient search parameter specifies both the Patient and Group resource types as targets. Here's the full search parameter definition:

   {
      "resourceType" : "SearchParameter",
      "id" : "clinical-patient",
      "extension" : [{
        "url" : "http://hl7.org/fhir/StructureDefinition/structuredefinition-standards-status",
        "valueCode" : "trial-use"
      }],
      "url" : "http://hl7.org/fhir/SearchParameter/clinical-patient",
      "version" : "4.0.1",
      "name" : "patient",
      "status" : "draft",
      "experimental" : false,
      "date" : "2019-11-01T09:29:23+11:00",
      "publisher" : "Health Level Seven International (Patient Care)",
      "contact" : [{
        "telecom" : [{
          "system" : "url",
          "value" : "http://hl7.org/fhir"
        }]
      },
      {
        "telecom" : [{
          "system" : "url",
          "value" : "http://www.hl7.org/Special/committees/patientcare/index.cfm"
        }]
      }],
      "description" : "Multiple Resources: \r\n\r\n* [AllergyIntolerance](allergyintolerance.html): Who the sensitivity is for\r\n* [CarePlan](careplan.html): Who the care plan is for\r\n* [CareTeam](careteam.html): Who care team is for\r\n* [ClinicalImpression](clinicalimpression.html): Patient or group assessed\r\n* [Composition](composition.html): Who and/or what the composition is about\r\n* [Condition](condition.html): Who has the condition?\r\n* [Consent](consent.html): Who the consent applies to\r\n* [DetectedIssue](detectedissue.html): Associated patient\r\n* [DeviceRequest](devicerequest.html): Individual the service is ordered for\r\n* [DeviceUseStatement](deviceusestatement.html): Search by subject - a patient\r\n* [DiagnosticReport](diagnosticreport.html): The subject of the report if a patient\r\n* [DocumentManifest](documentmanifest.html): The subject of the set of documents\r\n* [DocumentReference](documentreference.html): Who/what is the subject of the document\r\n* [Encounter](encounter.html): The patient or group present at the encounter\r\n* [EpisodeOfCare](episodeofcare.html): The patient who is the focus of this episode of care\r\n* [FamilyMemberHistory](familymemberhistory.html): The identity of a subject to list family member history items for\r\n* [Flag](flag.html): The identity of a subject to list flags for\r\n* [Goal](goal.html): Who this goal is intended for\r\n* [ImagingStudy](imagingstudy.html): Who the study is about\r\n* [Immunization](immunization.html): The patient for the vaccination record\r\n* [List](list.html): If all resources have the same subject\r\n* [MedicationAdministration](medicationadministration.html): The identity of a patient to list administrations  for\r\n* [MedicationDispense](medicationdispense.html): The identity of a patient to list dispenses  for\r\n* [MedicationRequest](medicationrequest.html): Returns prescriptions for a specific patient\r\n* [MedicationStatement](medicationstatement.html): Returns statements for a specific patient.\r\n* [NutritionOrder](nutritionorder.html): The identity of the person who requires the diet, formula or nutritional supplement\r\n* [Observation](observation.html): The subject that the observation is about (if patient)\r\n* [Procedure](procedure.html): Search by subject - a patient\r\n* [RiskAssessment](riskassessment.html): Who/what does assessment apply to?\r\n* [ServiceRequest](servicerequest.html): Search by subject - a patient\r\n* [SupplyDelivery](supplydelivery.html): Patient for whom the item is supplied\r\n* [VisionPrescription](visionprescription.html): The identity of a patient to list dispenses for\r\n",
      "code" : "patient",
      "base" : ["AllergyIntolerance",
      "CarePlan",
      "CareTeam",
      "ClinicalImpression",
      "Composition",
      "Condition",
      "Consent",
      "DetectedIssue",
      "DeviceRequest",
      "DeviceUseStatement",
      "DiagnosticReport",
      "DocumentManifest",
      "DocumentReference",
      "Encounter",
      "EpisodeOfCare",
      "FamilyMemberHistory",
      "Flag",
      "Goal",
      "ImagingStudy",
      "Immunization",
      "List",
      "MedicationAdministration",
      "MedicationDispense",
      "MedicationRequest",
      "MedicationStatement",
      "NutritionOrder",
      "Observation",
      "Procedure",
      "RiskAssessment",
      "ServiceRequest",
      "SupplyDelivery",
      "VisionPrescription"],
      "type" : "reference",
      "expression" : "AllergyIntolerance.patient | CarePlan.subject.where(resolve() is Patient) | CareTeam.subject.where(resolve() is Patient) | ClinicalImpression.subject.where(resolve() is Patient) | Composition.subject.where(resolve() is Patient) | Condition.subject.where(resolve() is Patient) | Consent.patient | DetectedIssue.patient | DeviceRequest.subject.where(resolve() is Patient) | DeviceUseStatement.subject | DiagnosticReport.subject.where(resolve() is Patient) | DocumentManifest.subject.where(resolve() is Patient) | DocumentReference.subject.where(resolve() is Patient) | Encounter.subject.where(resolve() is Patient) | EpisodeOfCare.patient | FamilyMemberHistory.patient | Flag.subject.where(resolve() is Patient) | Goal.subject.where(resolve() is Patient) | ImagingStudy.subject.where(resolve() is Patient) | Immunization.patient | List.subject.where(resolve() is Patient) | MedicationAdministration.subject.where(resolve() is Patient) | MedicationDispense.subject.where(resolve() is Patient) | MedicationRequest.subject.where(resolve() is Patient) | MedicationStatement.subject.where(resolve() is Patient) | NutritionOrder.patient | Observation.subject.where(resolve() is Patient) | Procedure.subject.where(resolve() is Patient) | RiskAssessment.subject.where(resolve() is Patient) | ServiceRequest.subject.where(resolve() is Patient) | SupplyDelivery.patient | VisionPrescription.patient",
      "xpath" : "f:AllergyIntolerance/f:patient | f:CarePlan/f:subject | f:CareTeam/f:subject | f:ClinicalImpression/f:subject | f:Composition/f:subject | f:Condition/f:subject | f:Consent/f:patient | f:DetectedIssue/f:patient | f:DeviceRequest/f:subject | f:DeviceUseStatement/f:subject | f:DiagnosticReport/f:subject | f:DocumentManifest/f:subject | f:DocumentReference/f:subject | f:Encounter/f:subject | f:EpisodeOfCare/f:patient | f:FamilyMemberHistory/f:patient | f:Flag/f:subject | f:Goal/f:subject | f:ImagingStudy/f:subject | f:Immunization/f:patient | f:List/f:subject | f:MedicationAdministration/f:subject | f:MedicationDispense/f:subject | f:MedicationRequest/f:subject | f:MedicationStatement/f:subject | f:NutritionOrder/f:patient | f:Observation/f:subject | f:Procedure/f:subject | f:RiskAssessment/f:subject | f:ServiceRequest/f:subject | f:SupplyDelivery/f:patient | f:VisionPrescription/f:patient",
      "xpathUsage" : "normal",
      "target" : ["Patient",
      "Group"]
    }

Looking at the expression field, the only expression which can return something other than the Patient resource type is the DeviceUseStatement.subject expression, which can return Patient or Group. I don't know if that was done by design, but seems odd to specify that rather than DeviceUseStatement.subect.where(resolve() is Patient) (like the expressions for the other resource types) in a patient search parameter.

@kmbarton423 kmbarton423 added the P3 Priority 3 - Nice To Have label Feb 18, 2021
@tbieste
Copy link
Contributor

tbieste commented May 10, 2021

@lmsurpre, was looking through backlog issues, and after looking into this one, I came to the same conclusion that Mike did. Can this one be closed?

@lmsurpre
Copy link
Member Author

Its definitely an interesting case.

As Mike mentions, Observation.subject.where(resolve() is Patient) can only ever be a Patient.
But I see that is quite difficult to infer from the SearchParameter definition and I agree its probably better just to rely on the target field of the search parameter definition.

The problem with this is that https://hl7.org/fhir/search.html#reference explicitly mentions this search parameter as an example of a reference search parameter that shouldn't need the :[type] modifier:

In some cases, search parameters are defined with an implicitly limited scope. For example, Observation has an element subject, which is a reference to one of a number of types. This has a matching search parameter subject, which refers to any of the possible types. In addition to this, there is another search parameter patient, which also refers to Observation.subject, but is limited to only include references of type Patient. When using the patient search parameter, there is no need to specify ":Patient" as a modifier, or "Patient/" in the search value, as this must always be true.

@lmsurpre
Copy link
Member Author

I had a peak at the search parameters from R5 and it looks like they have updated the target for this clinical-subject search parameter to include just about every kind of resource type as its "target":

"code" : "patient",
      "base" : ["AllergyIntolerance",
      "CarePlan",
      "CareTeam",
      "ClinicalImpression",
      "Composition",
      "Condition",
      "Consent",
      "DetectedIssue",
      "DeviceRequest",
      "DeviceUsage",
      "DiagnosticReport",
      "DocumentManifest",
      "DocumentReference",
      "Encounter",
      "EpisodeOfCare",
      "FamilyMemberHistory",
      "Flag",
      "Goal",
      "ImagingStudy",
      "Immunization",
      "List",
      "MedicationAdministration",
      "MedicationDispense",
      "MedicationRequest",
      "MedicationUsage",
      "NutritionOrder",
      "Observation",
      "Procedure",
      "RiskAssessment",
      "ServiceRequest",
      "SupplyDelivery",
      "VisionPrescription"],
      "type" : "reference",
      "expression" : "AllergyIntolerance.patient | CarePlan.subject.where(resolve() is Patient) | CareTeam.subject.where(resolve() is Patient) | ClinicalImpression.subject.where(resolve() is Patient) | Composition.subject.where(resolve() is Patient) | Condition.subject.where(resolve() is Patient) | Consent.subject.where(resolve() is Patient) | DetectedIssue.patient | DeviceRequest.subject.where(resolve() is Patient) | DeviceUsage.subject | DiagnosticReport.subject.where(resolve() is Patient) | DocumentManifest.subject.where(resolve() is Patient) | DocumentReference.subject.where(resolve() is Patient) | Encounter.subject.where(resolve() is Patient) | EpisodeOfCare.patient | FamilyMemberHistory.patient | Flag.subject.where(resolve() is Patient) | Goal.subject.where(resolve() is Patient) | ImagingStudy.subject.where(resolve() is Patient) | Immunization.patient | List.subject.where(resolve() is Patient) | MedicationAdministration.subject.where(resolve() is Patient) | MedicationDispense.subject.where(resolve() is Patient) | MedicationRequest.subject.where(resolve() is Patient) | MedicationUsage.subject.where(resolve() is Patient) | NutritionOrder.patient | Observation.subject.where(resolve() is Patient) | Procedure.subject.where(resolve() is Patient) | RiskAssessment.subject.where(resolve() is Patient) | ServiceRequest.subject.where(resolve() is Patient) | SupplyDelivery.patient | VisionPrescription.patient",
"target" : ["Patient",
      "Group",
      "Account",
      "ActivityDefinition",
      "AdministrableProductDefinition",
      "AdverseEvent",
      "AllergyIntolerance",
      "Appointment",
      "AppointmentResponse",
      "AuditEvent",
      "Basic",
      "Binary",
      "BiologicallyDerivedProduct",
      "BodyStructure",
      "Bundle",
      "CapabilityStatement",
      "CapabilityStatement2",
      "CarePlan",
      "CareTeam",
      "CatalogEntry",
      "ChargeItem",
      "ChargeItemDefinition",
      "Citation",
      "Claim",
      "ClaimResponse",
      "ClinicalImpression",
      "ClinicalUseIssue",
      "CodeSystem",
      "Communication",
      "CommunicationRequest",
      "CompartmentDefinition",
      "Composition",
      "ConceptMap",
      "Condition",
      "ConditionDefinition",
      "Consent",
      "Contract",
      "Coverage",
      "CoverageEligibilityRequest",
      "CoverageEligibilityResponse",
      "DetectedIssue",
      "Device",
      "DeviceDefinition",
      "DeviceMetric",
      "DeviceRequest",
      "DeviceUsage",
      "DiagnosticReport",
      "DocumentManifest",
      "DocumentReference",
      "Encounter",
      "Endpoint",
      "EnrollmentRequest",
      "EnrollmentResponse",
      "EpisodeOfCare",
      "EventDefinition",
      "Evidence",
      "EvidenceReport",
      "EvidenceVariable",
      "ExampleScenario",
      "ExplanationOfBenefit",
      "FamilyMemberHistory",
      "Flag",
      "Goal",
      "GraphDefinition",
      "GuidanceResponse",
      "HealthcareService",
      "ImagingStudy",
      "Immunization",
      "ImmunizationEvaluation",
      "ImmunizationRecommendation",
      "ImplementationGuide",
      "Ingredient",
      "InsurancePlan",
      "InventoryReport",
      "Invoice",
      "Library",
      "Linkage",
      "List",
      "Location",
      "ManufacturedItemDefinition",
      "Measure",
      "MeasureReport",
      "Medication",
      "MedicationAdministration",
      "MedicationDispense",
      "MedicationKnowledge",
      "MedicationRequest",
      "MedicationUsage",
      "MedicinalProductDefinition",
      "MessageDefinition",
      "MessageHeader",
      "MolecularSequence",
      "NamingSystem",
      "NutritionIntake",
      "NutritionOrder",
      "NutritionProduct",
      "Observation",
      "ObservationDefinition",
      "OperationDefinition",
      "OperationOutcome",
      "Organization",
      "OrganizationAffiliation",
      "PackagedProductDefinition",
      "PaymentNotice",
      "PaymentReconciliation",
      "Permission",
      "Person",
      "PlanDefinition",
      "Practitioner",
      "PractitionerRole",
      "Procedure",
      "Provenance",
      "Questionnaire",
      "QuestionnaireResponse",
      "RegulatedAuthorization",
      "RelatedPerson",
      "RequestGroup",
      "ResearchStudy",
      "ResearchSubject",
      "RiskAssessment",
      "Schedule",
      "SearchParameter",
      "ServiceRequest",
      "Slot",
      "Specimen",
      "SpecimenDefinition",
      "StructureDefinition",
      "StructureMap",
      "Subscription",
      "SubscriptionStatus",
      "SubscriptionTopic",
      "Substance",
      "SubstanceDefinition",
      "SubstanceNucleicAcid",
      "SubstancePolymer",
      "SubstanceProtein",
      "SubstanceReferenceInformation",
      "SubstanceSourceMaterial",
      "SupplyDelivery",
      "SupplyRequest",
      "Task",
      "TerminologyCapabilities",
      "TestReport",
      "TestScript",
      "ValueSet",
      "VerificationResult",
      "VisionPrescription"]

I'm not sure where those are all coming from but I'd argue that, given the section I just quoted above, it should have a single target of type Patient.

@lmsurpre
Copy link
Member Author

It looks like there is an open issue for this that resolved to change it to just "Patient": https://jira.hl7.org/browse/FHIR-13601
Personally, I think we should list this as erratta under our Conformance.md documentation and change the published search parameter to have a single target.

@tbieste
Copy link
Contributor

tbieste commented May 12, 2021

That seems reasonable to do.

@lmsurpre
Copy link
Member Author

lmsurpre commented May 12, 2021

What is funny is that issue actually links to https://jira.hl7.org/browse/FHIR-15903 in which I raised this same issue. I wish I would have linked to that from here...

Anyway, I think it provides further evidence for modifying the search parameter that we ship in our registry.
I'll leave my comments https://jira.hl7.org/browse/FHIR-13601 to hopefully get it addressed in FHIR R5 so that we don't need to patch the spec resource.

UPDATE: FHIR-13601 was closed without actually fixing the issue, so I've opened https://jira.hl7.org/browse/FHIR-32876

@lmsurpre lmsurpre self-assigned this May 12, 2021
@lmsurpre lmsurpre added this to the Sprint 2021-07 milestone May 12, 2021
lmsurpre added a commit that referenced this issue May 12, 2021
The built-in SearchParameter-clinical-patient search parameter should
have a single target of type "Patient" but instead it has a target of
both "Patient" and "Group".

In this commit, I move the fhir-registry IndexGenerator from
src/main/java to src/test/java and add a step to modify this one spec
resource so that users can search on the `patient` search parameter
without needing a type modifier.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
@lmsurpre
Copy link
Member Author

I also opened https://jira.hl7.org/browse/FHIR-32305 for the same issue in the US Core search parameter definitions. Depending on the result of that discussion, we may want to patch those ones as well.

@tbieste
Copy link
Contributor

tbieste commented May 13, 2021

Successfully verified with searches such as these:
Observation?patient.gender=female
Observation?patient.name=john

@tbieste tbieste closed this as completed May 13, 2021
@lmsurpre lmsurpre changed the title Implicit base not picked up in FHIR chained search Implicit base not picked up for the patient search parameter May 17, 2021
@lmsurpre
Copy link
Member Author

lmsurpre commented Jun 8, 2021

I just found that http://hl7.org/fhir/SearchParameter/Provenance-patient has the same issue. I added this as a comment to https://jira.hl7.org/browse/FHIR-32876 and I will open a separate issue for it here as well so that we can patch it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working P3 Priority 3 - Nice To Have search
Projects
None yet
Development

No branches or pull requests

4 participants