Node: latitude/longitude storage #4292
Conversation
ghost
assigned 
|
@jywarren, wherein the code do we save the latitude/longitude as tags? Okay, let's suppose we have successfully created new columns in the node table and now we have latitude/longitude as floats and their precision. But all old nodes just have the latitude/longitude saved as tags... So we can not change all the current methods, otherwise, we break what already exists. What is your suggestion regarding this? Should we try to recursively retrieve lat/lon of the old nodes and save in these new columns? Create new endpoints? |
|
We can make a migration which would select all tags starting with lat and
Lon, and add the extra columns. How's that sound?
…On Thu, Dec 13, 2018, 12:14 AM Camila Araújo ***@***.*** wrote:
@jywarren <https://github.com/jywarren>, wherein the code do we save the
latitude/longitude as tags?
Okay, let's suppose we have successfully created new columns in the node
table and now we have latitude/longitude as floats and their precision. But
all old nodes just have the latitude/longitude saved as tags... So we can
not change all the current methods, otherwise, we break what already exists.
What is your suggestion regarding this? Should we try to recursively
retrieve lat/lon of the old nodes and save in these new columns? Create new
endpoints?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#4292 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJ7bIGVzekP3aU7lUF7XFQ6gBHht8ks5u4eIqgaJpZM4ZQ7pm>
.
|
|
Sounds good... I just don’t know how to do it! Do you have an example? |
|
Yes, sorry - take a look at some of the migrations in |
milaaraujo
force-pushed
the
latitudeLongitudeStorage
branch
from
4f952ac to
fbd2293
Compare
|
Hey @jywarren, working with migrations is harder than I thought it would be... I was able to transfer the value of the tags to the new columns, but the problem now is that I do not know how to maintain the float precision! I've already tried using decimal and a bunch of other things, but nothing worked. Do you have any suggestion? |
|
Ah, this is actually why we are storing precision separately - float
precision is not precisely controllable. But we definitely should make the
lat/lon fields larger - there's a way to increase the precision of those
fields ... let me see; something like this:
db/migrate/20130114213934_add_profile_tags.rb: #add_column :users,
:lat, :decimal, :precision => 20, :scale => 10, :default => 0.0
db/migrate/20130114213934_add_profile_tags.rb: #add_column :users,
:lon, :decimal, :precision => 20, :scale => 10, :default => 0.0
Make sense? That way we store an excess of precision (the decimals
will be inconsistent at greater than 20 precision) but we use the
integer precision field to truncate them during use.
…On Tue, Dec 18, 2018 at 4:08 AM Camila Araújo ***@***.***> wrote:
Working with migrations is harder than I thought it would be... I was able
to transfer the value of the tags to the new columns, but the problem now
is that I do not know how to maintain the float precision!
MariaDB [publiclab_dev]> select nid, `latitude`, `longitude`, `precision` from node WHERE latitude != 0;
+-----+----------+-----------+-----------+
| nid | latitude | longitude | precision |
+-----+----------+-----------+-----------+
| 10 | 8.81467 | 17.3655 | 15 |
| 11 | 37.119 | 77.0585 | 14 |
| 12 | 16.0045 | 69.3345 | 12 |
| 13 | 15.8983 | 44.8083 | 14 |
| 14 | 32.9407 | 14.6519 | 15 |
| 15 | 29.9538 | 27.6056 | 14 |
| 16 | 61.4815 | 31.1145 | 15 |
| 17 | 74.9755 | 15.2274 | 15 |
| 18 | 35.6731 | 45.0279 | 15 |
| 19 | 66.5724 | 75.7069 | 13 |
| 20 | 64.0738 | 74.0497 | 14 |
| 21 | 4.28079 | 48.1806 | 13 |
| 22 | 79.3643 | 8.03875 | 15 |
| 23 | 10.6207 | 26.9991 | 15 |
| 24 | 51.1041 | 21.7141 | 15 |
| 25 | 69.2186 | 1.40931 | 16 |
| 26 | 35.0613 | 0.326677 | 16 |
| 27 | 3.52222 | 50.2646 | 14 |
| 28 | 51.3302 | 34.8903 | 14 |
| 29 | 28.0015 | 15.9763 | 15 |
| 30 | 68.8966 | 17.9573 | 15 |
| 31 | 70.5209 | 18.5898 | 14 |
| 32 | 22.0404 | 7.71761 | 15 |
| 33 | 30.8439 | 48.84 | 15 |
| 34 | 74.9298 | 23.681 | 15 |
| 35 | 53.3268 | 48.9159 | 15 |
| 36 | 43.8986 | 36.4432 | 15 |
| 37 | 17.1721 | 33.6177 | 14 |
| 38 | 50.7842 | 56.5169 | 15 |
| 39 | 55.7599 | 17.1723 | 14 |
| 40 | 55.1529 | 45.7677 | 14 |
| 41 | 68.8346 | 63.8693 | 14 |
| 42 | 1.08384 | 37.9133 | 14 |
| 43 | 23.0147 | 48.0152 | 15 |
+-----+----------+-----------+-----------+
I've already tried using decimal and a bunch of other things, but nothing
worked. Do you have any suggestion?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#4292 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJ_j3PmWdL3GVY9H5MR_zpn_LnluLks5u6LB2gaJpZM4ZQ7pm>
.
|
|
@milaaraujo jywarren, I think it is working now: What do you think? How do we merge migrations? |
|
Ah, I think one last step is to update our schema.rb.example to the latest schema - with the extra fields you can copy in from your newly generated schema. The timestamp checksum at the top of the file will also have to be updated, and once these are added, the tests should pass. |
|
For an example, try looking at the history for the schema.rb.example file! |
Generated by 🚫 Danger |
|
Done! |
milaaraujo
changed the title
|
Hummmm, all checks have passed, but we have a warning: I copied just the extra fields and the version from my newly generated schema to the schema.rb.example - not the whole file. Is it okay? |
|
Ok, now I copied all the db/schema.rb to schema.rb.example. Can we merge it? |
db/schema.rb.example
Outdated
| t.string "slug" | ||
| t.integer "legacy_views", default: 0 | ||
| t.integer "views", default: 0 | ||
| t.decimal "latitude", precision: 20, scale: 17 |
There was a problem hiding this comment.
OK, sorry i have been traveling, but this and the line below I think are the only lines needed except for the timestamp at top. The rest is mostly whitespace changes as well as some encoding changes we don't actually want to modify in this PR. Thanks, Camila!
There was a problem hiding this comment.
No problem :)
Is it ok now?
milaaraujo
force-pushed
the
latitudeLongitudeStorage
branch
from
b6f95d3 to
9869cda
Compare
|
Awesome! Great work. 👍 👍 👍 |
| update "UPDATE `node` SET `longitude` = '" + long.to_s + "' WHERE nid = '" + node_id.to_s + "'" | ||
| else | ||
| lati = tag["name"].gsub("lat:","") | ||
| update "UPDATE `node` SET `latitude` = '" + lati + "' WHERE nid = '" + node_id.to_s + "'" |
There was a problem hiding this comment.
Hi @milaaraujo it looks like this didn't run correctly on production for some reason -- odd that it didn't fail on stable, but i'm looking into it now. I think in general we may want to stick to non-raw SQL in the future if possible as there may be more safeguards... but i don't really know that this is the issue right now:
Mysql2::Error: Incorrect decimal value: 'false' for column 'latitude' at row 1
ActiveRecord::StatementInvalid: Mysql2::Error: Incorrect decimal value: 'false' for column 'latitude' at row 1: UPDATE `node` SET `latitude` = 'false' WHERE nid = '2120'
I wonder if some of the lat/lon tags are not supplying numbers!!??
There was a problem hiding this comment.
Aha! I think this tag exists, though not sure where: https://publiclab.org/tag/lat:false
OK, i'll try to manually remove it and re-run the migration. If that doesn't work, we may want to amend the migration to only run if the latitude/longitude values are actually numbers...
There was a problem hiding this comment.
OK, deleted lat:false as a tag. I tried scanning on the console for any other non-numerical ones and wasn't able to find any. Hopefully it'll run properly this time, restarting! Sorry for the scare! This is publishing to production now, exciting!!!
|
Yes! Whew!!! All good and published now!
…On Thu, Jan 3, 2019, 6:49 PM Camila Araújo ***@***.*** wrote:
***@***.**** commented on this pull request.
------------------------------
In db/migrate/20181219043340_add_fields_to_node.rb
<#4292 (comment)>:
> + def up
+ add_column :node, :latitude, :decimal, :precision => 20, :scale => 17
+ add_column :node, :longitude, :decimal, :precision => 20, :scale => 17
+ add_column :node, :precision, :integer
+
+ sql = "SELECT `node`.`nid`,`term_data`.`name` FROM `node` LEFT OUTER JOIN `community_tags` ON `community_tags`.`nid` = `node`.`nid` LEFT OUTER JOIN `term_data` ON `term_data`.`tid` = `community_tags`.`tid` WHERE (term_data.name LIKE 'lat:%' or term_data.name LIKE 'lon:%')"
+ tags_array = ActiveRecord::Base.connection.exec_query(sql)
+ tags_array.each do |tag|
+ node_id = tag["nid"]
+ if tag["name"].include? 'lon:'
+ long = tag["name"].gsub("lon:","").to_f
+ update "UPDATE `node` SET `precision` = '" + decimals(long).to_s + "' WHERE nid = '" + node_id.to_s + "'"
+ update "UPDATE `node` SET `longitude` = '" + long.to_s + "' WHERE nid = '" + node_id.to_s + "'"
+ else
+ lati = tag["name"].gsub("lat:","")
+ update "UPDATE `node` SET `latitude` = '" + lati + "' WHERE nid = '" + node_id.to_s + "'"
Did it work?
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#4292 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AABfJ1XC-Ti4-jJSFwx1Qb9Owp1O9vxuks5u_pb-gaJpZM4ZQ7pm>
.
|
* populating new fields * adding precision/scale * updating example schema
Fixes #4183