fix(data): keep GeoPackage rows without geometry

crates/data/src/export/tests.rs

@@ -443,7 +443,8 @@ fn a_typeless_layer_exports_the_same_geopackage_streamed_or_whole() {
         assert_eq!(crate::io::write_path(&ds, &b).unwrap(), 0);
         assert_eq!((kind(&a), kind(&b)), (want.to_string(), want.to_string()));
         assert_eq!(read_back(&a).features, read_back(&b).features);
-        assert_eq!(read_back(&a).features.len(), 300);
+        // Every square and the feature without geometry, which a GeoPackage keeps.
+        assert_eq!(read_back(&a).features.len(), 301);
     }
 }
 
@@ -466,7 +467,8 @@ fn a_projected_streamed_geopackage_of_a_mixed_layer_keeps_every_kind() {
     assert_eq!((streamed.left_out, streamed.written), (0, 301));
     assert_eq!(crate::io::write_path_in(&mixed, &b, utm).unwrap(), 0);
     assert_eq!(read_back(&a).features, read_back(&b).features);
-    assert_eq!(read_back(&a).features.len(), 300);
+    // Every feature, the one without geometry included.
+    assert_eq!(read_back(&a).features.len(), 301);
 }
 
 /// A mixed layer added as a table to an existing GeoPackage, streamed or whole, gets a GEOMETRY

crates/data/src/source/gpkg.rs

@@ -3,7 +3,7 @@
 //! Extent queries use the table's R-tree spatial index (`rtree_<table>_<column>`) when it has one,
 //! and otherwise scan, testing the envelope in each geometry's header before decoding it.
 
-use super::gpkg_blob::{field_type, is_empty_blob, split_blob};
+use super::gpkg_blob::{field_type, is_empty_blob, is_whole_empty_blob, split_blob};
 use super::{Batch, FeatureSource, Reproject, Request, Sampler, SourceInfo, clamp_limit, in_request, stride};
 use crate::raster::Crs;
 use crate::{DataError, Feature, Field, FieldType, GeometryType, Result, Value, wkb};
