[Feature Req] Optionally use Transaction if supported

[Atreyagaurav]

Would it be possible to use transaction if supported, else use normal dataset.

Considering the output dataset being able to use any driver user wants, I want to speed up the writing process when I can. If the output is to gpkg file I'd like to use transaction, if it's to shp then the code I have will fail. So I'm hoping for something that can work on both cases.

The Idea I have is, considering the code for Transaction::commit uses the internal Dataset, we can probably add the commit method on the Dataset and do something like this:

let trans = d.start_transaction().is_ok();
// do stuffs with d: Dataset
if trans {
    d.commit()?;
}

Currently, You have to do something like this:

let mut trans = false;
if let Some(txn) = d.start_transaction() {
    // do stuffs with txn 
    trans = true;
}

if !trans {
    // do stuffs with d
}

You can't use if let ... {} else {} pattern, as you can't borrow d mutably on start_transaction() and on the else block at the same time. And due to rust's type system, you can't replace d: Dataset with txn: Transaction to do the same thing (although you can with deref).

But basically to write a code that works with drivers that support transaction and drivers that don't, I think the first pattern I suggested feels like the best way to do it, as the code is internally calling commit on the Dataset anyway.

gdal/src/vector/transaction.rs

Lines 48 to 58 in 5d886ab

	pub fn commit(mut self) -> Result<()> {
	let rv = unsafe { gdal_sys::GDALDatasetCommitTransaction(self.dataset.c_dataset()) };
	self.rollback_on_drop = false;
	if rv != OGRErr::OGRERR_NONE {
	return Err(GdalError::OgrError {
	err: rv,
	method_name: "GDALDatasetCommitTransaction",
	});
	}
	Ok(())
	}

[lnicola]

Yeah, this is quite annoying. You can probably make things easier today by doing:

let stuffs = |d: &Dataset| -> Result<()> { ... };
if /* transactions are supported */ {
    let txn = d.start_transaction()?;
    stuffs(&txn)?;
    txn.commit()?;
} else {
    stuffs(&d)?;
}

I don't know if adding Dataset::commit would work very well, because then you could commit without an active transaction (but I admit I didn't spend a lot of time thinking about this API).

Another possibility would be to check for OGRERR_UNSUPPORTED_OPERATION, and return a "fake" transaction where commit succeeds and rollback fails, and expose if this happened through a getter on Transaction. But maybe it would be too surprising for users who expect atomicity.

We could also add something like:

enum TransactionMode {
    /// Require the driver to support "efficient" transactions.
    Normal,
    /// Allow "emulated" (probably slower) transactions, if the driver supports them.
    Forced,
    /// Return a no-op `Transaction` if the driver doesn't support transactions,
    /// which always succeeds on `commit` and fails on `rollback`.
    /// These don't offer any ACID guarantees!
    Yolo,
}

[Atreyagaurav]

Yeah, the function thing is nice because of deref. That's why I was thinking, "We're doing the operations on the inner dataset anyway, so it might be ok to add methods there".

I was thinking we can error out on the commit without active transaction, but you're right, we won't catch it in compile time. So maybe the idea you have given about TransactionMode is will work. Having the transaction work if supported on the user facing end is nice.

But maybe it would be too surprising for users who expect atomicity.

For this, another way might be to add MaybeTransaction class, which will be the same as Transaction class, and will have the same methods, with one extra is_transaction flag, which will be used to decided whether to actually commit internally or not in the commit call. And of course we can not guarantee rollback success, so not having that method at all for this might be better.

We can have Dataset::maybe_start_transaction() which returns a MaybeTransaction, which will allow the type to be uniform whether the dataset can have transaction or not.

[Atreyagaurav]

Forgot to mention, but I'll do the implementation for this once we get to a reasonable idea we think is going to work.