[OSM-dev] PBF for History-Files (was: visible-Flag in PBF)

Scott Crosby scrosby06 at gmail.com
Thu May 12 00:05:40 BST 2011


On Tue, May 10, 2011 at 11:55 AM, Jochen Topf <jochen at remote.org> wrote:
> On Mon, May 09, 2011 at 05:21:40PM -0500, Scott Crosby wrote:
>> On May 8, 2011 5:21 AM, "Jochen Topf" <jochen at remote.org> wrote:

>> > So I propose the following:
>> >
>> > 1. Add the visible flag as I have already proposed:
>> > =========================================
>> > --- a/src/osmformat.proto
>> > +++ b/src/osmformat.proto
>> > @@ -126,6 +126,7 @@ message Info {
>> > optional int64 changeset = 3;
>> > optional int32 uid = 4;
>> > optional uint32 user_sid = 5; // String IDs
>> > + optional bool visible = 6 [default = true];
>> > }
>> >
>> > /** Optional metadata that may be included into each primitive. Special
>> dense format used in DenseNodes. */
>> > @@ -135,6 +136,7 @@ message DenseInfo {
>> > repeated sint64 changeset = 3 [packed = true]; // DELTA coded
>> > repeated sint32 uid = 4 [packed = true]; // DELTA coded
>> > repeated sint32 user_sid = 5 [packed = true]; // String IDs for usernames.
>> DELTA coded
>> > + repeated bool visible = 6 [packed = true];
>> > }
>>
>> I agree with this in principal. I veto this implementation
>> strategy. Info/DenseInfo are designed so that they could be stripped out
>> without affecting geographic information. As the visibility flag is critical
>> to understanding the map, it cannot be placed in these sections. I'm
>> prepared to commit this patch later this week:
>>
>>
>> diff --git a/src/osmformat.proto b/src/osmformat.proto
>> index 44e24f7..b9c17b2 100644
>> --- a/src/osmformat.proto
>> +++ b/src/osmformat.proto
>> @@ -165,6 +165,7 @@ message Node {
>>
>>     required sint64 lat = 8;
>>     required sint64 lon = 9;
>> +   optional bool deleted = 11;
>>  }
>>
>>  /* Used to densly represent a sequence of nodes that do not have any tags.
>> @@ -190,6 +191,7 @@ message DenseNodes {
>>
>>     // Special packing of keys and vals into one array. May be empty if all
>> node
>>     repeated int32 keys_vals = 10 [packed = true];
>> +   repeated bool deleted = 11;
>>  }
>>
>>
>> @@ -202,6 +204,7 @@ message Way {
>>     optional Info info = 4;
>>
>>     repeated sint64 refs = 8 [packed = true];  // DELTA coded
>> +   optional bool deleted = 11;
>>  }
>>
>>  message Relation {
>> @@ -222,5 +225,6 @@ message Relation {
>>     repeated int32 roles_sid = 8 [packed = true];
>>     repeated sint64 memids = 9 [packed = true]; // DELTA encoded
>>     repeated MemberType types = 10 [packed = true];
>> +   optional bool deleted = 11;
>>  }
>>
>>
>> Opinions?
>
> It repeats information that should be defined once in my opinion.
>
> It also reverses the meaning of the well-established "visible" flag by calling
> it "deleted".
>

> And it doesn't make sense to not have the Info message in a history file, but
> have the deleted (or visible) flag alone, because you need the version
> information to find out which version of an object you need. There can be
> several versions of the same object with visible=true. If you want to find out
> the current/last version of an object you need the one with the highest version
> number if visible=true. If you want to find a version at a different point in
> time you need the timestamp.  So the visible attribute is no different than the
> version and/or timestamp attributes, really. At least in the case of the
> history file.

>
> So I don't see the case where it would make sense to leave out the Info message,
> but keep the visible or deleted flag.

My reason for putting the invisible flag outside of the Info/DenseInfo
goes away. I was unaware of the full semantics of history files. You
need the version number and timestamp to decode them. With these
semantics, we'll need to include a requirement that PBFH files must
not omit Info or DenseInfo.  (I'm not sure I entirely like those
semantics, but that's another issue.)

I withdraw my veto, but I'd like to see what comes out of Brett's
inquiry about OSM changeset and OSM history files before I apply your
patch. Thank you for explaining things.

>>
>> I propose defining a new 'required_feature' of:
>>    ContainsDeletedHistoricalInformation
>
> Maybe better "ContainsHistoricalInformation". I think the "deleted" is
> confusing, because it contains not just deleted information but also old
> versions of the objects.

Fine.

>
>> The rules is:
>>
>>    A file without required_feature ContainsDeletedHistoricalInformation is
>> invalid if it contains anything in the 'invisible' fields. A conforming
>> reader MUST reject any such file.
>
> I'd say:
> * A conforming writer MUST NOT write this field.
> * A conforming reader MUST ignore this field.

>
> That puts the burden on the writer to do the right thing and makes the reader
> a) more robust and

Wouldn't files with the invisible field but without the
required_feature flag be broken? I don't see why a reader could have
any confidence it is reading the file correctly. It should reject
those files.

>>    A file with required_feature ContainsDeletedHistoricalInformation MUST
>> have an invisible flag for each entity in each block.
>
> Sounds good.
>
>> In addition:
>>
>>   Software is encouraged to support files that
>> contain ContainsDeletedHistoricalInformation wherever feasible. By, for
>> instance, ignoring any 'deleted' entity.
>
> No. That is not enough to understand historical information. It is much more
> complicated to deal with historical information so we should not put any rules
> on this in the format description.

Yes. I concur, the semantics of OSM history files are not what I
thought they were.

Scott



More information about the dev mailing list