Skip to content
Permalink
Browse files

[OGR] Attempt to use actual ogr_fid also if subset string is set

If a subset string is set on an OGR layer, the feature iterator returns features with ids taken from a sequence starting from 0, regardless of the original feature id.

This causes a mismatch in the data shown by the identify results table and the attribute widget.
  • Loading branch information
manisandro committed Sep 2, 2017
1 parent fa8e1a7 commit 8eaeaaa0c0bb8ea12bba06eeed5778e3b93844db
@@ -42,6 +42,7 @@ QgsOgrFeatureIterator::QgsOgrFeatureIterator( QgsOgrFeatureSource* source, bool
, ogrLayer( nullptr )
, mSubsetStringSet( false )
, mFetchGeometry( false )
, mOrigFidAdded( false )
, mExpressionCompiled( false )
, mFilterFids( mRequest.filterFids() )
, mFilterFidsIt( mFilterFids.constBegin() )
@@ -67,7 +68,7 @@ QgsOgrFeatureIterator::QgsOgrFeatureIterator( QgsOgrFeatureSource* source, bool

if ( !mSource->mSubsetString.isEmpty() )
{
ogrLayer = QgsOgrProviderUtils::setSubsetString( ogrLayer, mConn->ds, mSource->mEncoding, mSource->mSubsetString );
ogrLayer = QgsOgrProviderUtils::setSubsetString( ogrLayer, mConn->ds, mSource->mEncoding, mSource->mSubsetString, mOrigFidAdded );
if ( !ogrLayer )
{
return;
@@ -171,7 +172,24 @@ bool QgsOgrFeatureIterator::nextFeatureFilterExpression( QgsFeature& f )
bool QgsOgrFeatureIterator::fetchFeatureWithId( QgsFeatureId id, QgsFeature& feature ) const
{
feature.setValid( false );
OGRFeatureH fet = OGR_L_GetFeature( ogrLayer, FID_TO_NUMBER( id ) );
OGRFeatureH fet;
if ( mOrigFidAdded )
{
OGR_L_ResetReading( ogrLayer );
while (( fet = OGR_L_GetNextFeature( ogrLayer ) ) )
{
if ( OGR_F_GetFieldAsInteger64( fet, 0 ) == id )
{
break;
}
OGR_F_Destroy( fet );
}
}
else
{
fet = OGR_L_GetFeature( ogrLayer, FID_TO_NUMBER( id ) );
}

if ( !fet )
{
return false;
@@ -296,7 +314,14 @@ void QgsOgrFeatureIterator::getFeatureAttribute( OGRFeatureH ogrFet, QgsFeature

bool QgsOgrFeatureIterator::readFeature( OGRFeatureH fet, QgsFeature& feature ) const
{
feature.setFeatureId( OGR_F_GetFID( fet ) );
if ( mOrigFidAdded )
{
feature.setFeatureId( OGR_F_GetFieldAsInteger64( fet, 0 ) );
}
else
{
feature.setFeatureId( OGR_F_GetFID( fet ) );
}
feature.initAttributes( mSource->mFields.count() );
feature.setFields( mSource->mFields ); // allow name-based attribute lookups

@@ -78,6 +78,7 @@ class QgsOgrFeatureIterator : public QgsAbstractFeatureIteratorFromSource<QgsOgr
OGRLayerH ogrLayer;

bool mSubsetStringSet;
bool mOrigFidAdded;

//! Set to true, if geometry is in the requested columns
bool mFetchGeometry;
@@ -3416,14 +3416,17 @@ OGRwkbGeometryType QgsOgrProvider::ogrWkbSingleFlatten( OGRwkbGeometryType type

OGRLayerH QgsOgrProvider::setSubsetString( OGRLayerH layer, OGRDataSourceH ds )
{
return QgsOgrProviderUtils::setSubsetString( layer, ds, mEncoding, mSubsetString );
bool origFidAdded = false;
return QgsOgrProviderUtils::setSubsetString( layer, ds, mEncoding, mSubsetString, origFidAdded );
}

OGRLayerH QgsOgrProviderUtils::setSubsetString( OGRLayerH layer, OGRDataSourceH ds, QTextCodec* encoding, const QString& subsetString )
OGRLayerH QgsOgrProviderUtils::setSubsetString( OGRLayerH layer, OGRDataSourceH ds, QTextCodec* encoding, const QString& subsetString, bool &origFidAdded )
{
QByteArray layerName = OGR_FD_GetName( OGR_L_GetLayerDefn( layer ) );
OGRSFDriverH ogrDriver = OGR_DS_GetDriver( ds );
QString ogrDriverName = OGR_Dr_GetName( ogrDriver );
bool origFidAddAttempted = false;
origFidAdded = false;

if ( ogrDriverName == "ODBC" ) //the odbc driver does not like schema names for subset
{
@@ -3440,12 +3443,33 @@ OGRLayerH QgsOgrProviderUtils::setSubsetString( OGRLayerH layer, OGRDataSourceH
sql = encoding->fromUnicode( subsetString );
else
{
sql = "SELECT * FROM " + quotedIdentifier( layerName, ogrDriverName );
QByteArray fidColumn = OGR_L_GetFIDColumn( layer );

sql = QByteArray( "SELECT " );
if ( !fidColumn.isEmpty() )
{
sql += fidColumn + " as orig_ogc_fid, ";
origFidAddAttempted = true;
}
sql += "* FROM " + quotedIdentifier( layerName, ogrDriverName );
sql += " WHERE " + encoding->fromUnicode( subsetString );
}

QgsDebugMsg( QString( "SQL: %1" ).arg( encoding->toUnicode( sql ) ) );
return OGR_DS_ExecuteSQL( ds, sql.constData(), nullptr, nullptr );
OGRLayerH subsetLayer = OGR_DS_ExecuteSQL( ds, sql.constData(), nullptr, nullptr );

// Check if first column is orig_ogc_fid
if ( origFidAddAttempted && subsetLayer )
{
OGRFeatureDefnH fdef = OGR_L_GetLayerDefn( subsetLayer );
if ( OGR_FD_GetFieldCount( fdef ) > 0 )
{
OGRFieldDefnH fldDef = OGR_FD_GetFieldDefn( fdef, 0 );
origFidAdded = qstrcmp( OGR_Fld_GetNameRef( fldDef ), "orig_ogc_fid" ) == 0;
}
}

return subsetLayer;
}

void QgsOgrProvider::open( OpenMode mode )
@@ -400,7 +400,7 @@ class QgsOgrProviderUtils
{
public:
static void setRelevantFields( OGRLayerH ogrLayer, int fieldCount, bool fetchGeometry, const QgsAttributeList &fetchAttributes, bool firstAttrIsFid );
static OGRLayerH setSubsetString( OGRLayerH layer, OGRDataSourceH ds, QTextCodec* encoding, const QString& subsetString );
static OGRLayerH setSubsetString( OGRLayerH layer, OGRDataSourceH ds, QTextCodec* encoding, const QString& subsetString, bool &origFidAdded );
static QByteArray quotedIdentifier( QByteArray field, const QString& ogrDriverName );

/** Quote a value for placement in a SQL string.
@@ -129,6 +129,57 @@ def testFidSupport(self):
got = [(f.attribute('fid'), f.attribute('intfield')) for f in vl.dataProvider().getFeatures(QgsFeatureRequest().setFilterExpression("fid = 12"))]
self.assertEqual(got, [(12, 123)])

def testSubsetStringFids(self):
""" tests that feature ids are stable even if a subset string is set """

tmpfile = os.path.join(self.basetestpath, 'subsetStringFids.sqlite')
ds = ogr.GetDriverByName('SQLite').CreateDataSource(tmpfile)
lyr = ds.CreateLayer('test', geom_type=ogr.wkbPoint, options=['FID=fid'])
lyr.CreateField(ogr.FieldDefn('type', ogr.OFTInteger))
lyr.CreateField(ogr.FieldDefn('value', ogr.OFTInteger))
f = ogr.Feature(lyr.GetLayerDefn())
f.SetFID(0)
f.SetField(0, 1)
f.SetField(1, 11)
lyr.CreateFeature(f)
f = ogr.Feature(lyr.GetLayerDefn())
f.SetFID(1)
f.SetField(0, 1)
f.SetField(1, 12)
lyr.CreateFeature(f)
f = ogr.Feature(lyr.GetLayerDefn())
f.SetFID(2)
f.SetField(0, 1)
f.SetField(1, 13)
lyr.CreateFeature(f)
f = ogr.Feature(lyr.GetLayerDefn())
f.SetFID(3)
f.SetField(0, 2)
f.SetField(1, 14)
lyr.CreateFeature(f)
f = ogr.Feature(lyr.GetLayerDefn())
f.SetFID(4)
f.SetField(0, 2)
f.SetField(1, 15)
lyr.CreateFeature(f)
f = ogr.Feature(lyr.GetLayerDefn())
f.SetFID(5)
f.SetField(0, 2)
f.SetField(1, 16)
lyr.CreateFeature(f)
f = None
ds = None

vl = QgsVectorLayer(tmpfile + "|subset=type=2", 'test', 'ogr')
self.assertTrue(vl.isValid())

req = QgsFeatureRequest()
req.setFilterExpression("value=16")
it = vl.getFeatures(req)
f = QgsFeature()
self.assertTrue(it.nextFeature(f))
self.assertTrue(f.id() == 5)


if __name__ == '__main__':
unittest.main()

0 comments on commit 8eaeaaa

Please sign in to comment.
You can’t perform that action at this time.