Skip to content

Commit 2e7dea7

Browse files
authored
fix(geoarrow-array): Fix validation of sliced geometry arrays (#1391)
### Change list - Validation through `new`, e.g. `LineStringArray::new` was incorrect for any input array with offsets that had been sliced. - This PR fixes the validation and adds tests through the `new` constructor for sliced arrays. Closes #1390, closes #1387
1 parent eeb8819 commit 2e7dea7

File tree

9 files changed

+169
-25
lines changed

9 files changed

+169
-25
lines changed

Cargo.lock

Lines changed: 12 additions & 12 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ exclude = ["js"]
1717
resolver = "2"
1818

1919
[workspace.package]
20-
version = "0.6.1"
20+
version = "0.6.2"
2121
authors = ["Kyle Barron <[email protected]>"]
2222
edition = "2024"
2323
license = "MIT OR Apache-2.0"

python/CHANGELOG.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44

55
## [0.6.0] - 2025-10-16
66

7-
## What's Changed
7+
### What's Changed
88

99
- docs: include constructor docs in the table of contents by @keewis in https://github.com/geoarrow/geoarrow-rs/pull/1347
1010
- fix(geoarrow-flatgeobuf): Fix FlatGeobuf reader with missing (null) properties by @kylebarron in https://github.com/geoarrow/geoarrow-rs/pull/1356
@@ -16,7 +16,7 @@
1616
- fix(python): Fix `__geo_interface__` export for GeoScalar by @kylebarron in https://github.com/geoarrow/geoarrow-rs/pull/1384
1717
- feat(python): Constuctor for GeoArray by @kylebarron in https://github.com/geoarrow/geoarrow-rs/pull/1385
1818

19-
## New Contributors
19+
### New Contributors
2020

2121
- @keewis made their first contribution in https://github.com/geoarrow/geoarrow-rs/pull/1347
2222

rust/geoarrow-array/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@
44

55
## Unreleased
66

7+
## 0.6.2 - 2025-10-27
8+
9+
fix(geoarrow-array): Fix validation of sliced geometry arrays #1391
10+
711
## 0.6.0 - 2025-10-15
812

913
- fix(geoarrow-array): Fix persisting CRS when creating `GeometryArray` from `Field` by @kylebarron in https://github.com/geoarrow/geoarrow-rs/pull/1326

rust/geoarrow-array/src/array/linestring.rs

Lines changed: 71 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,10 @@ pub(super) fn check(
4444
));
4545
}
4646

