Hi Frank On Wed, Jul 21, 2010 at 4:45 PM, Frank Warmerdam <[email protected]> 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 _______________________________________________ gdal-dev mailing list [email protected] http://lists.osgeo.org/mailman/listinfo/gdal-dev
