On 28 May 2018 at 10:43, Nyall Dawson <[email protected]> wrote: > On 4 September 2017 at 18:56, Sandro Mani <[email protected]> wrote: >> >> On 03.09.2017 23:50, Even Rouault wrote: >> >>> Ok, so if we only do this for non-select queries, we should be safe? >> >> >> >> Yes. I suspect the case where a full SELECT statement is used are rare, and >> people able to do that can tweak the request. It would be unreliable to try >> modifying it. >> >> >> >>> >> >>> > > Something like this [1] seems to fix the issue, don't know if it's the >> >>> > > >> >>> > > best way forward though? >> >>> > >> >>> > I've left a few comments in the commit >> >>> >> >>> Many thanks. I've pushed a revised version at [1]. >> >> >> >> I've added a couple suggestions for more robustness, but looks good to me. >> >> Thanks, filed PR: https://github.com/qgis/QGIS/pull/5122 > > I've been tracking down a performance regression in 3.0 and have > tracked it back to this PR. > > What's happening is that in > https://github.com/qgis/QGIS/blob/master/src/providers/ogr/qgsogrfeatureiterator.cpp#L204 > , everytime a new feature is requested with a particular ID the ENTIRE > layer is being read until that feature is found! This destroys QGIS > performance when more than a handful of feature IDs are requested - > e.g. try setting a subset string on a layer, then selecting a couple > of hundred points and zooming to selection. > > I've been trying to find a good solution to this without luck - if I > use OGR_L_SetAttributeFilter to specifically request the desired > feature with orig_ogr_fid equal to the requested feature id, then it's > a bit better. There's no longer the need to iterate through all the > features, but it's still very very slow compared to a direct > OGR_L_GetFeature call. > > Even do you have any good tricks we could utilise here to speed this > up? Any other pointers for approaches to test?
Actually - unless I'm missing something, I don't believe that the whole condition at https://github.com/qgis/QGIS/blob/master/src/providers/ogr/qgsogrfeatureiterator.cpp#L204 is even required, because it appears that OGR_L_GetFeature works correctly with the feature IDs taken from the original fid column. In other words, when ExecuteSQL is used, we can't safely rely on OGR_F_GetFID to return a usable feature id and have to use the ogc_fid attribute value instead... BUT when requesting a feature from the layer by ID, we CAN safely use OGR_L_GetFeature with IDs taken from ogc_fid and be confident that we get the desired feature returned. Is that interpretation correct? If so, this fix is implemented in: https://github.com/qgis/QGIS/pull/7091 Nyall _______________________________________________ QGIS-Developer mailing list [email protected] List info: https://lists.osgeo.org/mailman/listinfo/qgis-developer Unsubscribe: https://lists.osgeo.org/mailman/listinfo/qgis-developer