47-
if *geom_offsets.last() as usize != coords.len() {
47+
// Offset can be smaller than coords length if sliced
48+
if *geom_offsets.last() as usize > coords.len() {
4849
return Err(GeoArrowError::InvalidGeoArrow(
49-
"largest geometry offset must match coords length".to_string(),
50+
"largest geometry offset must not be longer than coords length".to_string(),
5051
));
5152
}
5253

@@ -321,6 +322,8 @@ impl GeometryTypeId for LineStringArray {
321322

322323
#[cfg(test)]
323324
mod test {
325+
use arrow_array::RecordBatch;
326+
use arrow_schema::Schema;
324327
use geo_traits::to_geo::ToGeoLineString;
325328
use geoarrow_schema::{CoordType, Dimension};
326329

@@ -401,4 +404,70 @@ mod test {
401404

402405
assert_ne!(arr1, arr2.slice(0, 2));
403406
}
407+
408+
#[test]
409+
fn test_validation_with_sliced_array() {
410+
let arr = linestring::array(CoordType::Interleaved, Dimension::XY);
411+
let sliced = arr.slice(0, 1);
412+
413+
let back = LineStringArray::try_from((
414+
sliced.to_array_ref().as_ref(),
415+
arr.extension_type().clone(),
416+
))
417+
.unwrap();
418+
assert_eq!(back.len(), 1);
419+
}
420+
421+
#[test]
422+
fn slice_then_go_through_arrow() {
423+
let arr = linestring::array(CoordType::Separated, Dimension::XY);
424+
let sliced_array = arr.slice(0, 1);
425+
426+
let ls_array: LineStringArray = (
427+
sliced_array.to_array_ref().as_ref(),
428+
arr.extension_type().clone(),
429+
)
430+
.try_into()
431+
.unwrap();
432+
assert_eq!(ls_array.len(), 1);
433+
}
434+
435+
#[test]
436+
fn slice_back_from_arrow_rs_record_batch() {
437+
let arr = linestring::array(CoordType::Separated, Dimension::XY);
438+
let field = arr.extension_type().to_field("geometry", true);
439+
let schema = Schema::new(vec![field]);
440+
441+
let batch = RecordBatch::try_new(Arc::new(schema), vec![arr.to_array_ref()]).unwrap();
442+
let sliced_batch = batch.slice(0, 1);
443+
444+
let array = sliced_batch.column(0);
445+
let field = sliced_batch.schema_ref().field(0);
446+
let ls_array: LineStringArray = (array.as_ref(), field).try_into().unwrap();
447+
assert_eq!(ls_array.len(), 1);
448+
}
449+
450+
#[test]
451+
fn slice_back_from_arrow_rs_array() {
452+
let arr = linestring::array(CoordType::Separated, Dimension::XY);
453+
let field = arr.extension_type().to_field("geometry", true);
454+
455+
let array = arr.to_array_ref();
456+
let sliced_array = array.slice(0, 1);
457+
458+
let ls_array: LineStringArray = (sliced_array.as_ref(), &field).try_into().unwrap();
459+
assert_eq!(ls_array.len(), 1);
460+
}
461+
462+
#[test]
463+
fn slice_back_from_arrow_rs_array_with_nulls() {
464+
let arr = linestring::ls_array(CoordType::Separated);
465+
let field = arr.extension_type().to_field("geometry", true);
466+
467+
let array = arr.to_array_ref();
468+
let sliced_array = array.slice(0, 1);
469+
470+
let ls_array: LineStringArray = (sliced_array.as_ref(), &field).try_into().unwrap();
471+
assert_eq!(ls_array.len(), 1);
472+
}
404473
}

rust/geoarrow-array/src/array/multilinestring.rs

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,10 @@ pub(super) fn check(
5454
));
5555
}
5656

57-
if *geom_offsets.last() as usize != ring_offsets.len_proxy() {
57+
// Offset can be smaller than length if sliced
58+
if *geom_offsets.last() as usize > ring_offsets.len_proxy() {
5859
return Err(GeoArrowError::InvalidGeoArrow(
59-
"largest geometry offset must match ring offsets length".to_string(),
60+
"largest geometry offset must not be longer than ring offsets length".to_string(),
6061
));
6162
}
6263

@@ -502,4 +503,27 @@ mod test {
502503
assert_ne!(arr1, arr2.slice(0, 2));
503504
}
504505
}
506+
507+
#[test]
508+
fn test_validation_with_sliced_array() {
509+
let arr = multilinestring::array(CoordType::Interleaved, Dimension::XY);
510+
let sliced = arr.slice(0, 1);
511+
512+
let back = MultiLineStringArray::try_from((
513+
sliced.to_array_ref().as_ref(),
514+
arr.extension_type().clone(),
515+
))
516+
.unwrap();
517+
assert_eq!(back.len(), 1);
518+
}
519+
520+
#[test]
521+
fn test_validation_with_array_sliced_by_arrow_rs() {
522+
let arr = multilinestring::array(CoordType::Interleaved, Dimension::XY);
523+
let sliced = arr.to_array_ref().slice(0, 1);
524+
525+
let back = MultiLineStringArray::try_from((sliced.as_ref(), arr.extension_type().clone()))
526+
.unwrap();
527+
assert_eq!(back.len(), 1);
528+
}
505529
}

rust/geoarrow-array/src/array/multipoint.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,10 @@ pub(super) fn check(
4444
));
4545
}
4646

