Repository navigation
fix(data): keep GeoPackage rows without geometry #1673
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
8911ef7
2334e48
d7eb5ab
d0d1c3d
c587620
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1001,8 +1001,8 @@ fn writes_geopackage_geoparquet_and_shapefile() { | |
| let holds_all = matches!(ext, "gpkg" | "parquet"); | ||
| assert_eq!(left_out, if holds_all { 0 } else { 1 }, "{ext}"); | ||
| let (back, _) = crate::io::read_path(&p).unwrap(); | ||
| // GeoParquet retains nullable geometry; the other readers omit rows without a shape. | ||
| assert_eq!(back.features.len(), if ext == "parquet" { 3 } else { 2 }, "{ext}"); | ||
| // GeoParquet and GeoPackage keep a row without a shape; the other formats can't hold it. | ||
| assert_eq!(back.features.len(), if holds_all { 3 } else { 2 }, "{ext}"); | ||
| assert_eq!(back.geometry_type, GeometryType::Polygon, "{ext}"); | ||
| let e = back.extent(); | ||
| assert_eq!((e.min_x, e.min_y, e.max_x, e.max_y), (-30.0, -40.0, 21.0, 21.0), "{ext}"); | ||
|
|
@@ -2383,6 +2383,44 @@ fn shapefile_complete_chunks_keep_all_rows_order_attributes_and_budget() { | |
| } | ||
|
|
||
| /// Every feature of a complete traversal, in order, checking each chunk against the budget. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit (quality, can wait for the release quality pass): this line was the doc comment of
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 2334e48: the line is back on |
||
| /// GeoPackage rows without geometry (NULL, or an empty geometry) keep their row and attributes in | ||
| /// whole reads and visits, with and without the R-tree and in the browser's reader, and are left | ||
| /// out of a read of an extent. | ||
| #[test] | ||
| fn geopackage_keeps_rows_without_geometry() { | ||
| let d = dir().join("geometryless"); | ||
| std::fs::create_dir_all(&d).unwrap(); | ||
| let (indexed, scan) = (d.join("indexed.gpkg"), d.join("scan.gpkg")); | ||
| for (p, rtree) in [(&indexed, true), (&scan, false)] { | ||
| write_gpkg(p, rtree); | ||
| let c = rusqlite::Connection::open(p).unwrap(); | ||
| let empty = b"GP\x00\x11\x11\x0f\x00\x00".to_vec(); | ||
| c.execute("INSERT INTO \"grid \"\"q\"\"\" VALUES (601, NULL, 'no place', 7, 1)", []).unwrap(); | ||
| c.execute("INSERT INTO \"grid \"\"q\"\"\" VALUES (602, ?1, 'empty', 8, 0)", [empty]).unwrap(); | ||
| } | ||
| let sources: Vec<Box<dyn FeatureSource>> = vec![ | ||
| open_path(&indexed, None).unwrap(), | ||
| open_path(&scan, None).unwrap(), | ||
| Box::new(gpkg_mem::GpkgMemSource::open_shared(Shared(Arc::new(std::fs::read(&scan).unwrap())), None).unwrap()), | ||
| ]; | ||
| for src in &sources { | ||
| let format = src.info().format; | ||
| assert_eq!(src.info().count, Some(602), "{format}"); | ||
| let whole = src.read(&Request::all()).unwrap(); | ||
| assert_eq!((whole.features.len(), whole.skipped), (602, 0), "{format}"); | ||
| let bare: Vec<_> = whole.features.iter().filter(|f| f.geometry.is_none()).map(|f| (f.id, f.attrs[0].clone())).collect(); | ||
| assert_eq!(bare, vec![(601, Value::Text("no place".into())), (602, Value::Text("empty".into()))], "{format}"); | ||
| let (rows, summary, _) = visit_all(src.as_ref(), None, 1024); | ||
| assert_eq!((rows, summary.matched), (whole.features, 602), "{format}"); | ||
| let world = Extent::new(-180.0, -85.0, 180.0, 85.0); | ||
| let part = src.read(&Request { extent: Some(world), limit: 10_000 }).unwrap(); | ||
| assert_eq!(part.features.len(), 600, "{format}"); | ||
| assert_eq!(visit_all(src.as_ref(), Some(world), 1024).0.len(), 600, "{format}"); | ||
| } | ||
| } | ||
|
|
||
| /// Every chunk a visit of `extent` gives, joined, checking each fits `budget` (or is one row too | ||
| /// big for it): the rows, the visit's summary and how many chunks came. | ||
| fn visit_all(src: &dyn FeatureSource, extent: Option<Extent>, budget: usize) -> (Vec<Feature>, Batch, usize) { | ||
| let (mut rows, mut calls) = (Vec::new(), 0); | ||
| let summary = src | ||
|
|
@@ -2491,20 +2529,20 @@ fn geopackage_flatgeobuf_and_geoparquet_visit_what_a_whole_read_gives() { | |
| let error = src.visit_chunks(None, 4096, &mut |_| Ok(())).unwrap_err(); | ||
| assert!(matches!(&error, crate::DataError::Io(e) if e.kind() == std::io::ErrorKind::NotFound), "{error}"); | ||
| } | ||
| // An unreadable geometry is skipped and counted, and a row without one left out, as a read does. | ||
| // An unreadable geometry is skipped and counted, and a row without one kept, as a read does. | ||
| drop(sources); | ||
| let c = rusqlite::Connection::open(&gpkg_scan).unwrap(); | ||
| c.execute_batch("INSERT INTO \"grid \"\"q\"\"\" (geom, name) VALUES (X'4750000300000000DEAD', 'bad'), (NULL, 'none');").unwrap(); | ||
| drop(c); | ||
| let src = open_path(&gpkg_scan, None).unwrap(); | ||
| let whole = src.read(&Request::all()).unwrap(); | ||
| assert_eq!((whole.features.len(), whole.skipped), (600, 1)); | ||
| assert_eq!((whole.features.len(), whole.skipped), (601, 1)); | ||
| let (rows, summary, _) = visit_all(src.as_ref(), None, 4096); | ||
| assert_eq!((rows, summary.skipped), (whole.features, 1)); | ||
| // The browser's reader of the same file skips and leaves out the same rows. | ||
| // The browser's reader of the same file skips and keeps the same rows. | ||
| let mem = gpkg_mem::GpkgMemSource::open_shared(Shared(Arc::new(std::fs::read(&gpkg_scan).unwrap())), None).unwrap(); | ||
| let whole = mem.read(&Request::all()).unwrap(); | ||
| assert_eq!((whole.features.len(), whole.skipped), (600, 1)); | ||
| assert_eq!((whole.features.len(), whole.skipped), (601, 1)); | ||
| let (rows, summary, _) = visit_all(&mem, None, 4096); | ||
| assert_eq!((rows, summary.skipped), (whole.features, 1)); | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.