[gdal-dev] RFC 29: OGR Set Desired Fields

[Martin Dobias]

Hi Frank

On Wed, Jul 21, 2010 at 4:45 PM, Frank Warmerdam <warmerdam at pobox.com> wrote:
> Martin Dobias wrote:
>>
>> Hi,
>>
>> based on the discussion about optimizing the performance of reading
>> features in OGR few days ago, I've prepared an RFC that proposes API
>> for limiting which fields will be fetched when reading features:
>>
>> http://trac.osgeo.org/gdal/wiki/rfc29_desired_fields
>>
>> Any comments or suggestions are highly welcome.
>
> Martin,
>
> Some thoughts:
>
> 1) Should there be a TestCapability() value indicating whether
> the field selection option works for a particular driver?  It
> doesn't seem critical to me.

I don't think it is necessary either.


> 2) Should we try to overload this function to also allow
> selection or deselection of the "special" fields geometry
> and style?  We try to treat them a bit like fields in
> things like http://trac.osgeo.org/gdal/wiki/rfc6_sqlgeom.

There are use cases where ignoring the geometry would save some time,
so it would be fine if the client could decide whether to fetch it or
not...


> 3) I dislike the passing of an unnecessary nNumFields
> parameter and unintuitive use of it as a special flag.

Agreed, I was in doubt whether to include it or not anyway.

> 4) I think you need to make it clear whether where the
> method is implemented.  I get the impression the OGRLayer
> class will implement the method, and the OGRLayer class
> will keep around a desired fields array.

Yes, that was my idea.

> Should driver
> specific classes be able to override this method to detect
> when the desired list changes?   I think so, even though
> most drivers using the desired fields functionality will
> not need to do so.

I thought that overriding of the method will not be necessary. But
there's no particular reason why the method couldn't be virtual...


> 5) I'd like the RFC to be clear that some (many) drivers
> may not support this functionality in which case all
> fields will be returned instead of some being null.

It's vaguely mentioned in the Compatibility section. I'll make it more clear.


> As an alternative, if we would like to support the "special
> fields" for geometry and style perhaps we should offer
> a method like:
>
> virtual OGRErr OGRLayer::SetIgnoredFields( const char **papszFields );
>
> The argument is a list of fields to be ignored, by name, and
> the special field names "OGR_GEOMETRY" and "OGR_STYLE" will
> be interpreted to refer to the geometry and style values of
> a feature.
>
> Passing NULL for papszFields will clear the ignored list.
>
> The method will return OGRERR_NONE as long as all the field
> names are able to be resolved, even if the method does not
> support selection of fields.
>
> I chose passing by field name so that we could handle
> OGR_GEOMETRY, OGR_STYLE and possibly some other special
> fields in the future.  I changed the sense to "ignored"
> so that we wouldn't accidentally drop things like
> geometry and style just because they weren't explicitly
> listed in a desired list.

This makes sense to me.

How do you see the implementation? Should OGRLayer contain the array
m_pabIgnoredFields of booleans similar to the original proposal,
together with booleans for the special fields, such sa
m_bIgnoreGeometry, m_bIgnoreStyle?


> Note, I'm willing to implement this functionality for a
> bunch of drivers I think might benefit, and I also think
> the RFC should indicate we will connect this to the
> -select option of ogr2ogr.

Ok, I will add that.

So, in case there will be no further comments, I'll update the RFC to
match the API you've proposed.
Thanks a lot for your thoughts.

Regards
Martin