47-
if *geom_offsets.last() as usize != coords.len() {
47+
// Offset can be smaller than coords length if sliced
48+
if *geom_offsets.last() as usize > coords.len() {
4849
return Err(GeoArrowError::InvalidGeoArrow(
49-
"largest geometry offset must match coords length".to_string(),
50+
"largest geometry offset must not be longer than coords length".to_string(),
5051
));
5152
}
5253

rust/geoarrow-array/src/array/multipolygon.rs

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ pub(super) fn check(
5151
"nulls mask length must match the number of values".to_string(),
5252
));
5353
}
54+
55+
// Offset can be smaller than coords length if sliced
5456
if *ring_offsets.last() as usize != coords.len() {
5557
return Err(GeoArrowError::InvalidGeoArrow(
5658
"largest ring offset must match coords length".to_string(),
@@ -63,9 +65,9 @@ pub(super) fn check(
6365
));
6466
}
6567

66-
if *geom_offsets.last() as usize != polygon_offsets.len_proxy() {
68+
if *geom_offsets.last() as usize > polygon_offsets.len_proxy() {
6769
return Err(GeoArrowError::InvalidGeoArrow(
68-
"largest geometry offset must match polygon offsets length".to_string(),
70+
"largest geometry offset must not be longer than polygon offsets length".to_string(),
6971
));
7072
}
7173

@@ -555,4 +557,27 @@ mod test {
555557
assert_ne!(arr1, arr2.slice(0, 2));
556558
}
557559
}
560+
561+
#[test]
562+
fn test_validation_with_sliced_array() {
563+
let arr = multipolygon::array(CoordType::Interleaved, Dimension::XY);
564+
let sliced = arr.slice(0, 1);
565+
566+
let back = MultiPolygonArray::try_from((
567+
sliced.to_array_ref().as_ref(),
568+
arr.extension_type().clone(),
569+
))
570+
.unwrap();
571+
assert_eq!(back.len(), 1);
572+
}
573+
574+
#[test]
575+
fn test_validation_with_array_sliced_by_arrow_rs() {
576+
let arr = multipolygon::array(CoordType::Interleaved, Dimension::XY);
577+
let sliced = arr.to_array_ref().slice(0, 1);
578+
579+
let back =
580+
MultiPolygonArray::try_from((sliced.as_ref(), arr.extension_type().clone())).unwrap();
581+
assert_eq!(back.len(), 1);
582+
}
558583
}

rust/geoarrow-array/src/array/polygon.rs

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,10 @@ pub(super) fn check(
5656
));
5757
}
5858

59-
if *geom_offsets.last() as usize != ring_offsets.len_proxy() {
59+
// Offset can be smaller than length if sliced
60+
if *geom_offsets.last() as usize > ring_offsets.len_proxy() {
6061
return Err(GeoArrowError::InvalidGeoArrow(
61-
"largest geometry offset must match ring offsets length".to_string(),
62+
"largest geometry offset must not be longer than ring offsets length".to_string(),
6263
));
6364
}
6465

@@ -488,4 +489,24 @@ mod test {
488489

489490
assert_ne!(arr1, arr2.slice(0, 2));
490491
}
492+
493+
#[test]
494+
fn test_validation_with_sliced_array() {
495+
let arr = polygon::array(CoordType::Interleaved, Dimension::XY);
496+
let sliced = arr.slice(0, 1);
497+
498+
let back =
499+
PolygonArray::try_from((sliced.to_array_ref().as_ref(), arr.extension_type().clone()))
500+
.unwrap();
501+
assert_eq!(back.len(), 1);
502+
}
503+
504+
#[test]
505+
fn test_validation_with_array_sliced_by_arrow_rs() {
506+
let arr = polygon::array(CoordType::Interleaved, Dimension::XY);
507+
let sliced = arr.to_array_ref().slice(0, 1);
508+
509+
let back = PolygonArray::try_from((sliced.as_ref(), arr.extension_type().clone())).unwrap();
510+
assert_eq!(back.len(), 1);
511+
}
491512
}

0 commit comments

Comments
 (0)