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

[Frank Warmerdam]

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