[Gdal-dev] RFC 6: OGR support for querying and selection by geometry and feature style

[Tamas Szekeres]

Frank,

I've added the proposed changes to RFC6_QUERYGEOM.dox, and prepared
the necessary patch against the current CVS version, and made the
corresponding tests.

To make the things easier I've slightly changed the negotiated concept as:

1. Instead of adding the new functions GetSpecialFieldAs* I've added the special
field accessors to the existing GetFieldAs* functions.
2. I've made the necessary changes to transfer the static string in
GetFieldAsString
to a member value so as to make the code thread safe. The destructor
of the OGRFeature
will finally destroy this string.
3. According to #2 I've changed your responsibility during the
implementation. And you might want to consider how the
OGRFeature::GetFieldAsString changes will affect the other parts of
the OGR code.

Since the necessary changes are ready to commit if I get the green
lamp the feature can be added to the upcoming 4.10.0.

Best Regards,

Tamas Szekeres



2006/10/25, Frank Warmerdam <warmerdam at pobox.com>:
> Tamas Szekeres wrote:
> >>   2) I'd like to be able to register a function to fetch the special
> >>      fields so we don't have to have the special fetch code appearing
> >>      in so many places.  At the very least a way of registering a
> >> function
> >>      in an array parallel to the SpecialFieldNames[] and
> >> SpecialFieldTypes[]
> >>      arrays. But perhaps it would be better to actually have methods
> >> on the
> >>      OGRFeature like:
> >>         const char *GetSpecialFieldAsString( int nSpecialFieldCode );
> >>         int         GetSpecialFieldAsInteger( int nSpecialFieldCode );
> >>
> >
> > No problem about adding the field value accessor function for every special
> > field. I have already been thinking of it but would not want to
> > disturb the existing
> > code so much. Anyway it would make the things more clear with the overhead
> > of an extra function call.
>
> Tamas,
>
> I'll let you take a first crack at updating the RFC to reflect this.  I'd
> suggest not updating your patches for the time being in case there is any
> other changes suggested.
>
> > It causes much of pain figuring out how GDAL/OGR handles the ownership
> > of the function parameters and return values. In this case the
> > difference of
> > the usage between exportToWkt and GetStyleString/getGeometryName scared
> > me highly. I would be the happiest if we eliminated the need of the
> > StringFinalizer class. The current solution preserves the returned wkt
> > within the
> > scope of the function. However it will not cause problems if the subsequent
> > Get*AsString() overrides the value because we are using the returned
> > value just
> > after the funcion call.
> > At this point I will leave you the task to consider how it will affect
> > the other
> > part of the OGR library. But I like this idea as being the warrior
> > against unnecessary
> > allocations and static variables.
>
> Excellent.  Lets go with this process.
>
> By the way, the documentation for methods like exportToWkt() should make
> it clear that the call gets ownership of the returned string.  For methods
> that return a "const char *" and that don't talk about ownership, it should
> be assumed that this is a pointer to an internal string that should not
> be altered by the caller and that may be altered in the future.
>
> >>   5) I'd like to see a section in the RFC describing the test suite items
> >>      you will add to the GDAL autotest.
> >>
> >
> > I'm afraid I'm not too familiar with GDAL autotest but I'm keen to dig
> > into it
> > more deeply if i have time. But now I can provide only the data (in
> > mapinfo format) and
> > the SQL query samples if needed. Currently I am testing the code using
> > step by step execution of ogrinfo in debug mode.
>
> OK, I've added a note in the RFC on this topic, and will implement the test
> script myself.  I've also taken the liberty of adding a section addressing
> "who does what".
>
> >>   6) I think the backward compatibility section of the RFC should mention
> >>      that the new special fields will potentially conflict with regard
> >>      fields with the given names, and to describe which will take
> >> precidence
> >>      when there is a conflict.
> >>
> >
> > No problem with this either. I would suggest a preceding control
> > character like '@' but it
> > will break my existing mapfiles (~500)  ;-)
>
> I don't mind there being a conflict as long as the circumstances are
> clear.
>
> I have also taken the liberty of adding a note in the RFC that the OGR SQL
> document (gdal/ogr/ogr_sql.dox) needs to be updated, and changing the overall
> name of the RFC to be briefer.  It should be updated on the web site by the
> time this email arrives anywhere.
>
>    http://www.gdal.org/rfc6_sqlgeom.html
>
> I've left the rest of the updates to you.
>
> Best regards,
> --
> ---------------------------------------+--------------------------------------
> I set the clouds in motion - turn up   | Frank Warmerdam, warmerdam at pobox.com
> light and sound - activate the windows | http://pobox.com/~warmerdam
> and watch the world go round - Rush    | President OSGeo, http://osgeo.org
>
>