orchestracities / ngsi-timeseries-api

QuantumLeap: a FIWARE Generic Enabler to support the usage of NGSIv2 (and NGSI-LD experimentally) data in time-series databases
https://quantumleap.rtfd.io/
MIT License
38 stars 49 forks source link

Possible precision loss w/ Crate 4.4.+ geoqueries #483

Open c0c0n3 opened 3 years ago

c0c0n3 commented 3 years ago

This is just a reminder to check out if geo queries are still working with enough accuracy in Crate 4.4+. See this comment to PR #481

Geo queries use the location centroid column, not the location column, but the problem should be the same, i.e. we might have to use _doc[loc_centroid] instead of just the plain column name when building Crate geo queries.

amotl commented 3 years ago

Hi @c0c0n3,

I just want to point out https://github.com/orchestracities/ngsi-timeseries-api/issues/430#issuecomment-777781639 ff. in this context.

With kind regards, Andreas.

c0c0n3 commented 3 years ago

Hi @amotl, thanks for this, I'd just plain forgot about that! Thank goodness you're always around when we need you :-)

daminichopra commented 3 years ago

Hi @chicco785 @c0c0n3 , I have been trying to test for comment but not able to notify data to quantum leap using geo queries and not able to find proper usage of geo queries API . Please let me know if I am missing something. Also, I would like to contribute in this issue.

c0c0n3 commented 3 years ago

Hi @daminichopra thanks alot for looking into this! Geo-queries are documented in the API spec, then we've got Python docs too, e.g. https://github.com/orchestracities/ngsi-timeseries-api/blob/master/src/geocoding/slf/__init__.py and you can also look at the original pr #111...

daminichopra commented 3 years ago

@c0c0n3 , I will look into it. Thank you

daminichopra commented 3 years ago

Hi @c0c0n3 , Below are some observation points after tested geo queries with Crate 4.4+

  1. Regarding this comment we have to use _doc[loc_centroid] instead of just the plain column name
    select latitude(_doc['location_centroid']), longitude(_doc['location_centroid']) from ettest;

  2. Also, SELECT _doc['location'] instead of SELECT location

Note: Tested above queries with both crate 4.4.2 and 4.4.3

daminichopra commented 3 years ago

Soft Reminder!! Please let me know if any further investigation is needed to be done. Thank you

c0c0n3 commented 3 years ago

Hi @daminichopra!

Sorry for the slow reply, I was away on holiday and busy w/ other projects. Also I'll be away for another two weeks, but I'll try to catch up w/ QL issues in September.

Re: your investigation, that's great thank you so much. I don't think we need to dig deeper, since it looks like you've already got to the bottom of it :-)

github-actions[bot] commented 3 years ago

Stale issue message

daminichopra commented 2 years ago

Hi @c0c0n3 @chicco785 , Please close this issue if no further investigation required. Thank you

c0c0n3 commented 2 years ago

@daminichopra like I said you've done a thorough investigation and clarified what the situation actually is. Thanks alot for that :-) But shouldn't we keep this issue as a reminder to possibly update the Crate geo query generator (translators.crate_geo_query) as you suggested? That is, use _doc['location_centroid'] instead of the plain column name of location_centroid?