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.

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

Related objects in XML #768

Open
Zverik opened this issue Jun 23, 2014 · 21 comments
Open

Related objects in XML #768

Zverik opened this issue Jun 23, 2014 · 21 comments
Labels
api Related to the XML or JSON APIs

Comments

@Zverik
Copy link
Contributor

Zverik commented Jun 23, 2014

I propose to include a tag <parent type="..." ref="..." /> to nodes and ways requested directly (i.e. not by map call and not to xml extracts) which list ways and relations that contain given nodes and ways (and relations). Those tags go at the end of object definitions, right before object's closing tag.

This will allow editors to know when an object being edited (split, deleted) is a member of some other object, not present in downloaded area. And consequently this would greatly reduce number of broken relations and failed preconditions.

Since it does not change any existing XML tags, it is unlikely this would break any editors. I'm sure at least JOSM and iD (and my Level0) would support this tag from the start.

@tomhughes
Copy link
Member

I don't see what this has to do with the web site code?

@tomhughes
Copy link
Member

Unless you are proposing that we should add it to our responses, which I think is very unlikely.

@Zverik
Copy link
Contributor Author

Zverik commented Jun 23, 2014

This has nothing to do with web site code — I propose to change API XML output. Why it is unlikely? This won't affect map calls or extracts (e.g. planet dumps), just calls requesting individual objects.

@tomhughes
Copy link
Member

Well I was thinking that there is no way for the web site to efficiently determine the parents, but given your narrow definition of parent I guess it is possible.

It's probably something that should be discussed in a more general forum first though, especially as changing the API these days really means changing cgimap as much as it means changing the rails code.

@tomhughes
Copy link
Member

I guess there are two major questions here that we will need to answer - is the reduced performance (extra database lookups is bound to mean some loss of performance) worth the benefit gained and, if we do want to do it, is it something we would add now, or something to be considered in the next version of the API.

@Zverik
Copy link
Contributor Author

Zverik commented Jun 23, 2014

The problem with making the next version of API, it is too big. We would end up discussing for months a better way to represent relations so they are not broken ever again, and the area datatype would come up, and all the rainbow ponies stuff from that wiki page. We can make it small (i.e. I've extracted small and doable things which could go into API 0.7), but this would still need an effort of several programmers and a clear vision of the goal.

As for performance, we should test it, of course. I'm sure @pnorman will think of something, he's great at improving performance :)

I have posted the topic to dev@ for discussion.

@tomhughes
Copy link
Member

I don't mind one way or the other how big or small API 0.7 is, but if you want to propose an API change I would suggest coming to an EWG meeting on IRC rather than opening tickets.

@joto
Copy link

joto commented Jun 23, 2014

I think this would be an API change that needs to be discussed with a larger audience. Some parsers might not be able to cope with that change. And we have to keep in mind that the next thing people will then ask for is to have the same info in planet dumps. Would be quite useful there, too, to have this informaton. :-)

@simonpoole
Copy link
Contributor

IMHO I think it would be better to simply include the (parent-)objects in question if we want to support something in this direction, which in turn probably requires an additional flag to turn it on for backward compatability. Which Zveriks solution would need anyway see jotos comment (there is sure to be tons of stuff that will barf, correctly so).

Reasoning: having the parent object(s) avalable allows the editor to "do the right thing" instead of just popping up a warning which will mostly be ignored.

@pnorman
Copy link
Contributor

pnorman commented Jun 23, 2014

Failed preconditions aren't an issue with delete if not used. With splitting, yes.

As for impact, it's an additional join against way_nodes to backfill and at that point it's basically not different than a /full call in SQL required, with obviously less bandwidth.

To better understand the queries, it's probably easiest to look at cgimap, which has clearer queries.

@Zverik
Copy link
Contributor Author

Zverik commented Jun 23, 2014

Including complete parent objects would result in long and complex queries (while it can be supported). That's why no editor automatically calls /ways and /relations for every downloaded object. The proposed change simply notifies editors that there is something referencing the object, this way — because it is always included — editors cannot ignore it. "Simple" editors can even prohibit deleting an object if it is a member of anything not downloaded.

Including <parent> tags in planet dumps and map call responses is unncesessary, because all information is already there.

I see EWG meeting is today, I'll be there to discuss this proposal.

@pnorman
Copy link
Contributor

pnorman commented Jun 23, 2014

Including complete parent objects would result in long and complex queries (while it can be supported).

The long part of the query is the same if you want to either get the parents of an object or know if it has parents. The additional stage of going from knowing it has parents to including the parents adds relatively little query time.

@simonpoole
Copy link
Contributor

I don't quite see why the -query- would be more complex, yes the the result would be larger (if you happen to hit a mega relation or way). Note I wasn't suggesting returning all the elements of the parent objects in question, that is clearly not necessary.

@Zverik
Copy link
Contributor Author

Zverik commented Jun 23, 2014

Well, I'm not against Simon's proposal to add a new request for getting objects with their parents. But the point of this issue is to make such references mandatory.

I checked cgimap and apidb schema, and for relation members, all fields are indexed; for way_nodes table node ids are not, though. Does it mean that at least parent relations should be fast to query?

@tomhughes
Copy link
Member

It's current_way_nodes that matters, and that does have an index on the node ids.

@Zverik
Copy link
Contributor Author

Zverik commented Jun 23, 2014

Ah, sorry, missed the "other indexes" chapter. Then yes, why do we assume the penalty would be too big — it will be present, yes, but should be reasonable and, I hope, not exceed the gain.

@jfirebaugh
Copy link
Member

I'm not sure iD would be able to benefit much from this unless parents were included with the map call. It relies almost exclusively on the map call and almost never uses direct requests for individual entities. Introducing individual API calls and the asynchronicity they would require whenever an entity is edited is probably not feasible.

On the other hand, a map call that included parents would make openstreetmap/iD#2248 easy to fix.

@Zverik
Copy link
Contributor Author

Zverik commented Jun 23, 2014

I thought about adding parent objects and consider it a bad idea, regardless of simplicity and speed compared to just adding types and IDs. Because that brings us the problem of recursion: imagine a node that is contained in a way that is contained in a relation that is... and so on. The query would become very slow for such trees. So no, I'm still for adding types and ids.

@drolbr
Copy link
Contributor

drolbr commented Jun 24, 2014

By the way, what about e.g.

node(10381519); out meta; (way(bn);rel(bn);); out ids;

or

way(135216175); out meta; rel(bw); out ids;

in Overpass QL?
( for convenience http://overpass-turbo.eu/s/3Tn )

You could also query both the main API and Overpass API in parallel in case you are really cautious to get the right version. This would allow to keep main API call unchanged.

@Zverik
Copy link
Contributor Author

Zverik commented Jun 24, 2014

Roland, as for me, I'm quite cautious now with touching ways and nodes outside downloaded area. But not every mapper is like me: most just download an area in josm and get carried away while mapping. And then they spend extra hour on fixing relation conflicts. So while one can use Overpass API to query missing information, it would be better to have it in the source data from the start, so you can't break anything even if you don't understand OSM data structures.

@smsm1
Copy link
Contributor

smsm1 commented Jun 24, 2014

I use the auto downloader in JOSM so that I don't have a problem of not having the data downloaded for the area. Maybe that should be enabled by default, or it made harder in the editor to edit outside the downloaded area.

@mmd-osm mmd-osm added the api Related to the XML or JSON APIs label Jun 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Related to the XML or JSON APIs
Projects
None yet
Development

No branches or pull requests

9 participants