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.

Sign up for GitHub

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

Jump to bottom

Incorrect query being generated for radius-based location (near) search #2879

Closed
lmsurpre opened this issue Oct 20, 2021 · 1 comment
Closed

Incorrect query being generated for radius-based location (near) search #2879

lmsurpre opened this issue Oct 20, 2021 · 1 comment
Assignees
@lmsurpre
Labels
bug Something isn't working search
Milestone
Sprint 2021-14

Comments

@lmsurpre
Copy link
Member

Describe the bug
While updating the unit tests as part of #2728 I noticed that SQL being generated by the NewLocationParmBehaviorUtil is slightly different from the query from the old LocationParmBehaviorUtil

query diff

Environment
main

To Reproduce
Steps to reproduce the behavior:

  1. Update NewLocationParmBehaviorUtilTest.runTestBoundingRadius to use NewLocationParmBehaviorUtil and execute the test

Expected behavior
the query should match the old query (at least in function if not in form)

Additional context
original implementation was based on this: http://janmatuschek.de/LatitudeLongitudeBoundingCoordinates
@lmsurpre lmsurpre added the bug Something isn't working label Oct 20, 2021
@lmsurpre lmsurpre self-assigned this Oct 20, 2021
@lmsurpre lmsurpre added this to the Sprint 2021-14 milestone Oct 20, 2021
@prb112 prb112 added the search label Oct 20, 2021
lmsurpre added a commit that referenced this issue Oct 21, 2021
@lmsurpre
issue #2879 - fix last operand in arcRadius for radius search
74b6f14 
should be cosine, not arccosine

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
@lmsurpre lmsurpre mentioned this issue Oct 21, 2021
Merged
lmsurpre added a commit that referenced this issue Oct 21, 2021
@lmsurpre
issue #2879 - fix last operand in arcRadius for radius search
c200330 
should be cosine, not arccosine

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Oct 21, 2021
@lmsurpre
issue #2879 - fix last operand in arcRadius for radius search
3043f0e 
should be cosine, not arccosine

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Oct 21, 2021
@lmsurpre
issue #2879 - fix last operand in arcRadius for radius search
5a71eb4 
should be cosine, not arccosine

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Oct 21, 2021
@lmsurpre
issue #2879 - revamp radius-based distance calculation
2fa6fb2 
I don't think this was ever working right. Now I think it at least
follows the formulas laid out at

Depends on the RDBMS supporting the `RADIANS` function (which supposedly
works the same across at least Derby, Db2, and Postgresql).

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Oct 21, 2021
@lmsurpre
issue #2879 - revamp radius-based distance calculation
bf8983a 
I don't think this was ever working right. Now I think it at least
follows the formulas laid out at

Depends on the RDBMS supporting the `RADIANS` function (which supposedly
works the same across at least Derby, Db2, and Postgresql).

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Oct 21, 2021
@lmsurpre
issue #2879 - revamp radius-based distance calculation
47a1654 
I don't think this was ever working right. Now I think it at least
follows the formulas laid out at

Depends on the RDBMS supporting the `RADIANS` function (which supposedly
works the same across at least Derby, Db2, and Postgresql).

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Oct 21, 2021
@lmsurpre
issue #2879 - revamp radius-based distance calculation
50bd801 
I don't think this was ever working right. Now I think it at least
follows the formulas laid out at

Depends on the RDBMS supporting the `RADIANS` function (which supposedly
works the same across at least Derby, Db2, and Postgresql).

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
lmsurpre added a commit that referenced this issue Oct 22, 2021
@lmsurpre
Merge pull request #2885 from IBM/issue-2879
4da0495 
issue #2879 - revamp radius-based distance calculation
@prb112
Copy link
Contributor

prb112 commented Oct 25, 2021

Created locations at -89,89 and moved to 111.20KM distance -88,89.
https://localhost:9443/fhir-server/api/v4/Location?near=88.0|-89.0|111.20|km

However, 88,-88 with 111.22
https://localhost:9443/fhir-server/api/v4/Location?near=88.0|-88.0|111.22|km

It's slightly off, it's OK as it's unlikely to have such a big impact when it's in the lower meridians.

QA Complete.

@prb112 prb112 closed this as completed Oct 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@lmsurpre lmsurpre

Labels
bug Something isn't working search
Projects
None yet
Milestone
Sprint 2021-14
Development

No branches or pull requests
2 participants
@prb112 @lmsurpre