Opened 8 years ago

Closed 8 years ago

#6538 closed defect (fixed)

FID detection in GeoJSON results wrong feature count

Reported by: Bishop Owned by: warmerdam
Priority: normal Milestone: 2.1.1
Component: OGR_SF Version: 2.1.0
Severity: normal Keywords: geojson ogr
Cc:

Description

I have geojson file with "id" property and "ID" field.

In GDAL 1.x such file read correctly and return 45 features. In GDAL 2.1 I received only 1 feature. This is because ID field in all features have the same value. And GDAL prefer field value than json id property.

According to the specification "If a feature has a commonly used identifier, that identifier should be included as a member of the feature object with the name "id"" (http://geojson.org/geojson-spec.html#feature-objects)

It seems to me there may be simple fix here:

Change

OGRGeoJSONLayer::DefaultFIDColumn = "id"

To more rarely used ogc_fid (and not overlapped name as id)

OGRGeoJSONLayer::DefaultFIDColumn = "ogc_fid"

in https://github.com/OSGeo/gdal/blob/trunk/gdal/ogr/ogrsf_frmts/geojson/ogrgeojsonlayer.cpp#L44

Here it is test GeoJSON: http://opendata25.primorsky.ru/ngw/api/resource/176/geojson

Here it is ogrinfo output:

ogrinfo -so 176.geojson OGRGeoJSON
INFO: Open of `176.geojson'
      using driver `GeoJSON' successful.

Layer name: OGRGeoJSON
Geometry: Multi Polygon
Feature Count: 1
Extent: (14674348.198273, 5320781.295264) - (14676734.899287, 5322387.777885)
Layer SRS WKT:
PROJCS["WGS 84 / Pseudo-Mercator",
    GEOGCS["WGS 84",
        DATUM["WGS_1984",
            SPHEROID["WGS 84",6378137,298.257223563,
                AUTHORITY["EPSG","7030"]],
            AUTHORITY["EPSG","6326"]],
        PRIMEM["Greenwich",0,
            AUTHORITY["EPSG","8901"]],
        UNIT["degree",0.0174532925199433,
            AUTHORITY["EPSG","9122"]],
        AUTHORITY["EPSG","4326"]],
    PROJECTION["Mercator_1SP"],
    PARAMETER["central_meridian",0],
    PARAMETER["scale_factor",1],
    PARAMETER["false_easting",0],
    PARAMETER["false_northing",0],
    UNIT["metre",1,
        AUTHORITY["EPSG","9001"]],
    AXIS["X",EAST],
    AXIS["Y",NORTH],
    EXTENSION["PROJ4","+proj=merc +a=6378137 +b=6378137 +lat_ts=0.0 +lon_0=0.0 +x_0=0.0 +y_0=0 +k=1.0 +units=m +nadgrids=@null +wktext +no_defs"],
    AUTHORITY["EPSG","3857"]]
FID Column = ID
CODE: Integer (0.0)
SHAPE_AREA: Real (0.0)
NAME: String (0.0)
OBJECTID: Integer (0.0)
SHAPE_LENG: Real (0.0)
ID: Integer (0.0)

Change History (10)

comment:1 by Even Rouault, 8 years ago

Dmitry,

I see exactly the same behaviour when running "ogrinfo -ro -al -q -geom=summary 176.geojson" on both trunk and 1.11. I get 45 features reported. There's indeed a confusion between the ID property and id special field, but that shouldn't affect the feature count.

comment:2 by Bishop, 8 years ago

Replying to rouault:

I get 45 features reported. There's indeed a confusion between the ID property and id special field, but that shouldn't affect the feature count.

It's very strange. I looked what's happening under debugger:

  1. The SetFIDColumn for "ID" appeared here: https://github.com/OSGeo/gdal/blob/trunk/gdal/ogr/ogrsf_frmts/geojson/ogrgeojsonreader.cpp#L449
  1. Set OGRFeature FID to "O" appeared here: https://github.com/OSGeo/gdal/blob/trunk/gdal/ogr/ogrsf_frmts/geojson/ogrgeojsonreader.cpp#L874
  1. Deleting previous OGRFeature with FID == 0 appeared here: https://github.com/OSGeo/gdal/blob/trunk/gdal/ogr/ogrsf_frmts/mem/ogrmemlayer.cpp#L331

I tested GDAL 2.1, not trunk. But, it seems to me, that current changes in trunk in geojson driver, don't affect this behavior.

Also, I see the wrong behavior here: https://github.com/OSGeo/gdal/blob/trunk/gdal/ogr/ogrsf_frmts/geojson/ogrgeojsonreader.cpp#L620

GeoJSON specification, as I quoted upper, not defined that type should "id" property have (not exactly string).

I think in this place some variable (i.e. bool bHasFIDProperty) should be set. And if bHasFIDProperty == true, not SetFID from field value. But what should be with FIDColumn? The simplest way is to undefined it - SetFIDColumn("").

comment:3 by Even Rouault, 8 years ago

Apologies, I thought I was tested against trunk, but I wasn't. I can reproduce with trunk.

comment:4 by Even Rouault, 8 years ago

Resolution: fixed
Status: newclosed

In 34330:

GeoJSON: fix wrong behaviour when there's a 'id' at Feature level and 'id' or 'ID' field in properties (closes #6538)

comment:5 by Even Rouault, 8 years ago

In 34331:

GeoJSON: fix wrong behaviour when there's a 'id' at Feature level and 'id' or 'ID' field in properties (closes #6538)

comment:6 by Even Rouault, 8 years ago

Milestone: 2.1.1
Version: unspecified2.1.0

comment:7 by Even Rouault, 8 years ago

In 34333:

GeoJSON: fix memleak introduced in previous commit (references #6538)

comment:8 by Even Rouault, 8 years ago

In 34334:

GeoJSON: fix memleak introduced in previous commit (references #6538)

comment:9 by Bishop, 8 years ago

Resolution: fixed
Status: closedreopened

GeoJSON specification not limit FID with only positive numbers. I tested current trunk and get this result:

$ ogrinfo 81.geojson
ERROR 6: negative FID are not supported
...
ERROR 6: negative FID are not supported
ERROR 6: negative FID are not supported
More than 1000 errors or warnings have been reported. No more will be reported from now.
INFO: Open of `81.geojson'
      using driver `GeoJSON' successful.
1: OGRGeoJSON

Аnd in

ogrinfo -al -so -ro 81.geojson OGRGeoJSON
ERROR 6: negative FID are not supported
...
ERROR 6: negative FID are not supported
ERROR 6: negative FID are not supported
More than 1000 errors or warnings have been reported. No more will be reported from now.
INFO: Open of `81.geojson'
      using driver `GeoJSON' successful.

Layer name: OGRGeoJSON
Geometry: Unknown (any)
Feature Count: 0
Layer SRS WKT:
GEOGCS["WGS 84",
    DATUM["WGS_1984",
        SPHEROID["WGS 84",6378137,298.257223563,
            AUTHORITY["EPSG","7030"]],
        AUTHORITY["EPSG","6326"]],
    PRIMEM["Greenwich",0,
        AUTHORITY["EPSG","8901"]],
    UNIT["degree",0.0174532925199433,
        AUTHORITY["EPSG","9122"]],
    AUTHORITY["EPSG","4326"]]
name: String (0.0)
label: String (0.0)
address: String (0.0)
paramJsonAll: String (0.0)
objectId: Integer (0.0)
paramJsonFull: String (0.0)

The test geojson file link: https://drive.google.com/open?id=0BzlLlHyrgQHkcTEtRGRQNjdrcTA

According to GDAL architecture and methods like virtual OGRFeature * GetFeature (GIntBig nFID) maybe we need to set own (internal GDAL) id, and geojson id move to extra JSon members?

Another more simple fix - just let to use negative ID's. GIntBig support negative numbers.

comment:10 by Even Rouault, 8 years ago

Resolution: fixed
Status: reopenedclosed

trunk r34361, branches/2.1 r34362 "GeoJSON: in case top level id is a negative integer, put the value in a 'id' attribute (#6538)"

Note: See TracTickets for help on using tickets.