fix(data): keep GeoPackage rows without geometry

crates/data/src/source/gpkg.rs

@@ -307,14 +307,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_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)
@@ -325,15 +329,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

@@ -4,9 +4,21 @@
 use crate::FieldType;
 use rustgis_geom::Extent;
 
-/// A GeoPackage geometry blob flagged empty: valid, with nothing to draw.
+/// A GeoPackage geometry blob flagged empty: valid, with nothing to draw. Its header must be
+/// whole (8 bytes and the envelope its flags give); a cut-off one is unreadable, not empty.
 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)
+    b.starts_with(b"GP") && b.get(3).is_some_and(|&f| f & 0b1_0000 != 0 && 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.
@@ -18,13 +30,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

@@ -270,6 +270,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));
@@ -278,16 +279,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_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(());
@@ -333,35 +338,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_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

@@ -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}");
@@ -2382,6 +2382,44 @@ 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 but cut off 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 cut off before its header ends: unreadable, skipped and counted.
+        c.execute("INSERT INTO \"grid \"\"q\"\"\" VALUES (603, X'47500011', '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(603), "{format}");
+        let whole = src.read(&Request::all()).unwrap();
+        assert_eq!((whole.features.len(), whole.skipped), (602, 1), "{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, 1), "{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);
@@ -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));
 }