[QGIS-Developer] QGIS/OGR: FeatureIds reassigned on write to data provider?

[Sandro Mani]

On 20.09.2017 17:39, Even Rouault wrote:
>
> On mercredi 20 septembre 2017 17:32:27 CEST Sandro Mani wrote:
>
> > On 19.09.2017 19:15, Even Rouault wrote:
>
> > > On mardi 19 septembre 2017 18:30:50 CEST Sandro Mani wrote:
>
> > > > On 11.07.2017 14:31, Even Rouault wrote:
>
> > > > > We could potentially imagine to defer the repacking at
>
> > > > >
>
> > > > > leaveUpdateMode() time, actually in the close() method which 
> is called
>
> > > > >
>
> > > > > when the reference counter of the update mode drops to zero (*),
>
> > > > >
>
> > > > > instead of doing it at the end of each operation.
>
> > > >
>
> > > > To come back to this, I was looking at moving replack() to 
> close(), but
>
> > > >
>
> > > > noticed that syncToDisc() has the logic "for shapefiles, remove 
> spatial
>
> > > >
>
> > > > index files and create a new index" which calls close. And 
> syncToDisk is
>
> > > >
>
> > > > still called after every change. So should syncToDisk be also 
> moved to
>
> > > >
>
> > > > close(), or at least blocked if an updateMode is active?
>
> > >
>
> > > Yes that would probably make sense to block syncToDisk() while in
>
> > > updateMode (so when mUpdateModeStackDepth > 0), but probably only if
>
> > > enterUpdateMode() is called by QgsVectorLayer::startEditing(), and not
>
> > > when called implicitly by doInitialActionsForEdition(), which might be
>
> > > the case if someone directly works at the provider level
>
> > >
>
> > > bool QgsOgrProvider::doInitialActionsForEdition()
>
> > >
>
> > > {
>
> > >
>
> > > if ( !mValid )
>
> > >
>
> > > return false;
>
> > >
>
> > > if ( !mWriteAccess && mWriteAccessPossible && mDynamicWriteAccess )
>
> > >
>
> > > {
>
> > >
>
> > > QgsDebugMsg( "Enter update mode implictly" );
>
> > >
>
> > > if ( !enterUpdateMode() )
>
> > >
>
> > > return false;
>
> > >
>
> > > }
>
> > >
>
> > > return true;
>
> > >
>
> > > }
>
> > >
>
> > > So probably a mImplicitUpdateMode flag should be set above near the
>
> > > QgsDebugMsg(), and the check to decide whether we can defer
>
> > > syncToDisk() would be ( !mImplicitUpdateMode && 
> mUpdateModeStackDepth > 0)
>
> >
>
> > Hm but will that flag never be cleared again once set? Since AFAICT a
>
> > there is no leaveUpdateMode matching the enterUpdateMode called by
>
> > doInitialActionsForEdition.
>
> >
>
> > As I understand, the approach to do changes with stable ids would be to
>
> > first enterUpdateMode, then perform the changes, then leaveUpdateMode,
>
> > at which point the changes are synced and the dataset repacked.
>
> Yes
>
> > With the
>
> > mImplicitUpdateMode flag, if anyone worked directly at provider level
>
> > before you come along, deferring syncToDisk would never happen since the
>
> > mImplicitUpdateMode was set and remains set.
>
> Yes my idea was to keep the current behaviour where repack() is done 
> frequently, as you cannot know when the dataset should be in a 
> consistant state.
>
> But we could also decide to defer syncToDisk() at provider deletion 
> (would make the code simpler)
>
> I have no idea of the existing use cases behind and if that would 
> break some.
>
Suppose something like [1] should both allow reliably deferring 
syncToDisk when an updateMode was explicitly entered while keeping the 
current behaviour of immediate syncToDisk for implicit update mode.

Sandro

[1] 
https://github.com/manisandro/QGIS/commit/13f451bdcbb653e9d69e55c79a5ba520d3f1712e
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osgeo.org/pipermail/qgis-developer/attachments/20170920/0279c8d4/attachment.html>