georust / gdal

Rust bindings for GDAL
https://crates.io/crates/gdal
MIT License
367 stars 94 forks source link

Error creating a gdb file with OpenFileGDB driver #392

Closed callmeahab closed 1 year ago

callmeahab commented 1 year ago

Trying to write to a file with OpenFileGDB driver returns this error:

GDAL method 'GDALCreate' returned a NULL pointer. Error msg: 'Only vector datasets supported'.

This is the abbreviated version of how I'm creating a gdb file:

    let temp_path = env::temp_dir().join(path);
    let driver = DriverManager::get_driver_by_name("OpenFileGDB")?;
   driver.create_vector_only(temp_path)?

There are no errors when using proprietary FileGDB driver, but OpenFileGDB should support this operation starting from 3.5 version

Thanks

lnicola commented 1 year ago

Which GDAL version are you running?

callmeahab commented 1 year ago

I'm running gdal 3.6.4

callmeahab commented 1 year ago

Changing this line to OpenFileGDB results in the same error: https://github.com/georust/gdal/blob/master/src/vector/layer.rs#L1210

callmeahab commented 1 year ago

This is what I get for ogrinfo --formats:

FITS -raster,vector- (rw+): Flexible Image Transport System
  PCIDSK -raster,vector- (rw+v): PCIDSK Database File
  netCDF -raster,multidimensional raster,vector- (rw+s): Network Common Data Format
  PDS4 -raster,vector- (rw+vs): NASA Planetary Data System 4
  VICAR -raster,vector- (rw+v): MIPL VICAR file
  JP2OpenJPEG -raster,vector- (rwv): JPEG-2000 driver based on OpenJPEG library
  PDF -raster,vector- (rw+vs): Geospatial PDF
  MBTiles -raster,vector- (rw+v): MBTiles
  BAG -raster,multidimensional raster,vector- (rw+v): Bathymetry Attributed Grid
  EEDA -vector- (ro): Earth Engine Data API
  OGCAPI -raster,vector- (rov): OGCAPI
  ESRI Shapefile -vector- (rw+v): ESRI Shapefile
  MapInfo File -vector- (rw+v): MapInfo File
  UK .NTF -vector- (rov): UK .NTF
  LVBAG -vector- (rov): Kadaster LV BAG Extract 2.0
  OGR_SDTS -vector- (rov): SDTS
  S57 -vector- (rw+v): IHO S-57 (ENC)
  DGN -vector- (rw+v): Microstation DGN
  OGR_VRT -vector- (rov): VRT - Virtual Datasource
  Memory -vector- (rw+): Memory
  CSV -vector- (rw+v): Comma Separated Value (.csv)
  NAS -vector- (rov): NAS - ALKIS
  GML -vector- (rw+v): Geography Markup Language (GML)
  GPX -vector- (rw+v): GPX
  LIBKML -vector- (rw+v): Keyhole Markup Language (LIBKML)
  KML -vector- (rw+v): Keyhole Markup Language (KML)
  GeoJSON -vector- (rw+v): GeoJSON
  GeoJSONSeq -vector- (rw+v): GeoJSON Sequence
  ESRIJSON -vector- (rov): ESRIJSON
  TopoJSON -vector- (rov): TopoJSON
  Interlis 1 -vector- (rw+v): Interlis 1
  Interlis 2 -vector- (rw+v): Interlis 2
  OGR_GMT -vector- (rw+v): GMT ASCII Vectors (.gmt)
  GPKG -raster,vector- (rw+vs): GeoPackage
  SQLite -vector- (rw+v): SQLite / Spatialite
  ODBC -vector- (ro):
  WAsP -vector- (rw+v): WAsP .map format
  PGeo -vector- (ro): ESRI Personal GeoDatabase
  MSSQLSpatial -vector- (rw+): Microsoft SQL Server Spatial Database
  PostgreSQL -vector- (rw+): PostgreSQL/PostGIS
  OpenFileGDB -vector- (rw+v): ESRI FileGDB
  DXF -vector- (rw+v): AutoCAD DXF
  CAD -raster,vector- (rovs): AutoCAD Driver
  FlatGeobuf -vector- (rw+v): FlatGeobuf
  Geoconcept -vector- (rw+v): Geoconcept
  GeoRSS -vector- (rw+v): GeoRSS
  VFK -vector- (ro): Czech Cadastral Exchange Data Format
  PGDUMP -vector- (w+v): PostgreSQL SQL dump
  OSM -vector- (rov): OpenStreetMap XML and PBF
  GPSBabel -vector- (rw+): GPSBabel
  OGR_PDS -vector- (rov): Planetary Data Systems TABLE
  WFS -vector- (rov): OGC WFS (Web Feature Service)
  OAPIF -vector- (ro): OGC API - Features
  EDIGEO -vector- (rov): French EDIGEO exchange format
  SVG -vector- (rov): Scalable Vector Graphics
  Idrisi -vector- (rov): Idrisi Vector (.vct)
  XLS -vector- (ro): MS Excel format
  ODS -vector- (rw+v): Open Document/ LibreOffice / OpenOffice Spreadsheet
  XLSX -vector- (rw+v): MS Office Open XML spreadsheet
  Elasticsearch -vector- (rw+): Elastic Search
  Carto -vector- (rw+): Carto
  AmigoCloud -vector- (rw+): AmigoCloud
  SXF -vector- (rov): Storage and eXchange Format
  Selafin -vector- (rw+v): Selafin
  JML -vector- (rw+v): OpenJUMP JML
  PLSCENES -raster,vector- (ro): Planet Labs Scenes API
  CSW -vector- (ro): OGC CSW (Catalog  Service for the Web)
  VDV -vector- (rw+v): VDV-451/VDV-452/INTREST Data Format
  GMLAS -vector- (rwv): Geography Markup Language (GML) driven by application schemas
  MVT -vector- (rw+v): Mapbox Vector Tiles
  NGW -raster,vector- (rw+s): NextGIS Web
  MapML -vector- (rw+v): MapML
  Parquet -vector- (rw+v): (Geo)Parquet
  Arrow -vector- (rw+v): (Geo)Arrow IPC File Format / Stream
  TIGER -vector- (rov): U.S. Census TIGER/Line
  AVCBin -vector- (rov): Arc/Info Binary Coverage
  AVCE00 -vector- (rov): Arc/Info E00 (ASCII) Coverage
  HTTP -raster,vector- (ro): HTTP Fetching Wrapper
