Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gdal 2.0.0 #20627

Closed
qgib opened this issue Mar 31, 2015 · 9 comments
Closed

gdal 2.0.0 #20627

qgib opened this issue Mar 31, 2015 · 9 comments
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter! Data Provider Related to specific vector, raster or mesh data providers
Milestone

Comments

@qgib
Copy link
Contributor

qgib commented Mar 31, 2015

Author Name: Mark Johnson (Mark Johnson)
Original Redmine Issue: 12479
Affected QGIS version: 2.8.1
Redmine category:data_provider/ogr
Assignee: Nyall Dawson


I have been working with the gdal 2.0.0 development code for a while and have noticed that due to the implementation of gdal RFC 41, code changes will be needed so that qgis will react in the same way with bot gdal 1.* and 2.*.

All of the changes are in
src/providers/ogr/qgsogrprovider.cpp

in the functions

  • QgsOgrProvider::subLayers()
  • QgsOgrProvider::getOgrGeomType

Also there is a problem in

  • QgsOgrProvider::setSubsetString
    where the needed dialect parameter is set to NULL instead of "OGRSQL"

All 3 cases effect only table that have MORE than 1 geometry.

Up to gdal 1.11.2, each geometry field is treated as 1 layer.
Starting with gdal 2.0 each table is treated a 1 layer and the geometry fields of that table must be retrieved.

For QgsOgrProvider::getOgrGeomType only one line must be changed:
from:

to:

