Openstreetmap-website: Give the changesets_count in changeset metadata

Created on 13 May 2019  路  19Comments  路  Source: openstreetmap/openstreetmap-website

iD adds a "changesets_count" tag to every changeset it creates. This can be used by QA tools, and is intended as a hint to other mappers when dealing with mistakes.

I'm in favour of the concept (of providing additional context to the changeset), but I'm not keen on the current implementation. It is not additional data, it's just using the changeset tags to display information that is already held elsewhere in the database. It's a close equivalent of adding a "latitude" tag to a node.

So we should show the changesets_count in the changeset metadata, and avoid having to store all these additional tags in the database. Doing so centrally will also mean that the information is available for every changeset, and avoids every other editor from having to duplicate the logic from iD.

We could do this in two different ways:

  • With a sufficiently smart SQL query that, for a given changeset, works out which changeset this is for the given user, or
  • Adding a changeset_count bigint to the changesets table

The second one seems like it's also duplicating data, but at least it would be a lot less overhead to store an extra integer per changeset than an extra changeset_tag record!

Most helpful comment

You can of course also include the total number of edits made by a user, to have a bit more context:

edits

All 19 comments

https://wiki.openstreetmap.org/wiki/API_v0.6#Details_of_a_user already returns the number of changesets, and it is available via the web UI (even when not logged on).

I suppose one could argue that iD provides a tad more information (the number of changesets when the changeset was created), but it doesn't seem to be that useful really except if you want to put somebody on a pillory for past sins.

but it doesn't seem to be that useful really except if you want to put somebody on a pillory for past sins.

or what if we just.. made new people feel welcome..

but it doesn't seem to be that useful really except if you want to put somebody on a pillory for past sins.

or what if we just.. made new people feel welcome..

As said the current changeset count is already known, including that for newbies, so it can only be of use if you want to go back in history to a point in time when the changeset count was substantially different than the current state.

the number of changesets when the changeset was created

Exactly, that's what I want to recreate

but it doesn't seem to be that useful

I don't want to argue too much about whether it's useful or not here. It's being done already, I just want to focus on the work of doing it better from a technical point of view.

Ignoring the question of whether it's useful or not I can't say I'm especially keen on adding it as builtin metadata, at least under that name. Frankly I see it as confusing to return something purporting to be a changeset count that is actually a snapshot of the historical count at the time that changeset was created.

What it actually amounts to is a per-user sequence number for the changeset and if we were going to add it I would to name it along those lines so that it's purpose/meaning was clearer.

