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

_include search param doesn't support effective paging #346

Closed
ajdorenk opened this issue Oct 31, 2019 · 3 comments
Closed

_include search param doesn't support effective paging #346

ajdorenk opened this issue Oct 31, 2019 · 3 comments
Assignees
Labels
bug Something isn't working search
Milestone

Comments

@ajdorenk
Copy link

ajdorenk commented Oct 31, 2019

Describe the bug
The FHIR spec states the following in regards to the _include search param: "When search results are paged, each page of search results should include the matching includes for the resources in each page, so that each page stands alone as a coherent package." (https://www.hl7.org/fhir/search.html#include). However, when I run a query such as Appointment?date=ge2019-07-05T18:00:00.000Z&date=le2019-07-06T18:00:00.000Z&_include=Appointment:patient&_count=15, I'm seeing behavior that doesn't match the spec. Specifically, out of the 43 total resources, there's 1 Practitioner resource, 21 Patient resources, and 21 Appointment resource.

There's two problems that I can tell:

  1. I'm only asking to include the patient field of the Appointment, so the related Practitioner resource should not be returned.
  2. For _count=15, according to the spec, I'd expect a mix of Appointment and related Patient resources to be returned. However, the first 15 resources returned are a Practitioner resource and 14 Patient resources... no Appointments. So, each page definitely does not "stand alone as a coherent package."

To Reproduce
Steps to reproduce the behavior:

  1. Perform a search for Appointment resources with _include=Appointment:patient as part of the query. Specify a count that's only a fraction of the total resources being returned.

Expected behavior
I would expect no Practitioner resource to be returned. Furthermore, I'd expect the first Appointment to be returned, followed by its related Patient resource. Then I'd expect the next Appointment and related Patient. Etc.

@ajdorenk ajdorenk added the bug Something isn't working label Oct 31, 2019
@lmsurpre lmsurpre added this to the Sprint 4 milestone Oct 31, 2019
@lmsurpre
Copy link
Member

lmsurpre commented Oct 31, 2019

Sounds like we actually have two issues here:

  1. paging for _incudes/_revinclude is not implemented per spec
  2. _incudes/_revinclude response is including types we don't want

@lmsurpre
Copy link
Member

lmsurpre commented Oct 31, 2019

For number 1, we currently execute one big COUNT query which attempts to count all of the resources (both the query ones and the included ones). I can't explain the reported behavior yet, but based on the spec it sounds like we should be looking at just the resource type being queried (which should actually simplify this query a lot).

After the COUNT we execute another big query that UNIONs all the different _include resource types. Instead, it might be simpler to move to executing two separate queries:

  • one to get the resource being queried (matching the COUNT query) and specifying the LIMIT or RANGE for the given page.
  • one to get all the resources that we should be including along with the resources returned from that first query. and this is also where we should be addressing number 2 above.

@albertwang-ibm
Copy link
Contributor

I used https://alberts-mbp.fios-router.home:9443/fhir-server/api/v4/Appointment?date=ge2019-07-05T18:00:00.000Z&date=le2019-12-06T18:00:00.000Z&_include=Appointment:patient&_count=15 in my local test.
These are what I found:
(1) the query generated is fine, there is no practitioner union there, I only find appointments and patient resource tables in the query.
SELECT RESOURCE_ID, LOGICAL_RESOURCE_ID, VERSION_ID, LAST_UPDATED, IS_DELETED, DATA, LOGICAL_ID
FROM
(SELECT R.RESOURCE_ID, R.LOGICAL_RESOURCE_ID, R.VERSION_ID, R.LAST_UPDATED, R.IS_DELETED, R.DATA, LR.LOGICAL_ID
FROM Appointment_RESOURCES R JOIN Appointment_LOGICAL_RESOURCES LR ON R.LOGICAL_RESOURCE_ID=LR.LOGICAL_RESOURCE_ID AND R.RESOURCE_ID = LR.CURRENT_RESOURCE_ID
WHERE R.IS_DELETED <> 'Y'
AND R.LOGICAL_RESOURCE_ID IN (SELECT LOGICAL_RESOURCE_ID FROM Appointment_DATE_VALUES WHERE (PARAMETER_NAME_ID=37 AND ((DATE_VALUE >= ?))))
AND R.LOGICAL_RESOURCE_ID IN (SELECT LOGICAL_RESOURCE_ID FROM Appointment_DATE_VALUES WHERE (PARAMETER_NAME_ID=37 AND ((DATE_VALUE <= ?))))

UNION ALL
SELECT R.RESOURCE_ID, R.LOGICAL_RESOURCE_ID, R.VERSION_ID, R.LAST_UPDATED, R.IS_DELETED, R.DATA, LR.LOGICAL_ID
FROM Patient_RESOURCES R JOIN Patient_LOGICAL_RESOURCES LR ON R.LOGICAL_RESOURCE_ID=LR.LOGICAL_RESOURCE_ID AND R.RESOURCE_ID = LR.CURRENT_RESOURCE_ID
WHERE R.IS_DELETED <> 'Y'
AND R.RESOURCE_ID = LR.CURRENT_RESOURCE_ID AND ('Patient/' || LR.LOGICAL_ID IN ('eLE3hAy2zo')))
COMBINED_RESULTS OFFSET 0 ROWS FETCH NEXT 15 ROWS ONLY :End prepared statement with 2 parameters begin parameter #1: 2019-07-05 14:00:00.0 :end parameter begin parameter #2: 2019-12-06 13:00:00.0 :end parameter
(2) the generated resources are also fine. I don't find any practitioner there.

(3) the paging issue is as excepted, because currently we paging all the returned resources together.

@albertwang-ibm albertwang-ibm self-assigned this Nov 4, 2019
albertwang-ibm added a commit that referenced this issue Nov 6, 2019
Signed-off-by: Albert Wang <xuwang@us.ibm.com>
albertwang-ibm added a commit that referenced this issue Nov 6, 2019
Signed-off-by: Albert Wang <xuwang@us.ibm.com>
albertwang-ibm added a commit that referenced this issue Nov 7, 2019
issue #346 paging fix for include/revinclude search
albertwang-ibm added a commit that referenced this issue Nov 7, 2019
Signed-off-by: Albert Wang <xuwang@us.ibm.com>
albertwang-ibm added a commit that referenced this issue Nov 7, 2019
Signed-off-by: Albert Wang <xuwang@us.ibm.com>
albertwang-ibm added a commit that referenced this issue Nov 7, 2019
Signed-off-by: Albert Wang <xuwang@us.ibm.com>
albertwang-ibm added a commit that referenced this issue Nov 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working search
Projects
None yet
Development

No branches or pull requests

3 participants