FR: add wrapper for OGR_F_SetFieldNull

[nms-scribe]

All of the "get" field functions on Feature return options, so I can know if the value is already null. But I don't see anything in the rust API for setting a field to None that shouldn't have a value, and OGR_F_UnsetField is only referenced in the pre-built bindings on a sourcecode search here.

My work-around right now is to use feature.set_field_double(<field_name>,f64::NAN) but I've only had to do this for double fields so far, so I don't know if this will work for integer, or if it will work in other drivers than geopackage.

[nms-scribe]

To update, that NAN workaround will not work correctly for integers, as it places a very low integer (assuming MIN) in the field instead. Also will not work for string fields.

However, setting it as an empty double list does work.

[lnicola]

We should add this, and you can actually add it yourself in the meanwhile (e.g. with an extension trait), but there are two things related things we should figure out:

• I wanted to get Fix use-after-free in Dataset::close #420 into a new release and yank the version currently on crates.io before merging any new changes, but I don't know when that will happen
• I think we should offer "by index" versions of these methods. I recently saw an app spending about 50% of its CPU time in a version strcasecmp just because of the current set_field API

[nms-scribe]

I'm a little embarrassed, after playing around with an extension trait and finding some problems, I realized that the function I really wanted a wrapper for is OGR_F_SetFieldNull. I've updated the title.

I'm not sure what unsetting a field does, it led to some random errors like ERROR 1: failed to bind FID '3200' to statement but only when the feature is updated. I feel it may have corrupted my data, as it looked strange in QGIS (fortunately it was easy to recreate). But when I found OGR_F_SetFieldNull and used that instead, everything looks fine.

Anyway, here's my trait if anyone else following this wants it:

use gdal::errors::GdalError;
use gdal::vector::Feature;
use gdal_sys;
use std::ffi::CString;

pub(crate) trait FeatureFix {
    fn set_field_null(&self, field_name: &str) -> Result<(),GdalError>;
}

impl FeatureFix for Feature<'_> {

    fn set_field_null(&self, field_name: &str) -> Result<(),GdalError> {

        // copied from `field_idx_from_name` because it was private
        let c_str_field_name = CString::new(field_name)?;
        let field_id = unsafe { gdal_sys::OGR_F_GetFieldIndex(self.c_feature(), c_str_field_name.as_ptr()) };
        if field_id == -1 {
            return Err(GdalError::InvalidFieldName {
                field_name: field_name.to_string(),
                method_name: "OGR_F_GetFieldIndex",
            });
        }

        unsafe { gdal_sys::OGR_F_SetFieldNull(self.c_feature(), field_id) };
        Ok(())
    }
}

[nms-scribe]

@lnicola , @metasim As it looks like the code in the commit was copied from my suggestion, I think there's been a mistake. Maybe a lack of communication on my part.

A comment in my code indicated that a large portion of it was copied from field_idx_from_name, because I was using a trait and couldn't access that private function.

Unless I'm missing something, shouldn't this fix be implemented with a call to field_idx_from_name to replace the code I copied from that, so more like:

fn set_field_null(&self, field_name: &str) -> Result<(),GdalError> {

        let field_id = self::field_idx_from_name(field_name)?;

        unsafe { gdal_sys::OGR_F_SetFieldNull(self.c_feature(), field_id) };
        Ok(())
    }

(Snippet above has not been checked for errors or tested)

[lnicola]

@nms-scribe you're right, thank you. I was even complaining earlier that field_idx_from_name is private, but didn't notice it here. Will fix tomorrow or so.