Skip to content

Feature request: Order Location searches using near in closest distance to center of the box #4650

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

Closed
jkiddo opened this issue Mar 16, 2023 · 11 comments · Fixed by #4687
Closed

Comments

@jkiddo
Copy link
Contributor

jkiddo commented Mar 16, 2023

Currently, the searches on Location using near are not returned in the order closest to the center. It would be awesome if that could be done - either using the _sort parameter or just default to that order (aka. Manhatten distance).

@jkiddo
Copy link
Contributor Author

jkiddo commented Mar 17, 2023

Something like (if we go the circle way):

SELECT t0.RES_ID FROM HFJ_SPIDX_COORDS t0 WHERE ((t0.HASH_IDENTITY = ?) AND (6371 * acos(cos(radians(t0.SP_LATITUDE >= ?)) * cos(radians(center_lat = ?)) * cos(radians(center_lon) - radians(t0.SP_LONGITUDE = ?)) + sin(radians(t0.SP_LATITUDE = ?)) * sin(radians(t0.SP_LATITUDE = ?))))) AS distance ORDER BY distance ASC

or if continuing with the box search:

SELECT latitude, longitude, ABS(lat - :center_lat) + ABS(lon - :center_lon) AS manhattan_distance FROM your_table WHERE lat BETWEEN :box_lat1 AND :box_lat2 AND lon BETWEEN :box_lon1 AND :box_lon2 ORDER BY manhattan_distance ASC

@brianpos
Copy link

If the database underneath has native support that will be far better.

@jkiddo
Copy link
Contributor Author

jkiddo commented Mar 18, 2023

Quick search show that postgres, ms and h2 all support cosine functions

@jkiddo
Copy link
Contributor Author

jkiddo commented Mar 24, 2023

@brianpos do you mean that its a problem to let the database do Cosine operations?

@jamesagnew
Copy link
Collaborator

This is a neat idea.

I think personally I'd prefer for it to only sort this way if a _sort is provided, given that i'd expect this would almost certainly add a measurable slowdown.. but if you explicitly chose to sort on a near SP, this feels exactly like what would be the commonsense expected behaviour....

@jamesagnew
Copy link
Collaborator

Just adding to this, @jkiddo do you have an opinion on the circle vs box part?

We did think about implementing the circle math back when we originally built the near support, but we ultimately picked the box model because we figured it was likely to perform much better in a cross-database way.

We've never really had any feedback on that decision though. I know a bunch of people who use this feature, but it's not obvious that it's the optimal solution from a "how would I expect this to work" kind of perspective.

@jkiddo
Copy link
Contributor Author

jkiddo commented Mar 24, 2023

It's safe to say that the box model way is probably the most cross-database friendly way to do it for sure (fun fact: the SQL is the same across PostgresQL and H2 AFAIK). Whether its a box or a circle is my least important aspect of the feature request. Most important to me is the sort part. @jamesagnew I totally agree that the proper way to do it is using a _sort-provided parameter. I could REALLY use the sort part in a setup of mine - and I think it makes a lot of sense to either sort it as closest to center (the most common pick) or farthest to center.

@jkiddo
Copy link
Contributor Author

jkiddo commented Mar 27, 2023

That PR looks amazing!

While on this topic - there is also the http://hl7.org/fhir/R4/location.html#positional (R4) / https://www.hl7.org/fhir/location.html#8.7.6.1.1 (R5) extension mentioned on the search results:

<entry> 
    <resource>
        <Location>
            <!-- location details -->
        </Location>
    </resource>
    <search>
        <extension url="http://hl7.org/fhir/StructureDefinition/location-distance">
            <valueDistance >
                <!-- The distance that this location resource is from the provided point in the query -->
                <value value="10.5"/>
                <unit value="km"/>
            </valueDistance>
        </extension>
    </search>
</entry> 

I tried to shoehorn that into the results using an interceptor when querying but I quickly found that using that approach only worked for the first result page (as it was evaulated only on /Location searches) and not _getpages . Would it be too cumbersome to also include that (I imagine that is probably way harder to include as well)?

@jkiddo
Copy link
Contributor Author

jkiddo commented Mar 29, 2023

@jamesagnew did you have a look at my last comment?

@jamesagnew
Copy link
Collaborator

I did - but the change was already under review and I think adding the distance extension to search results would be a bigger change in order to do it right. Would probably need some work on the search result bundle generation infrastructure so I think it deserves its own ticket.

@jkiddo
Copy link
Contributor Author

jkiddo commented Mar 29, 2023

@jamesagnew Makes perfect sense! Thank you so much for the improvement! (Would you like me to create a new ticket for that?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants