raksha-life / rescuekerala

Website for coordinating rehabilitation of people affected in the Kerala Floods
https://keralarescue.in
MIT License
676 stars 575 forks source link

Why are we still using char feild to store latlng, #408

Open vmwsree opened 6 years ago

vmwsree commented 6 years ago

Change charfield

We should be using an extra field store them as two floats or polyfield because to find nearest people through Django queries. other wise it has to be processed in frontend

batpad commented 6 years ago

+1 to this. Can we outline the work involved to do this? As I see it:

If setting up dependencies for geodjango is not something we want to do, even two float fields for lat and lng would suffice as @vmwsree suggests - steps above would be similar.

@vmwsree am sanjayb on the slack - feel free to ping me and we can try and see how to make this happen. There seems to be interest in this from a few people.

tachyons commented 6 years ago

Backward compatibility

batpad commented 6 years ago

Backward compatibility

This would be for data consumers, right? So the lat-lngs should be returned the same way as they currently are - this is on the /data end-point .. anywhere else?

tachyons commented 6 years ago

For the data which is already present in the database, there will be corrupted or non standard records in var char field. It is to be cleaned up before changing the field type, otherwise data will be lost

bobinson commented 6 years ago

any performance impact due to incorrect data type ?

batpad commented 6 years ago

It is to be cleaned up before changing the field type, otherwise data will be lost.

This will need some manual cleaning. Also, I would recommend keeping the existing field so the data can be extracted if needed.

any performance impact due to incorrect data type ?

So the reason to do this as I see it is to enable proximity / bounding box filtering for the map view. This will allow optimizations on the map page that would allow it to avoid having to fetch ALL data like it currently does. So, it's not an immediate impact - and refactoring to use spatial queries on the frontend would also likely be a bunch of work, but changing the data type will make that optimization possible.

The problem is that as the data grows, the request at /data is just going to get too big, regardless of any server-side caching, and the most obvious way to fix would be to do some sort of spatial queries -- like fetch very coarse data if the user is zoomed out and fetched more detailed data only at a higher zoom level or some such.

So, in short - this is only needed if we want to be able to spatial proximity queries, and otherwise is probably not a priority.

bobinson commented 6 years ago

So, in short - this is only needed if we want to be able to spatial proximity queries, and otherwise is probably not a priority.

+1