nb: changeset metadata call is currently in CGImap land (e.g. https://api.openstreetmap.org/api/0.6/changeset/12). If there's some consensus here, can you please also open up an issue in the cgimap repo?

What about changeset replication on https://planet.openstreetmap.org/replication/changesets?

but it doesn't seem to be that useful really except if you want to put somebody on a pillory for past sins.

Exactly the reverse actually - with a DWG hat on I often say to people "by all means raise it with (some other user), but you do realise that (some changeset) was only their 5th edit, so please be nice to them".

@bhousel Since you added this feature to iD originally, I want to check with you that you're happy with the general proposal (moving the data from changeset tags to changeset metadata)? I don't want to do this and then find out that there's some reason for iD to stick to the tags-based method!

@bhousel Since you added this feature to iD originally, I want to check with you that you're happy with the general proposal (moving the data from changeset tags to changeset metadata)? I don't want to do this and then find out that there's some reason for iD to stick to the tags-based method!

@gravitystorm - You could focus your attention on a lot of things today. Why this?

Obsoleting a changeset tag that iD uses that you don't like (and that the DWG finds useful). That's really the thing you want to work on right now?

(for the avoidance of doubt) I'd find this info _more_ useful in changeset metadata rather than changeset tags, since it'd be there for all editors.

@gravitystorm I apologize - my previous comment was mean. Yes I would be ok if you added this to the changeset metadata.

_iD still will add whatever tags we want to for things that people find useful._ I catch a lot of shit for the tags that iD uses, and I realize that that's not what you were intending with this issue..

We already have a secondary index on fields user_id and created_at for the changesets table.

Maybe we could leverage this index and thus avoid adding an extra db field, assuming an index range scan is fast enough to deliver the number of changesets created before a given point in time.

This would significantly reduce the implementation cost associated with having some extra db fields.

explain verbose select count(user_id) from changesets where user_id = 1 and created_at < '2019-02-08 16:56:26.117157';
                                               QUERY PLAN                                                
---------------------------------------------------------------------------------------------------------
 Aggregate  (cost=3.85..3.86 rows=1 width=8)
   Output: count(user_id)
   ->  Index Scan using changesets_created_at_idx on public.changesets  (cost=0.27..3.84 rows=6 width=8)
         Output: user_id
         Index Cond: (changesets.created_at < '2019-02-08 16:56:26.117157'::timestamp without time zone)
         Filter: (changesets.user_id = 1)


You could focus your attention on a lot of things today. Why this?

FWIW, I saw a discussion on the talk mailing list archives that reminded me about this topic, something that I thought about a while ago. I thought I should log the ticket while I remembered. It wasn't intended to blow up in any way, it's not an urgent task, and I'm sorry that it has caused you hassle.

It also wasn't the only thing I worked on yesterday, since I was also discussing spam prevention with the sysadmins, and changes to the division of labour of website moderation with DWG. Both of those were via email though, so perhaps it looked like I was picking on iD yesterday. Not my intention either.

An finally, endlessly refactoring the codebase gets dull after a while, and sometimes I want to think about shiny new problems. Scratching an itch and all that.

I catch a lot of shit for the tags that iD uses

Sure, I understand. We all seem to catch a lot of shit for all sorts of reasons. It sucks.

_iD still will add whatever tags we want to for things that people find useful._

Sure, I understand that too. I didn't want this topic to be some kind of criticism of iD, more of a recognition of a useful idea, and a way to roll it out to all editors and all changesets while also making slight optimisations in database usage. But perhaps other people will take it differently.

nb: changeset metadata call is currently in CGImap land (e.g. https://api.openstreetmap.org/api/0.6/changeset/12). If there's some consensus here, can you please also open up an issue in the cgimap repo?

What about changeset replication on https://planet.openstreetmap.org/replication/changesets?

Yep, when we nail down the details here, we'll roll it out to the other things that read directly from the database (like cgimap and replication).

We already have a secondary index on fields user_id and created_at for the changesets table.

I'm not sure why your example used the changesets_created_at_idx instead of the changesets_user_id_created_at_idx? Maybe it's just postgres not caring because there's not enough data in that db?

A challenge will be figuring out a nice way to make this available in the rails models as an attribute for the changeset.

Ah, yes, that鈥檚 the wrong index due to lack of data.

https://medium.com/@eric.programmer/the-sql-alternative-to-counter-caches-59e2098b7d7 - not quite the same topic, but interesting info.

In the interest of keeping things as simple as possible, maybe only return this info as part of the HTML page first (e.g. https://www.openstreetmap.org/changeset/1). That should be just fine for the DWG use case above, unless they're really keen on API and XML fiddling. If there's some further demand for an API solution, do that in a second step only.

Now with 100k table entries, explain result looks like it should.

explain verbose select count(user_id) from changesets where user_id = 1220430 and created_at < '2019-04-08 16:56:26.117157';
                                                                  QUERY PLAN                                                                  
----------------------------------------------------------------------------------------------------------------------------------------------
 Aggregate  (cost=4.44..4.45 rows=1 width=8)
   Output: count(user_id)
   ->  Index Only Scan using changesets_user_id_created_at_idx on public.changesets  (cost=0.42..4.44 rows=1 width=8)
         Output: user_id, created_at
         Index Cond: ((changesets.user_id = 1220430) AND (changesets.created_at < '2019-04-08 16:56:26.117157'::timestamp without time zone))

You can of course also include the total number of edits made by a user, to have a bit more context:

edits

Was this page helpful?
0 / 5 - 0 ratings