Skip to content

Commit b27014d

Browse files
committed
Add Dataset::maybe_batch for faster dataset write when possible
1 parent 9692dae commit b27014d

File tree

3 files changed

+100
-1
lines changed

3 files changed

+100
-1
lines changed

CHANGES.md

+1
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
- Add `Defn::geometry_type` ([#562](https://github.com/georust/gdal/pull/562))
2525
- Add `Defn::field_index` and `Feature::field_index` ([#581](https://github.com/georust/gdal/pull/581))
2626
- Add `Dataset::has_capability` for dataset capability check ([#581](https://github.com/georust/gdal/pull/585))
27+
- Add `Dataset::maybe_batch` which always succeeds to use transaction when possible for speed reasons -- rollbacks if operation fails under transaction ([#584](https://github.com/georust/gdal/pull/584))
2728

2829
### Fixed
2930

src/test_utils.rs

+35
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,41 @@ pub fn open_gpkg_for_update(path: &Path) -> (TempPath, Dataset) {
136136
(temp_path, ds)
137137
}
138138

139+
/// Copies the given file to a temporary file and opens it for writing. When the returned
140+
/// `TempPath` is dropped, the file is deleted.
141+
pub fn open_dataset_for_update(path: &Path) -> (TempPath, Dataset) {
142+
use std::fs;
143+
use std::io::Write;
144+
145+
let input_data = fs::read(path).unwrap();
146+
let (mut file, temp_path) = tempfile::Builder::new()
147+
// using the whole filename as suffix should be fine (can't
148+
// use .extension() for .shp.zip and such)
149+
.suffix(
150+
path.file_name()
151+
.unwrap_or_default()
152+
.to_string_lossy()
153+
.as_ref(),
154+
)
155+
.tempfile()
156+
.unwrap()
157+
.into_parts();
158+
file.write_all(&input_data).unwrap();
159+
// Close the temporary file so that Dataset can open it safely even if the filesystem uses
160+
// exclusive locking (Windows?).
161+
drop(file);
162+
163+
let ds = Dataset::open_ex(
164+
&temp_path,
165+
DatasetOptions {
166+
open_flags: GDALAccess::GA_Update.into(),
167+
..DatasetOptions::default()
168+
},
169+
)
170+
.unwrap();
171+
(temp_path, ds)
172+
}
173+
139174
/// Assert numerical difference between two expressions is less than
140175
/// 64-bit machine epsilon or a specified epsilon.
141176
///

src/vector/transaction.rs

+64-1
Original file line numberDiff line numberDiff line change
@@ -177,11 +177,48 @@ impl Dataset {
177177
}
178178
Ok(Transaction::new(self))
179179
}
180+
181+
/// Optionally start a transaction before running `func` for performance
182+
///
183+
/// This uses transaction if the dataset supports it, otherwise it
184+
/// runs the `func` function as it is on the dataset. If the
185+
/// closure results in error, and it is a transaction, it is
186+
/// rolled back. If there is error in rollback it is ignored.
187+
pub fn maybe_batch(&mut self, func: impl Fn(&Dataset) -> Result<()>) -> Result<()> {
188+
let force = 0; // since this is for speed
189+
let rv = unsafe { gdal_sys::GDALDatasetStartTransaction(self.c_dataset(), force) };
190+
if rv == OGRErr::OGRERR_UNSUPPORTED_OPERATION {
191+
func(self)
192+
} else if rv == OGRErr::OGRERR_NONE {
193+
let res = func(self);
194+
if res.is_ok() {
195+
let rv = unsafe { gdal_sys::GDALDatasetCommitTransaction(self.c_dataset()) };
196+
if rv != OGRErr::OGRERR_NONE {
197+
Err(GdalError::OgrError {
198+
err: rv,
199+
method_name: "GDALDatasetCommitTransaction",
200+
})
201+
} else {
202+
res
203+
}
204+
} else {
205+
let _ = unsafe { gdal_sys::GDALDatasetRollbackTransaction(self.c_dataset()) };
206+
// ignoring rollback error
207+
res
208+
}
209+
} else {
210+
// transaction supported but failed
211+
Err(GdalError::OgrError {
212+
err: rv,
213+
method_name: "GDALDatasetStartTransaction",
214+
})
215+
}
216+
}
180217
}
181218

182219
#[cfg(test)]
183220
mod tests {
184-
use crate::test_utils::{fixture, open_gpkg_for_update};
221+
use crate::test_utils::{fixture, open_dataset_for_update, open_gpkg_for_update};
185222
use crate::vector::{Geometry, LayerAccess};
186223
use crate::Dataset;
187224

@@ -241,4 +278,30 @@ mod tests {
241278
let mut ds = Dataset::open(fixture("roads.geojson")).unwrap();
242279
assert!(ds.start_transaction().is_err());
243280
}
281+
282+
#[test]
283+
fn test_maybe_batch() {
284+
let (_temp_path, mut ds) = open_gpkg_for_update(&fixture("poly.gpkg"));
285+
let orig_feature_count = ds.layer(0).unwrap().feature_count();
286+
287+
let res = ds.maybe_batch(|d| {
288+
let mut layer = d.layer(0).unwrap();
289+
layer.create_feature(polygon())
290+
});
291+
assert!(res.is_ok());
292+
assert_eq!(ds.layer(0).unwrap().feature_count(), orig_feature_count + 1);
293+
}
294+
295+
#[test]
296+
fn test_maybe_transaction_unsupported() {
297+
let (_temp_path, mut ds) = open_dataset_for_update(&fixture("roads.geojson"));
298+
let orig_feature_count = ds.layer(0).unwrap().feature_count();
299+
300+
let res = ds.maybe_batch(|d| {
301+
let mut layer = d.layer(0).unwrap();
302+
layer.create_feature(polygon())
303+
});
304+
assert!(res.is_ok());
305+
assert_eq!(ds.layer(0).unwrap().feature_count(), orig_feature_count + 1);
306+
}
244307
}

0 commit comments

Comments
 (0)