@@ -372,14 +372,18 @@ impl GpkgSource {
     fn feature(&self, r: &rusqlite::Row<'_>, native: Option<&Extent>, req: &Request) -> rusqlite::Result<Row> {
         // Integer keys, negative ones too, map to distinct ids.
         let Some(id) = r.get::<_, Option<i64>>(0)?.map(|i| i as u64) else { return Ok(Row::Bad) };
+        let attrs =
+            || self.info.fields.iter().enumerate().map(|(i, f)| r.get_ref(i + 2).map(|v| value(v, f.ty)).unwrap_or(Value::Null)).collect();
         let blob = match r.get_ref(1)? {
-            ValueRef::Blob(b) => b,
-            ValueRef::Null => return Ok(Row::Skip),
+            ValueRef::Blob(b) if !is_whole_empty_blob(b) => b,
+            // A row without geometry (NULL, or an empty one) is kept in a whole read; it has no
+            // place to match an extent.
+            ValueRef::Null | ValueRef::Blob(_) if req.extent.is_none() => {
+                return Ok(Row::Feature(Feature { id, geometry: None, attrs: attrs() }));
+            }
+            ValueRef::Null | ValueRef::Blob(_) => return Ok(Row::Skip),
             _ => return Ok(Row::Bad),
         };
-        if is_empty_blob(blob) {
-            return Ok(Row::Skip);
-        }
         let Some((envelope, body)) = split_blob(blob) else { return Ok(Row::Bad) };
         if let (Some(q), Some(e)) = (native, envelope)
             && !e.intersects(q)
@@ -390,15 +394,13 @@ impl GpkgSource {
         if !in_request(&geometry, req) {
             return Ok(Row::Skip);
         }
-        let attrs =
-            self.info.fields.iter().enumerate().map(|(i, f)| r.get_ref(i + 2).map(|v| value(v, f.ty)).unwrap_or(Value::Null)).collect();
-        Ok(Row::Feature(Feature { id, geometry: Some(geometry), attrs }))
+        Ok(Row::Feature(Feature { id, geometry: Some(geometry), attrs: attrs() }))
     }
 }
 
 enum Row {
     Feature(Feature),
-    /// Outside the request, or without geometry.
+    /// Outside the request (a row without geometry is, when an extent is asked for).
     Skip,
     /// Unreadable.
     Bad,

crates/data/src/source/gpkg_blob.rs

@@ -9,6 +9,24 @@ pub(crate) fn is_empty_blob(b: &[u8]) -> bool {
     b.starts_with(b"GP") && b.get(3).is_some_and(|f| f & 0b1_0000 != 0)
 }
 
+/// A blob flagged empty whose header is whole (version 0, the one GeoPackage defines; 8 bytes and
+/// the envelope its flags give): a row the readers keep without geometry. Any other is unreadable,
+/// not empty.
+pub(crate) fn is_whole_empty_blob(b: &[u8]) -> bool {
+    is_empty_blob(b) && b.get(2) == Some(&0) && b.get(3).and_then(|&f| envelope_len(f)).is_some_and(|n| b.len() >= 8 + n)
+}
+
+/// The length of the envelope a header's flags give; `None` for a code the standard doesn't define.
+fn envelope_len(flags: u8) -> Option<usize> {
+    match (flags >> 1) & 0b111 {
+        0 => Some(0),
+        1 => Some(32),
+        2 | 3 => Some(48),
+        4 => Some(64),
+        _ => None,
+    }
+}
+
 /// A GeoPackage geometry blob: the header's envelope (if any) and the WKB after it.
 pub(crate) fn split_blob(b: &[u8]) -> Option<(Option<Extent>, &[u8])> {
     let (magic, rest) = b.split_at_checked(2)?;
@@ -18,13 +36,7 @@ pub(crate) fn split_blob(b: &[u8]) -> Option<(Option<Extent>, &[u8])> {
     }
     let &flags = rest.get(1)?;
     let little = flags & 1 == 1;
-    let env_len = match (flags >> 1) & 0b111 {
-        0 => 0,
-        1 => 32,
-        2 | 3 => 48,
-        4 => 64,
-        _ => return None,
-    };
+    let env_len = envelope_len(flags)?;
     if flags & 0b1_0000 != 0 {
         // The empty flag.
         return None;

crates/data/src/source/gpkg_mem.rs

@@ -329,6 +329,7 @@ impl FeatureSource for GpkgMemSource {
         &self.info
     }
 
+    /// Keeps rows without geometry in whole reads; they have no location to match an extent.
     fn read(&self, req: &Request) -> Result<Batch> {
         let limit = clamp_limit(req.limit);
         let native = req.extent.map(|e| self.reproject.extent_from_lonlat(&e));
@@ -337,16 +338,20 @@ impl FeatureSource for GpkgMemSource {
         self.db
             .rows(&self.table, |rowid, r| {
                 let blob = match r.get(self.geom_col) {
-                    Some(Val::Blob(b)) => b,
-                    Some(Val::Null) | None => return ControlFlow::Continue(()),
+                    Some(Val::Blob(b)) if !super::gpkg_blob::is_whole_empty_blob(b) => b,
+                    // A row without geometry (NULL, or an empty one) is kept in a whole read; it
+                    // has no place to match an extent.
+                    Some(Val::Null | Val::Blob(_)) | None => {
+                        if native.is_none() && sample.wants() {
+                            sample.keep(Feature { id: rowid as u64, geometry: None, attrs: self.attrs(r) });
+                        }
+                        return ControlFlow::Continue(());
+                    }
                     Some(_) => {
                         skipped += 1;
                         return ControlFlow::Continue(());
                     }
                 };
-                if super::gpkg_blob::is_empty_blob(blob) {
-                    return ControlFlow::Continue(());
-                }
                 let Some((envelope, body)) = super::gpkg_blob::split_blob(blob) else {
                     skipped += 1;
                     return ControlFlow::Continue(());
@@ -392,35 +397,36 @@ impl FeatureSource for GpkgMemSource {
         let mut failed = None;
         self.db
             .rows(&self.table, |rowid, r| {
-                let blob = match r.get(self.geom_col) {
-                    Some(Val::Blob(b)) => b,
-                    Some(Val::Null) | None => return ControlFlow::Continue(()),
+                let geometry = match r.get(self.geom_col) {
+                    Some(Val::Blob(b)) if !super::gpkg_blob::is_whole_empty_blob(b) => {
+                        let Some((envelope, body)) = super::gpkg_blob::split_blob(b) else {
+                            out.skipped += 1;
+                            return ControlFlow::Continue(());
+                        };
+                        if let (Some(q), Some(e)) = (&native, envelope)
+                            && !e.intersects(q)
+                        {
+                            return ControlFlow::Continue(());
+                        }
+                        let Some(geometry) = wkb::decode(body).and_then(|g| self.reproject.geometry(g)) else {
+                            out.skipped += 1;
+                            return ControlFlow::Continue(());
+                        };
+                        if !in_request(&geometry, &req) {
+                            return ControlFlow::Continue(());
+                        }
+                        Some(geometry)
+                    }
+                    // Without geometry (NULL, or an empty one): kept unless an extent is asked for.
+                    Some(Val::Null | Val::Blob(_)) | None if native.is_none() => None,
+                    Some(Val::Null | Val::Blob(_)) | None => return ControlFlow::Continue(()),
                     Some(_) => {
                         out.skipped += 1;
                         return ControlFlow::Continue(());
                     }
                 };
-                if super::gpkg_blob::is_empty_blob(blob) {
-                    return ControlFlow::Continue(());
-                }
-                let Some((envelope, body)) = super::gpkg_blob::split_blob(blob) else {
-                    out.skipped += 1;
-                    return ControlFlow::Continue(());
-                };
-                if let (Some(q), Some(e)) = (&native, envelope)
-                    && !e.intersects(q)
-                {
-                    return ControlFlow::Continue(());
-                }
-                let Some(geometry) = wkb::decode(body).and_then(|g| self.reproject.geometry(g)) else {
-                    out.skipped += 1;
-                    return ControlFlow::Continue(());
-                };
-                if !in_request(&geometry, &req) {
-                    return ControlFlow::Continue(());
-                }
                 out.matched += 1;
-                match chunks.push(Feature { id: rowid as u64, geometry: Some(geometry), attrs: self.attrs(r) }) {
+                match chunks.push(Feature { id: rowid as u64, geometry, attrs: self.attrs(r) }) {
                     Ok(()) => ControlFlow::Continue(()),
                     Err(e) => {
                         failed = Some(e);

crates/data/src/source/tests.rs

@@ -1151,8 +1151,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}");
@@ -2532,6 +2532,48 @@ fn shapefile_complete_chunks_keep_all_rows_order_attributes_and_budget() {
     assert_eq!(rows, expected.features);
 }
 
+/// 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; one flagged empty with an unreadable header is skipped and counted.
+#[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();
+        // Flagged empty but unreadable, each skipped and counted: cut off before its header ends,
+        // with an envelope code the standard doesn't define, an envelope declared but missing, and a
+        // header version the standard doesn't define.
+        for (fid, blob) in [(603, "47500011"), (604, "4750001b0f000000"), (605, "475000130f000000"), (606, "475001110f000000")] {
+            c.execute(&format!("INSERT INTO \"grid \"\"q\"\"\" VALUES ({fid}, X'{blob}', 'cut', 9, 1)"), []).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(606), "{format}");
+        let whole = src.read(&Request::all()).unwrap();
+        assert_eq!((whole.features.len(), whole.skipped), (602, 4), "{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, summary.skipped), (whole.features, 602, 4), "{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 feature of a complete traversal, in order, checking each chunk against the budget.
 fn visit_all(src: &dyn FeatureSource, extent: Option<Extent>, budget: usize) -> (Vec<Feature>, Batch, usize) {
     let (mut rows, mut calls) = (Vec::new(), 0);
@@ -2641,20 +2683,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));
 }