[gdal-dev] OGR : OCI driver improvement

[Frank Warmerdam]

Nicolas,

The proposed changes seem reasonable, but I am concerned about complexity and
risk of breaking the driver in some rarely tested cases.  The OCI
driver is pretty
fragile.

On Mon, Jul 4, 2011 at 8:32 AM, Nicolas Simon
<nicolas.simon at spw.wallonie.be> wrote:
> Hi all,
>
> I'd like to propose some OCI driver improvement.
>
> 1) Enable insert from different client. This could be done by managing the generation of new FID on DB side instead of client side.
> Technically this could be done with an oracle sequence and a trigger to fill the FID.
> This proposition modify the way FID are handled ( I propose to do as in PostGIS driver). This means that the FID of a feature (i.e. usually the value of the OGC_FID column for the feature) inserted into a table with CreateFeature() will be retrieved from the database and can be obtained with GetFID() even if an non-null FID was provided.
> This could be easily implemented in UnboundCreateFeature with a 'returning' clause.
> But I don't known if it's feasible in BoundCreateFeature since truth FIDs will be know later when OGROCITableLayer::FlushPendingFeatures() is called.

I'm not clear on what you will do if CreateFeature() is called on a
table for which the
the sequence and trigger to fill the FID do not already exist.  Will
you create it?  I'm
concerned this will not be honoured by other applications interacting with the
table.

> 2) By providing true update operation instead of delete + insert operations (cf SetFeture).
> This should provide better performance and enable to develop trigger on update if needed for other purpose.
> With this modification, OCI driver will have the following mapping between OGR concepts and Oracle operations:
>    OGRFeature::CreateFeature() <==> INSERT operation
>    OGRFeature::SetFeature() <==> UPDATE operation
>    OGRFeature::DeleteFeature() <==> DELETE operation
> This modify slightly the actual behavior since it disable the possibility of inserting a new record through SetFeature (in case of  they were no record to delete, the subsequent insert did the job). With the proposed implementation it'll be no longer the case.  "Update ... set ...  where FID = provided FID" command doesn't insert new record.

The main downside I see with implementing SetFeature() as an UPDATE is
complexity -
another whole set of code for assigning records.  But that isn't a
compelling reason
not to do it.

> 3) Add transaction support in the normal SQL sense.
> Since transaction is handled within a session and we have a session per OGROCIDataSource, a transaction will include operation on any layer of that datasource.
> I propose the following operating mode.
>
> Outside a transaction, we'll be in 'auto commit' mode.
>  This commit on success DeleteFeature, SetFeature and CreateFeature (not in MULTI_LOAD mode).
>  For CreateFeature (in MULTI_LOAD mode) data will be commited
>  - each time the buffer is full (when OGROCITableLayer::FlushPendingFeatures() is called)
>  - when OGROCITableLayer::SyncToDisk() is call
>  - before a new transaction start.
>
> Transaction start (through a call OGRLayer::StartTransaction()) for all layer in the DataSource.
> OGRLayer::CommitTransaction() will call OGROCITableLayer::SyncToDisk() for each layer of  the DataSource and issues one commit command for the session, and then return in auto commit mode (cf outside transaction mechanism)
> OGRLayer::RollbackTransaction() will drop PendingFeature (may be  through a private function that reset nWriteCacheUsed to 0) for each layer of the datasource and issues one rollback command for the session, and then return to auto commit mode.

This *seems* reasonable, but please understand that the OGR transaction
model is fairly weak.

> I would like to know if someone else is interested in these improvements ?
> Is it the way you want things work ?
>
> An other information is that I'm ready to work for these improvements.

I think the changes are desirable (in trunk only!) but if you provide them
I am hopefully you will take an ongoing interest in maintenance of the
OCI driver.   Hopefully you might also be open to taking on commit
access so you can do stuff more easily (perhaps after first submitting
patches via trac for review).

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    | Geospatial Programmer for Rent