From cf23d56aef0bca6d6f958dd00cc97587dace89e2 Mon Sep 17 00:00:00 2001 From: Sandro Mani Date: Mon, 25 Sep 2017 13:19:09 +0200 Subject: [PATCH 1/3] [OGR] Followup: Add orig_ogc_fid as last field to avoid changing field order --- src/providers/ogr/qgsogrfeatureiterator.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/providers/ogr/qgsogrfeatureiterator.cpp b/src/providers/ogr/qgsogrfeatureiterator.cpp index 314792b2534..4ce78d7bb0c 100644 --- a/src/providers/ogr/qgsogrfeatureiterator.cpp +++ b/src/providers/ogr/qgsogrfeatureiterator.cpp @@ -346,7 +346,12 @@ bool QgsOgrFeatureIterator::readFeature( OGRFeatureH fet, QgsFeature &feature ) { if ( mOrigFidAdded ) { - feature.setId( OGR_F_GetFieldAsInteger64( fet, 0 ) ); + OGRFeatureDefnH fdef = OGR_L_GetLayerDefn( ogrLayer ); + int lastField = OGR_FD_GetFieldCount( fdef ) - 1; + if ( lastField >= 0 ) + feature.setId( OGR_F_GetFieldAsInteger64( fet, lastField ) ); + else + feature.setId( OGR_F_GetFID( fet ) ); } else { From 15eaafdf0ce3117970d47050df972df3ce92a2c5 Mon Sep 17 00:00:00 2001 From: Sandro Mani Date: Mon, 25 Sep 2017 13:20:01 +0200 Subject: [PATCH 2/3] [OGR] Ensure orig_ogc_fid is never set as ignored field --- src/providers/ogr/qgsogrfeatureiterator.cpp | 1 + src/providers/ogr/qgsogrprovider.cpp | 6 +++++- tests/src/python/test_provider_ogr_sqlite.py | 12 ++++++++++++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/providers/ogr/qgsogrfeatureiterator.cpp b/src/providers/ogr/qgsogrfeatureiterator.cpp index 4ce78d7bb0c..04fef288c1b 100644 --- a/src/providers/ogr/qgsogrfeatureiterator.cpp +++ b/src/providers/ogr/qgsogrfeatureiterator.cpp @@ -176,6 +176,7 @@ QgsOgrFeatureIterator::QgsOgrFeatureIterator( QgsOgrFeatureSource *source, bool OGR_L_SetAttributeFilter( ogrLayer, nullptr ); } + //start with first feature rewind(); } diff --git a/src/providers/ogr/qgsogrprovider.cpp b/src/providers/ogr/qgsogrprovider.cpp index ac7953f14d3..4da8a7b43f4 100644 --- a/src/providers/ogr/qgsogrprovider.cpp +++ b/src/providers/ogr/qgsogrprovider.cpp @@ -1022,7 +1022,11 @@ void QgsOgrProviderUtils::setRelevantFields( OGRLayerH ogrLayer, int fieldCount, if ( !fetchAttributes.contains( i ) ) { // add to ignored fields - ignoredFields.append( OGR_Fld_GetNameRef( OGR_FD_GetFieldDefn( featDefn, firstAttrIsFid ? i - 1 : i ) ) ); + const char *fieldName = OGR_Fld_GetNameRef( OGR_FD_GetFieldDefn( featDefn, firstAttrIsFid ? i - 1 : i ) ); + if ( qstrcmp( fieldName, "orig_ogc_fid" ) != 0 ) + { + ignoredFields.append( fieldName ); + } } } diff --git a/tests/src/python/test_provider_ogr_sqlite.py b/tests/src/python/test_provider_ogr_sqlite.py index f4d5e07ef4f..5e573161d2f 100644 --- a/tests/src/python/test_provider_ogr_sqlite.py +++ b/tests/src/python/test_provider_ogr_sqlite.py @@ -254,6 +254,18 @@ class TestPyQgsOGRProviderSqlite(unittest.TestCase): self.assertTrue(it.nextFeature(f)) self.assertTrue(f.id() == 5) + # Ensure that orig_ogc_fid is still retrieved even if attribute subset is passed + req = QgsFeatureRequest() + req.setSubsetOfAttributes([]) + it = vl.getFeatures(req) + ids = [] + while it.nextFeature(f): + ids.append(f.id()) + self.assertTrue(len(ids) == 3) + self.assertTrue(3 in ids) + self.assertTrue(4 in ids) + self.assertTrue(5 in ids) + # Check that subset string is correctly set on reload vl.reload() self.assertTrue(vl.fields().at(vl.fields().count() - 1).name() == "orig_ogc_fid") From 6b7201f4edb8ba34962d6f88fb7114fe15620c25 Mon Sep 17 00:00:00 2001 From: Sandro Mani Date: Mon, 25 Sep 2017 13:47:54 +0200 Subject: [PATCH 3/3] [OGR] Use a ORIG_OGC_FID constant instead of hard-coding orig_ogc_fid --- src/providers/ogr/qgsogrprovider.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/providers/ogr/qgsogrprovider.cpp b/src/providers/ogr/qgsogrprovider.cpp index 4da8a7b43f4..8af552c897a 100644 --- a/src/providers/ogr/qgsogrprovider.cpp +++ b/src/providers/ogr/qgsogrprovider.cpp @@ -88,6 +88,8 @@ static const QString TEXT_PROVIDER_DESCRIPTION = static OGRwkbGeometryType ogrWkbGeometryTypeFromName( const QString &typeName ); +static const QByteArray ORIG_OGC_FID = "orig_ogc_fid"; + bool QgsOgrProvider::convertField( QgsField &field, const QTextCodec &encoding ) { @@ -1023,7 +1025,7 @@ void QgsOgrProviderUtils::setRelevantFields( OGRLayerH ogrLayer, int fieldCount, { // add to ignored fields const char *fieldName = OGR_Fld_GetNameRef( OGR_FD_GetFieldDefn( featDefn, firstAttrIsFid ? i - 1 : i ) ); - if ( qstrcmp( fieldName, "orig_ogc_fid" ) != 0 ) + if ( qstrcmp( fieldName, ORIG_OGC_FID ) != 0 ) { ignoredFields.append( fieldName ); } @@ -3564,7 +3566,7 @@ OGRLayerH QgsOgrProviderUtils::setSubsetString( OGRLayerH layer, GDALDatasetH ds fidColumn = "FID"; } - QByteArray sql = sqlPart1 + ", " + fidColumn + " as orig_ogc_fid" + sqlPart3; + QByteArray sql = sqlPart1 + ", " + fidColumn + " as " + ORIG_OGC_FID + sqlPart3; QgsDebugMsg( QString( "SQL: %1" ).arg( encoding->toUnicode( sql ) ) ); subsetLayer = GDALDatasetExecuteSQL( ds, sql.constData(), nullptr, nullptr ); @@ -3572,7 +3574,7 @@ OGRLayerH QgsOgrProviderUtils::setSubsetString( OGRLayerH layer, GDALDatasetH ds // If execute SQL fails because it did not find the fidColumn, retry with hardcoded FID if ( !subsetLayer ) { - QByteArray sql = sqlPart1 + ", " + "FID as orig_ogc_fid" + sqlPart3; + QByteArray sql = sqlPart1 + ", " + "FID as " + ORIG_OGC_FID + sqlPart3; QgsDebugMsg( QString( "SQL: %1" ).arg( encoding->toUnicode( sql ) ) ); subsetLayer = GDALDatasetExecuteSQL( ds, sql.constData(), nullptr, nullptr ); } @@ -3594,7 +3596,7 @@ OGRLayerH QgsOgrProviderUtils::setSubsetString( OGRLayerH layer, GDALDatasetH ds if ( fieldCount > 0 ) { OGRFieldDefnH fldDef = OGR_FD_GetFieldDefn( fdef, fieldCount - 1 ); - origFidAdded = qstrcmp( OGR_Fld_GetNameRef( fldDef ), "orig_ogc_fid" ) == 0; + origFidAdded = qstrcmp( OGR_Fld_GetNameRef( fldDef ), ORIG_OGC_FID ) == 0; } }