diff --git a/CHANGES.md b/CHANGES.md index 6b7aa0166..6f71dd3c9 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -28,6 +28,7 @@ - Add `Defn::field_index` and `Feature::field_index` ([#581](https://github.com/georust/gdal/pull/581)) - Add `Defn::geometry_field_index` and `Feature::geometry_field_index` ([#594](https://github.com/georust/gdal/pull/594)) - Add `Dataset::has_capability` for dataset capability check ([#581](https://github.com/georust/gdal/pull/585)) + - Add `Dataset::maybe_run_in_batch` which runs a user function inside a transaction if the dataset supports it, as an optimization for I/O; if the function fails, the transaction may or may not be rolled back ([#584](https://github.com/georust/gdal/pull/584)) ### Fixed diff --git a/src/test_utils.rs b/src/test_utils.rs index 65a448636..8c9171940 100644 --- a/src/test_utils.rs +++ b/src/test_utils.rs @@ -110,19 +110,12 @@ impl Drop for SuppressGDALErrorLog { /// Copies the given file to a temporary file and opens it for writing. When the returned /// `TempPath` is dropped, the file is deleted. pub fn open_gpkg_for_update(path: &Path) -> (TempPath, Dataset) { - use std::fs; - use std::io::Write; - - let input_data = fs::read(path).unwrap(); - let (mut file, temp_path) = tempfile::Builder::new() + let temp_path = tempfile::Builder::new() .suffix(".gpkg") .tempfile() .unwrap() - .into_parts(); - file.write_all(&input_data).unwrap(); - // Close the temporary file so that Dataset can open it safely even if the filesystem uses - // exclusive locking (Windows?). - drop(file); + .into_temp_path(); + std::fs::copy(path, &temp_path).unwrap(); let ds = Dataset::open_ex( &temp_path, @@ -136,6 +129,34 @@ pub fn open_gpkg_for_update(path: &Path) -> (TempPath, Dataset) { (temp_path, ds) } +/// Copies the given file to a temporary file and opens it for writing. When the returned +/// `TempPath` is dropped, the file is deleted. +pub fn open_dataset_for_update(path: &Path) -> (TempPath, Dataset) { + let temp_path = tempfile::Builder::new() + // using the whole filename as suffix should be fine (can't + // use .extension() for .shp.zip and such) + .suffix( + path.file_name() + .unwrap_or_default() + .to_string_lossy() + .as_ref(), + ) + .tempfile() + .unwrap() + .into_temp_path(); + std::fs::copy(path, &temp_path).unwrap(); + + let ds = Dataset::open_ex( + &temp_path, + DatasetOptions { + open_flags: GDALAccess::GA_Update.into(), + ..DatasetOptions::default() + }, + ) + .unwrap(); + (temp_path, ds) +} + /// Assert numerical difference between two expressions is less than /// 64-bit machine epsilon or a specified epsilon. /// diff --git a/src/vector/transaction.rs b/src/vector/transaction.rs index 2719e2103..6adb9a577 100644 --- a/src/vector/transaction.rs +++ b/src/vector/transaction.rs @@ -177,11 +177,52 @@ impl Dataset { } Ok(Transaction::new(self)) } + + /// Optionally start a transaction before running `func` for performance + /// + /// This uses transaction if the dataset supports it, otherwise it + /// runs the `func` function as it is on the dataset. If the + /// closure results in error, and it is a transaction, it is + /// rolled back. If the rollback fails, it not reported. + pub fn maybe_run_in_batch( + &mut self, + mut func: impl FnMut(&mut Dataset) -> Result<()>, + ) -> Result<()> { + let force = 0; // since this is for speed + let rv = unsafe { gdal_sys::GDALDatasetStartTransaction(self.c_dataset(), force) }; + if rv == OGRErr::OGRERR_UNSUPPORTED_OPERATION { + func(self) + } else if rv == OGRErr::OGRERR_NONE { + let res = func(self); + if res.is_ok() { + let rv = unsafe { gdal_sys::GDALDatasetCommitTransaction(self.c_dataset()) }; + if rv != OGRErr::OGRERR_NONE { + Err(GdalError::OgrError { + err: rv, + method_name: "GDALDatasetCommitTransaction", + }) + } else { + res + } + } else { + let _ = unsafe { gdal_sys::GDALDatasetRollbackTransaction(self.c_dataset()) }; + // ignore rollback error because it's not guaranteed to be + // supported, and the `func` failure is more important. + res + } + } else { + // transaction supported but failed + Err(GdalError::OgrError { + err: rv, + method_name: "GDALDatasetStartTransaction", + }) + } + } } #[cfg(test)] mod tests { - use crate::test_utils::{fixture, open_gpkg_for_update}; + use crate::test_utils::{fixture, open_dataset_for_update, open_gpkg_for_update}; use crate::vector::{Geometry, LayerAccess}; use crate::Dataset; @@ -241,4 +282,30 @@ mod tests { let mut ds = Dataset::open(fixture("roads.geojson")).unwrap(); assert!(ds.start_transaction().is_err()); } + + #[test] + fn test_maybe_run_in_batch() { + let (_temp_path, mut ds) = open_gpkg_for_update(&fixture("poly.gpkg")); + let orig_feature_count = ds.layer(0).unwrap().feature_count(); + + let res = ds.maybe_run_in_batch(|d| { + let mut layer = d.layer(0).unwrap(); + layer.create_feature(polygon()) + }); + assert!(res.is_ok()); + assert_eq!(ds.layer(0).unwrap().feature_count(), orig_feature_count + 1); + } + + #[test] + fn test_maybe_transaction_unsupported() { + let (_temp_path, mut ds) = open_dataset_for_update(&fixture("roads.geojson")); + let orig_feature_count = ds.layer(0).unwrap().feature_count(); + + let res = ds.maybe_run_in_batch(|d| { + let mut layer = d.layer(0).unwrap(); + layer.create_feature(polygon()) + }); + assert!(res.is_ok()); + assert_eq!(ds.layer(0).unwrap().feature_count(), orig_feature_count + 1); + } }