For subLayers() it is a bit more comlecated, since now 2 loops are needed

  1. Layer
  2. fields of layers
  if ( !mSubLayerList.isEmpty() )
    return mSubLayerList;
  int layer_number=0; // depending on the gdal-version being used, the final result may be different that the result of layerCount()
  int layer_count=layerCount();
  for ( int i = 0; i < layer_count ; i++ )
  {
    OGRLayerH layer = OGR_DS_GetLayer( ogrDataSource, i );
    OGRFeatureDefnH fdef = OGR_L_GetLayerDefn( layer );
    QString theLayerName = FROM8( OGR_FD_GetName( fdef ) );
    int fieldCount=OGR_FD_GetGeomFieldCount(fdef);
    for(int j=0; j<fieldCount; j++ )
    {
     QString theLayerFieldName = FROM8(OGR_GFld_GetNameRef(OGR_FD_GetGeomFieldDefn(fdef,j)));
     OGRwkbGeometryType layerGeomType=OGR_GFld_GetType(OGR_FD_GetGeomFieldDefn(fdef,j))
     QgsDebugMsg( QString( "layerGeomType = %1" ).arg( layerGeomType ) );
     if ( layerGeomType != wkbUnknown )
     { // gdal up to version 1.11.2 will return a layer-name using the ogr-format 'table_name(field_name)'. gdal starting with 2.0.0 will only return the table_name
      int theLayerFeatureCount = OGR_L_GetFeatureCount( layer, j );
      QString geom = ogrWkbGeometryTypeName( layerGeomType );
      // QgsMessageLog::logMessage( tr( "subLayers(%1) layer[%2 - %3/%4] OGR_FD_GetName[%5] field_count[%6/%7] OGR_GFld_GetNameRef[%8] layerGeomType[%9] feature_count[%10]" ).arg(GDALVersionInfo( "RELEASE_NAME" )).arg(layer_number).arg(i).arg(layer_count).arg( theLayerName).arg(j).arg(fieldCount).arg( theLayerFieldName).arg(layerGeomType).arg(theLayerFeatureCount), tr( "OGR" ) );    
      QString layer_name=QString("%1(%2)").arg( theLayerName ).arg(theLayerFieldName);  
      if ((fieldCount == 1)  || (theLayerName.endsWith( QString("(%1)").arg(theLayerFieldName))))
      { // gdal previous 2.0: on tables with 1 geometry, may not use the ogr-format 'table_name(field_name)' ; or already formatted in the  ogr-format 'table_name(field_name)'
       layer_name=QString("%1").arg(theLayerName);
      }
      mSubLayerList << QString( "%1:%2:%3:%4" ).arg( layer_number++ ).arg(layer_name).arg( theLayerFeatureCount == -1 ? tr( "Unknown" ) : QString::number( theLayerFeatureCount ) ).arg( geom );   
     }
     else
     { // This may not be needed
      QgsDebugMsg( "Unknown geometry type, count features for each geometry type" );
      .. 
     }
    }

All of the OGR function used existed in both in gdal 1.* and 2..
The main difference between 1.
and 2.0 is that label-name syntax 'table_name(field_name)'
in 1.: may only be used in a table with more than 1 geometry
in 2.
may be used for all geometries fields

Unfortunately I cannot test this on the present master code, since I cannot get it compiled.
This is the reason I have not submitted a diff or pull request.

These changes were tested on the qgis that I use and can compile (2.1.0) and return that same result when using gdal 1.11.2 and the present gdal 2.0.0dev code.
I have, however, looked a the ogr specific code in the areas that were change and it looksthe same to me.
The only difference is that now a ```if ( wkbFlatten( layerGeomType ) != wkbUnknown )



@qgib
Copy link
Contributor Author

qgib commented Mar 31, 2015

Author Name: Mark Johnson (Mark Johnson)


For setSubsetString, the 'OGRSQL' parameter must be used on layers that use the 'table_name(field_name)' convention.

this matter was talked about in the gdal ticket:

http://trac.osgeo.org/gdal/ticket/5903

and the final conclusion of Even Rouault was that this code must be adapted to work correctly with gdal 2.0 in cases where there is more than 1 geometry in a table.

@qgib
Copy link
Contributor Author

qgib commented Mar 31, 2015

Author Name: Giovanni Manghi (@gioman)


Hi,

many thanks for this. I would appreciate a lot if you could raise this issues also on the qgis developers mailing list and/or on qgis github repo with a patch. Thanks!


  • status_id was changed from Open to Feedback

@qgib
Copy link
Contributor Author

qgib commented Mar 31, 2015

Author Name: Mark Johnson (Mark Johnson)


Since I cannot compile the present version, creating a patch is difficult since there are other changes in the file.

@qgib
Copy link
Contributor Author

qgib commented Apr 5, 2015

Author Name: Paolo Cavallini (@pcav)


  • priority_id was changed from Normal to High

@qgib
Copy link
Contributor Author

qgib commented Apr 5, 2015

Author Name: Jürgen Fischer (@jef-n)


  • priority_id was changed from High to Normal
  • status_id was changed from Feedback to Open

@qgib
Copy link
Contributor Author

qgib commented Mar 20, 2017

Author Name: Mark Johnson (Mark Johnson)


This matter needs to be resolved for QGIS 3.


  • fixed_version_id was changed from Future Release - High Priority to Version 3.0

@qgib
Copy link
Contributor Author

qgib commented Apr 30, 2017

Author Name: Giovanni Manghi (@gioman)


  • regression was configured as 0
  • easy_fix was configured as 0

@qgib
Copy link
Contributor Author

qgib commented May 6, 2018

Author Name: Nyall Dawson (@nyalldawson)


Fixed in QGIS 3


  • status_id was changed from Open to Closed
  • description was changed from I have been working with the gdal 2.0.0 development code for a while and have noticed that due to the implementation of gdal RFC 41, code changes will be needed so that qgis will react in the same way with bot gdal 1.* and 2.*.

All of the changes are in
src/providers/ogr/qgsogrprovider.cpp

in the functions

  • QgsOgrProvider::subLayers()
  • QgsOgrProvider::getOgrGeomType

Also there is a problem in

  • QgsOgrProvider::setSubsetString
    where the needed dialect parameter is set to NULL instead of "OGRSQL"

All 3 cases effect only table that have MORE than 1 geometry.

Up to gdal 1.11.2, each geometry field is treated as 1 layer.
Starting with gdal 2.0 each table is treated a 1 layer and the geometry fields of that table must be retrieved.

For QgsOgrProvider::getOgrGeomType only one line must be changed:
from:

geomType = OGR_FD_GetGeomType( fdef );

to:

geomType = OGR_GFld_GetType(OGR_FD_GetGeomFieldDefn(fdef, 0));

For subLayers() it is a bit more comlecated, since now 2 loops are needed

  1. Layer
  2. fields of layers
  if ( !mSubLayerList.isEmpty() )
    return mSubLayerList;
  int layer_number=0; // depending on the gdal-version being used, the final result may be different that the result of layerCount()
  int layer_count=layerCount();
  for ( int i = 0; i < layer_count ; i++ )
  {
    OGRLayerH layer = OGR_DS_GetLayer( ogrDataSource, i );
    OGRFeatureDefnH fdef = OGR_L_GetLayerDefn( layer );
    QString theLayerName = FROM8( OGR_FD_GetName( fdef ) );
    int fieldCount=OGR_FD_GetGeomFieldCount(fdef);
    for(int j=0; j

All of the OGR function used existed in both in gdal 1.* and 2..
The main difference between 1.
and 2.0 is that label-name syntax 'table_name(field_name)'
in 1.: may only be used in a table with more than 1 geometry
in 2.
may be used for all geometries fields

Unfortunately I cannot test this on the present master code, since I cannot get it compiled.
This is the reason I have not submitted a diff or pull request.

These changes were tested on the qgis that I use and can compile (2.1.0) and return that same result when using gdal 1.11.2 and the present gdal 2.0.0dev code.
I have, however, looked a the ogr specific code in the areas that were change and it looksthe same to me.
The only difference is that now a

if ( wkbFlatten( layerGeomType ) != wkbUnknown )
is done, but since there is no 'wkbUnknown25D' there is noting to flatten and is really not needed.
to I have been working with the gdal 2.0.0 development code for a while and have noticed that due to the implementation of gdal RFC 41, code changes will be needed so that qgis will react in the same way with bot gdal 1.* and 2.*.

All of the changes are in
src/providers/ogr/qgsogrprovider.cpp

in the functions

  • QgsOgrProvider::subLayers()
  • QgsOgrProvider::getOgrGeomType

Also there is a problem in

  • QgsOgrProvider::setSubsetString
    where the needed dialect parameter is set to NULL instead of "OGRSQL"

All 3 cases effect only table that have MORE than 1 geometry.

Up to gdal 1.11.2, each geometry field is treated as 1 layer.
Starting with gdal 2.0 each table is treated a 1 layer and the geometry fields of that table must be retrieved.

For QgsOgrProvider::getOgrGeomType only one line must be changed:
from:

geomType = OGR_FD_GetGeomType( fdef );

to:

geomType = OGR_GFld_GetType(OGR_FD_GetGeomFieldDefn(fdef, 0));

For subLayers() it is a bit more comlecated, since now 2 loops are needed

  1. Layer
  2. fields of layers
  if ( !mSubLayerList.isEmpty() )
    return mSubLayerList;
  int layer_number=0; // depending on the gdal-version being used, the final result may be different that the result of layerCount()
  int layer_count=layerCount();
  for ( int i = 0; i < layer_count ; i++ )
  {
    OGRLayerH layer = OGR_DS_GetLayer( ogrDataSource, i );
    OGRFeatureDefnH fdef = OGR_L_GetLayerDefn( layer );
    QString theLayerName = FROM8( OGR_FD_GetName( fdef ) );
    int fieldCount=OGR_FD_GetGeomFieldCount(fdef);
    for(int j=0; j

All of the OGR function used existed in both in gdal 1.* and 2..
The main difference between 1.
and 2.0 is that label-name syntax 'table_name(field_name)'
in 1.: may only be used in a table with more than 1 geometry
in 2.
may be used for all geometries fields

Unfortunately I cannot test this on the present master code, since I cannot get it compiled.
This is the reason I have not submitted a diff or pull request.

These changes were tested on the qgis that I use and can compile (2.1.0) and return that same result when using gdal 1.11.2 and the present gdal 2.0.0dev code.
I have, however, looked a the ogr specific code in the areas that were change and it looksthe same to me.
The only difference is that now a

if ( wkbFlatten( layerGeomType ) != wkbUnknown )
is done, but since there is no 'wkbUnknown25D' there is noting to flatten and is really not needed.

@qgib qgib closed this as completed May 6, 2018
@qgib
Copy link
Contributor Author

qgib commented May 6, 2018

Author Name: Mark Johnson (Mark Johnson)


Are you sure this has been corrected?

fdef.GetGeomFieldCount() will return the amount, thus a loop should exist from 0 to amount

QgsOgrProvider::addSubLayerDetailsToSubLayerList
  QgsOgrFeatureDefn &fdef = layer->GetLayerDefn();
  // Get first column name,
  // TODO: add support for multiple
  QString geometryColumnName;
  if ( fdef.GetGeomFieldCount() )
  {
    OGRGeomFieldDefnH geomH = fdef.GetGeomFieldDefn( 0 );
    geometryColumnName = QString::fromUtf8( OGR_GFld_GetNameRef( geomH ) );
  }

This code retrieves only the first, so if more than one exists the others will not be listed.
There is also the 'TODO' comment to add support for this.


  • assigned_to_id was configured as Nyall Dawson

@qgib qgib added Bug Either a bug report, or a bug fix. Let's hope for the latter! Data Provider Related to specific vector, raster or mesh data providers labels May 25, 2019
@qgib qgib added this to the Version 3.0 milestone May 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Either a bug report, or a bug fix. Let's hope for the latter! Data Provider Related to specific vector, raster or mesh data providers
Projects
None yet
Development

No branches or pull requests

1 participant