Add Defn::geometry_field_index and Feature::geometry_field_index, drop Feature::geometry_by_name

Author: lnicola

CHANGES.md

@@ -14,16 +14,19 @@
   - Update `Feature::field_count` to return `usize` instead of `i32` ([#581](https://github.com/georust/gdal/pull/581))
   - Update `Feature::field_as_integer`, `Feature::field_as_integer64`, `Feature::field_as_double`, `Feature::field_as_string`, `Feature::field_as_datetime` to take the field index as `usize` instead of `i32` ([#581](https://github.com/georust/gdal/pull/581))
   - Drop `LayerAccess::create_feature_fields` ([#581](https://github.com/georust/gdal/pull/581))
+  - Drop `Feature::geometry_by_name` ([#594](https://github.com/georust/gdal/pull/594))
   - Update `SpatialRef::auth_name`, `SpatialRef::name`, `SpatialRef::angular_units_name`, `SpatialRef::linear_units_name` to return `Option<String>` instead of `Result<String>` ([#589](https://github.com/georust/gdal/pull/589))
 
 ### Added
 
   - Add `Dataset::delete_layer` ([#583](https://github.com/georust/gdal/pull/583))
+  - Add `Dataset::has_capability` ([#585](https://github.com/georust/gdal/pull/585))
   - Add a `bundled` feature for `gdal-sys` for building and statically linking a minimal bundled version of GDAL ([#517](https://github.com/georust/gdal/pull/517))
   - Add pre-built bindings for GDAL 3.10 ([#573](https://github.com/georust/gdal/pull/573))
   - Add methods `alternative_name`, `is_nullable`, `is_unique`, `default_value` to `Field` ([#561](https://github.com/georust/gdal/pull/561))
   - Add `Defn::geometry_type` ([#562](https://github.com/georust/gdal/pull/562))
   - Add `Defn::field_index` and `Feature::field_index` ([#581](https://github.com/georust/gdal/pull/581))
+  - Add `Defn::geometry_field_index` and `Feature::geometry_field_index` ([#594](https://github.com/georust/gdal/pull/594))
   - Add `Dataset::has_capability` for dataset capability check ([#581](https://github.com/georust/gdal/pull/585))
 
 ### Fixed

fixtures/two_geoms.csv

@@ -0,0 +1,2 @@
+WKT,_WKTanother_geometry
+"POINT (1 2)","POINT (10 20)"

src/vector/defn.rs

@@ -69,22 +69,53 @@ impl Defn {
 
     /// Get the index of a field.
     ///
+    /// The comparison is done case-insensitively, and if multiple fields match the requested
+    /// name, the first one is returned.
     /// If the field is missing, returns [`GdalError::InvalidFieldName`].
     ///
     pub fn field_index<S: AsRef<str>>(&self, field_name: S) -> Result<usize> {
-        let c_str_field_name = CString::new(field_name.as_ref())?;
+        self._field_index(field_name.as_ref())
+    }
+
+    fn _field_index(&self, field_name: &str) -> Result<usize> {
+        let c_str_field_name = CString::new(field_name)?;
         let field_idx =
             unsafe { gdal_sys::OGR_FD_GetFieldIndex(self.c_defn(), c_str_field_name.as_ptr()) };
         if field_idx == -1 {
             return Err(GdalError::InvalidFieldName {
-                field_name: field_name.as_ref().to_string(),
+                field_name: field_name.to_string(),
                 method_name: "OGR_FD_GetFieldIndex",
             });
         }
 
         let idx = field_idx.try_into()?;
         Ok(idx)
     }
+
+    /// Get the index of a geometry field.
+    ///
+    /// The comparison is done case-insensitively, and if multiple fields match the requested
+    /// name, the first one is returned.
+    /// If the field is missing, returns [`GdalError::InvalidFieldName`].
+    ///
+    pub fn geometry_field_index<S: AsRef<str>>(&self, field_name: S) -> Result<usize> {
+        self._geometry_field_index(field_name.as_ref())
+    }
+
+    fn _geometry_field_index(&self, field_name: &str) -> Result<usize> {
+        let c_str_field_name = CString::new(field_name)?;
+        let field_idx =
+            unsafe { gdal_sys::OGR_FD_GetGeomFieldIndex(self.c_defn(), c_str_field_name.as_ptr()) };
+        if field_idx == -1 {
+            return Err(GdalError::InvalidFieldName {
+                field_name: field_name.to_string(),
+                method_name: "OGR_FD_GetGeomFieldIndex",
+            });
+        }
+
+        let idx = field_idx.try_into()?;
+        Ok(idx)
+    }
 }
 
 pub struct FieldIterator<'a> {

src/vector/feature.rs

@@ -158,13 +158,18 @@ impl<'a> Feature<'a> {
     ///
     /// If the field is missing, returns [`GdalError::InvalidFieldName`].
     ///
+    /// Calling [`Defn::field_index`] once and caching the index can be faster, and should be preferred.
     pub fn field_index<S: AsRef<str>>(&self, field_name: S) -> Result<usize> {
-        let c_str_field_name = CString::new(field_name.as_ref())?;
+        self._field_index(field_name.as_ref())
+    }
+
+    fn _field_index(&self, field_name: &str) -> Result<usize> {
+        let c_str_field_name = CString::new(field_name)?;
         let field_idx =
             unsafe { gdal_sys::OGR_F_GetFieldIndex(self.c_feature(), c_str_field_name.as_ptr()) };
         if field_idx == -1 {
             return Err(GdalError::InvalidFieldName {
-                field_name: field_name.as_ref().to_string(),
+                field_name: field_name.to_string(),
                 method_name: "OGR_F_GetFieldIndex",
             });
         }
@@ -173,6 +178,31 @@ impl<'a> Feature<'a> {
         Ok(field_index)
     }
 
+    /// Get the index of a geometry field.
+    ///
+    /// If the field is missing, returns [`GdalError::InvalidFieldName`].
+    ///
+    /// Calling [`Defn::geometry_field_index`] once and caching the index can be faster, and should be preferred.
+    pub fn geometry_field_index<S: AsRef<str>>(&self, field_name: S) -> Result<usize> {
+        self._geometry_field_index(field_name.as_ref())
+    }
+
+    fn _geometry_field_index(&self, field_name: &str) -> Result<usize> {
+        let c_str_field_name = CString::new(field_name)?;
+        let field_idx = unsafe {
+            gdal_sys::OGR_F_GetGeomFieldIndex(self.c_feature(), c_str_field_name.as_ptr())
+        };
+        if field_idx == -1 {
+            return Err(GdalError::InvalidFieldName {
+                field_name: field_name.to_string(),
+                method_name: "OGR_F_GetGeomFieldIndex",
+            });
+        }
+
+        let field_index = field_idx.try_into()?;
+        Ok(field_index)
+    }
+
     /// Get the value of the specified field as a [`i32`].
     ///
     /// If the field is missing, returns [`GdalError::InvalidFieldIndex`].
@@ -365,20 +395,6 @@ impl<'a> Feature<'a> {
         }
     }
 
-    pub fn geometry_by_name(&self, field_name: &str) -> Result<&Geometry> {
-        let c_str_field_name = CString::new(field_name)?;
-        let idx =
-            unsafe { gdal_sys::OGR_F_GetGeomFieldIndex(self.c_feature, c_str_field_name.as_ptr()) };
-        if idx == -1 {
-            Err(GdalError::InvalidFieldName {
-                field_name: field_name.to_string(),
-                method_name: "geometry_by_name",
-            })
-        } else {
-            self.geometry_by_index(idx as usize)
-        }
-    }
-
     pub fn geometry_by_index(&self, idx: usize) -> Result<&Geometry> {
         if idx >= self.geometry.len() {
             return Err(GdalError::InvalidFieldIndex {

src/vector/layer.rs

@@ -1284,11 +1284,14 @@ mod tests {
             let geom = feature.geometry_by_index(0).unwrap();
             assert_eq!(geom.geometry_type(), OGRwkbGeometryType::wkbLineString);
             assert!(feature.geometry_by_index(1).is_err());
-            let geom = feature.geometry_by_name("");
+            let geom = feature.geometry_by_index(0);
             assert!(geom.is_ok());
-            let geom = feature.geometry_by_name("").unwrap();
+            let geom = feature.geometry_by_index(0).unwrap();
             assert_eq!(geom.geometry_type(), OGRwkbGeometryType::wkbLineString);
-            assert!(feature.geometry_by_name("FOO").is_err());
+            assert!(feature.geometry_by_index(1).is_err());
+
+            assert_eq!(feature.geometry_field_index("").unwrap(), 0);
+            assert!(feature.geometry_field_index("FOO").is_err());
         });
     }
 
@@ -1515,6 +1518,26 @@ mod tests {
         assert_eq!(geom_field.spatial_ref().unwrap(), spatial_ref2);
     }
 
+    #[test]
+    fn test_two_geom_fields() -> Result<()> {
+        let ds = Dataset::open(fixture("two_geoms.csv"))?;
+        let mut layer = ds.layer(0)?;
+
+        let geom_field_2_idx = layer
+            .defn()
+            .geometry_field_index("geom__WKTanother_geometry")
+            .unwrap();
+        assert_eq!(geom_field_2_idx, 1);
+
+        let feature = layer.features().next().unwrap();
+        let geom_1 = feature.geometry_by_index(0)?;
+        let geom_2 = feature.geometry_by_index(1)?;
+        assert_eq!(geom_1.get_point(0), (1.0, 2.0, 0.0));
+        assert_eq!(geom_2.get_point(0), (10.0, 20.0, 0.0));
+
+        Ok(())
+    }
+
     #[test]
     fn test_get_layer_by_name() {
         let ds = Dataset::open(fixture("roads.geojson")).unwrap();