Skip to content
Permalink
Browse files

Merge pull request #5122 from manisandro/ogr_subset_fid

[OGR] Attempt to use actual ogr_fid also if subset string is set
  • Loading branch information
manisandro committed Sep 4, 2017
2 parents 399c2ca + 217e700 commit c43cd995a239734ab26f0615b4ac231b12d1cd1a
@@ -43,6 +43,7 @@ QgsOgrFeatureIterator::QgsOgrFeatureIterator( QgsOgrFeatureSource *source, bool
, mConn( nullptr )
, ogrLayer( nullptr )
, mSubsetStringSet( false )
, mOrigFidAdded( false )
, mFetchGeometry( false )
, mExpressionCompiled( false )
, mFilterFids( mRequest.filterFids() )
@@ -69,7 +70,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;
@@ -197,7 +198,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;
@@ -323,7 +341,14 @@ void QgsOgrFeatureIterator::getFeatureAttribute( OGRFeatureH ogrFet, QgsFeature

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

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

bool mSubsetStringSet;
bool mOrigFidAdded;

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

OGRLayerH QgsOgrProvider::setSubsetString( OGRLayerH layer, OGRDataSourceH ds )
{
return QgsOgrProviderUtils::setSubsetString( layer, ds, textEncoding(), mSubsetString );
bool origFidAdded = false;
return QgsOgrProviderUtils::setSubsetString( layer, ds, textEncoding(), 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 == QLatin1String( "ODBC" ) ) //the odbc driver does not like schema names for subset
{
@@ -3478,12 +3481,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 )
@@ -264,7 +264,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.
@@ -199,6 +199,57 @@ def testDefaultValues(self):
self.assertTrue(vl.dataProvider().defaultValue(5).secsTo(QTime.currentTime()) < 1)
self.assertTrue(vl.dataProvider().defaultValue(6).secsTo(QDateTime.currentDateTime()) < 1)

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 c43cd99

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