callmeahab commented 1 year ago

This works fine in c++

#include <iostream>
#include <ogr_api.h>
#include <gdal_priv.h>

int main() {
    const char *pszDriverName = "OpenFileGDB";
    GDALDriver *poDriver;

    GDALAllRegister();

    poDriver = GetGDALDriverManager()->GetDriverByName(pszDriverName);
    if (poDriver == NULL) {
        printf("%s driver not available.\n", pszDriverName);
        exit(1);
    }

    GDALDataset *poDS;

    poDS = poDriver->Create("sample.gdb", 0, 0, 0, GDT_Unknown, NULL);
    if (poDS == NULL) {
        printf("Creation of output file failed.\n");
        exit(1);
    }

    return 0;
}
lnicola commented 1 year ago

Does poDriver->Create("sample.gdb", 0, 0, 0, GDT_Byte, NULL) work? Can't check right now.

I think we're playing a bit fast and loose with the create functions.

callmeahab commented 1 year ago

Sorry, yeah with GDT_Byte I'm getting the same error: Only vector datasets supported

lnicola commented 1 year ago

Yeah, we're picking u8 in create_vector_only. I wonder if it's a new change, if it was always broken, or if it only happens for some drivers. The tests pass, so maybe the latter.

callmeahab commented 1 year ago

Other drivers seem to work, I've found only OpenFileGDB breaking

callmeahab commented 1 year ago

If I change that argument to 0, which I assume is GDT_Unknown gdb file gets created, but I get: Unsupported geometry type for create layer function

lnicola commented 1 year ago

In the Rust or C++ version? It was working before in C++ with GDT_Unknown.

callmeahab commented 1 year ago

Works in c++, error's in rust, I'm updating this test https://github.com/georust/gdal/blob/master/src/vector/layer.rs#L1210 on the master branch

callmeahab commented 1 year ago

This is what I have changed here: https://github.com/georust/gdal/blob/master/src/driver.rs#L208

let c_dataset = unsafe {
    gdal_sys::GDALCreate(
        self.c_driver,
        c_filename.as_ptr(),
        0,
        0,
        0,
        0,
        null_mut(),
    )
};
lnicola commented 1 year ago

That's okay. The GDALCreate works, but the format doesn't support wkbUnknown geometries.

let mut layer = ds
    .create_layer(crate::LayerOptions {
        name: "test",
        srs: None,
        ty: OGRwkbGeometryType::wkbPoint,
        options: None,
    })
    .unwrap();

should work, assuming you update the rest of the test (extensions and removal at the end).

callmeahab commented 1 year ago

Yeah that works, do you want me to create a pr for this change?

Thanks a lot for your help.

lnicola commented 1 year ago

You can't change it directly to GDT_Unknown because it will break raster dataset. But yeah, a PR is always welcome.

For a minimal change, you can make _create_with_band_type_with_options take the GDALDataType instead of a generic.

callmeahab commented 1 year ago

Of course, I was thinking of creating a new function for vector datasets, since they don't use any of those options, but I can also add GDALDataType argument to existing function. Do I need to write a test that covers specifically .gdb files as well?

lnicola commented 1 year ago

I think create_vector_only + an updated _create_with_band_type_with_options should be enough.

Don't bother with the test, it will cause problems on older GDAL versions. Or you can gate it with a #[cfg], we do that in many places.

callmeahab commented 1 year ago

Ok, thanks

callmeahab commented 1 year ago

@lnicola Perhaps I'm missing something but removing the generic seems too disruptive when I looked at the code in more detail. Perhaps I can add a case for u128 and treat it as GDT_Unknown. Or write a new function just for vector creation.

lnicola commented 1 year ago

I wanted to file this against GDAL, but noticed there's an OpenFileGDB raster driver in 3.7, and GDALCreate can't know whether we want a raster or a vector dataset.

CC @rouault just in case. TL;DR we noticed that creating vector OpenFileGDB datasets was failing in 3.6 because we were passing GDT_Byte to GDALCreate. Switching to GDT_Unknown fixed it, but our previous code appeared to work with at least some of the other drivers.