[Qgis-developer] [Python] QgsFeatureIterator for geometryless PostgreSQL-Layer

[Martin Dobias]

On Tue, Apr 23, 2013 at 11:38 AM, Jürgen E. <jef at norbit.de> wrote:
> Hi Martin,
>
> On Tue, 16. Apr 2013 at 09:32:08 +0200, Martin Dobias wrote:
>> > IMHO that's not ok.  If the provider doesn't support multiple iterators,
>> > the second on should be rejected - forcing the caller to close the other
>> > one first.
>
>> You're right - when thinking about it again, it makes more sense to reject
>> new iterator rather than close the previous one with force. It should be
>> relatively simple to swap the behavior.
>
> Which only moves the problem upward.   Maybe we should handle setFilterFid
> requests like the old featureAtId.

Could you be more specific what would that mean implementation-wise?


>> > As we lost the ability to do featureAtId while iterating a layer, we now
>> > have a couple of places, where stuff behaves strange and it's not really
>> > obvious that it's causes by a setFilterFid request killing a surrounding
>> > iteration.
>
>> In the other thread (about updateFeature()) I have noted that we actually
>> never really supported featureAtId while iterating a layer - at least not in
>> all providers... AFAIK postgis always used only one cursor. With OGR provider
>> that could work - but maybe just for some drivers (e.g. shapefile). If I have
>> time I will try to do few tests how qgis 1.8 behaved.
>
> I think the postgres provider had a separate (very short lived) cursor for
> featureAtId.

Right, I thought it was the same cursor, but in fact they were called
differently ("qgisf[N]" vs "qgisfid[N]"). But still with OGR there is
not guarantee that it will work - if the driver does not implement
random access to features. Not sure about other providers...


>> > Rejecting it wouldn't cure that, but at least make is show (ok, that's what
>> > the error message does also do that now).  And unless we get the parallel
>> > setFilterFid request back, we'll need to identify and rewrite the looping
>> > code.
>
>> When porting the code to new API, were there really loops that combined
>> nextFeature() with featureAtId() ?
>
> Apparently.  But my porting was more an semi-automatic effort - so didn't
> really look closely at all the code.  And I wasn't aware that multiple
> iterators were a problem - I thought that was kind of the point of the new api.
> ;)

The point of the new API was to get a common flexible interface.
Multiple iterators are currently a problem mainly because previously
the providers were not designed to work that way. There was just the
implicit assumption that it's possible to use nextFeature() and
featureAtId() at the same time - although likely not working in all
cases. I would like to address multiple iterators in the 2.1 release
together with threaded rendering.


> But we have a few tickets for that.  Eg. http://hub.qgis.org/issues/7472.  I
> tried to link others related tickets there.  There are probably more.  The
> submitters usually are not aware, that the problems are related.

Thanks for gathering the related tickets into one place. Right now it
seems that all those problems lead to the handling of changed
geometries within vector layer's feature iterator
(fetchNextChangedGeomFeature()). We could update that code to first
finish iterating over features from data provider and only after that
fetch remaining features (by ID) with changed geometries that have not
been considered before (in the main loop) - at that time the internal
provider's iterator will be closed already, so there will be no
collision. This change should also lower the amount of queries to the
provider: if the feature with changed geometry is still in the filter
rectangle, no superfluous "feature at id" queries will be issued.

Does that sound good to you?

Regards
Martin