Skip to content
Permalink
Browse files

Fix FID real fields shifted when exported to GPKG

Fixes #34333
  • Loading branch information
elpaso committed Feb 7, 2020
1 parent d2639bc commit b0fceeff321bd784e38a6bcdca60c80def0ff73f
@@ -408,11 +408,22 @@ QgsVectorLayerExporter::ExportError QgsOgrProvider::createEmptyLayer( const QStr
}

QString newLayerName( layerName );

std::unique_ptr< QgsVectorFileWriter > writer = qgis::make_unique< QgsVectorFileWriter >(
uri, encoding, fields, wkbType,
srs, driverName, dsOptions, layerOptions, nullptr,
QgsVectorFileWriter::NoSymbology, nullptr,
layerName, action, &newLayerName );
uri,
encoding,
fields,
wkbType,
srs,
driverName,
dsOptions,
layerOptions,
nullptr,
QgsVectorFileWriter::NoSymbology,
nullptr,
layerName,
action,
&newLayerName );
layerName = newLayerName;

QgsVectorFileWriter::WriterError error = writer->hasError();
@@ -429,6 +440,7 @@ QgsVectorLayerExporter::ExportError QgsOgrProvider::createEmptyLayer( const QStr

{
bool firstFieldIsFid = false;
bool fidColumnIsField = false;
if ( !layerName.isEmpty() )
{
gdal::dataset_unique_ptr hDS( GDALOpenEx( uri.toUtf8().constData(), GDAL_OF_VECTOR, nullptr, nullptr, nullptr ) );
@@ -439,17 +451,30 @@ QgsVectorLayerExporter::ExportError QgsOgrProvider::createEmptyLayer( const QStr
{
// Expose the OGR FID if it comes from a "real" column (typically GPKG)
// and make sure that this FID column is not exposed as a regular OGR field (shouldn't happen normally)
const QString ogrFidColumnName { OGR_L_GetFIDColumn( hLayer ) };
firstFieldIsFid = !( EQUAL( OGR_L_GetFIDColumn( hLayer ), "" ) ) &&
OGR_FD_GetFieldIndex( OGR_L_GetLayerDefn( hLayer ), OGR_L_GetFIDColumn( hLayer ) ) < 0 &&
fields.indexFromName( OGR_L_GetFIDColumn( hLayer ) ) < 0;

OGR_FD_GetFieldIndex( OGR_L_GetLayerDefn( hLayer ), ogrFidColumnName.toUtf8() ) < 0 &&
fields.indexFromName( ogrFidColumnName.toUtf8() ) < 0;
// At this point we must check if there is a real FID field in the the fields argument,
// because in that case we don't want to shift all fields (see issue GH #34333)
// Check for unique values should be performed in client code.
for ( const auto &f : qgis::as_const( fields ) )
{
if ( f.name().compare( ogrFidColumnName, Qt::CaseSensitivity::CaseInsensitive ) == 0 )
{
fidColumnIsField = true;
break;
}
}
}
}
}

const bool shiftColumnsByOne { firstFieldIsFid &&( ! fidColumnIsField ) };

for ( QMap<int, int>::const_iterator attrIt = attrIdxMap.constBegin(); attrIt != attrIdxMap.constEnd(); ++attrIt )
{
oldToNewAttrIdxMap->insert( attrIt.key(), *attrIt + ( firstFieldIsFid ? 1 : 0 ) );
oldToNewAttrIdxMap->insert( attrIt.key(), *attrIt + ( shiftColumnsByOne ? 1 : 0 ) );
}
}

@@ -1523,6 +1523,36 @@ def testMinMaxDateField(self):
self.assertEqual(vl.dataProvider().uniqueValues(2), {QDate(2017, 1, 1), NULL, QDate(2018, 1, 1), QDate(2019, 1, 1), QDate(2010, 1, 1)})
self.assertEqual(vl.dataProvider().uniqueValues(3), {QDateTime(2022, 1, 1, 1, 1, 1), NULL, QDateTime(2019, 1, 1, 1, 1, 1), QDateTime(2021, 1, 1, 1, 1, 1), QDateTime(2010, 1, 1, 1, 1, 1)})

def testExporterWithFIDColumn(self):
"""Test issue GH #34333, a memory layer with FID is not exported correctly to GPKG"""

vl = QgsVectorLayer(
'Point?crs=epsg:4326&field=FID:integer(0)&field=name:string(20)',
'test',
'memory')

self.assertTrue(vl.isValid(), 'Provider not initialized')

ft = QgsFeature(vl.fields())
ft.setAttributes([123, 'text1'])
ft.setGeometry(QgsGeometry.fromWkt('Point(2 49)'))
myResult, myFeatures = vl.dataProvider().addFeatures([ft])
self.assertTrue(myResult)
self.assertTrue(myFeatures)

dest_file_name = tempfile.mktemp('.gpkg')
err = QgsVectorLayerExporter.exportLayer(vl, dest_file_name, "ogr", vl.crs(), False)
self.assertEqual(err[0], QgsVectorLayerExporter.NoError,
'unexpected import error {0}'.format(err))

# Open result and check
created_layer = QgsVectorLayer(dest_file_name, 'test', 'ogr')
self.assertTrue(created_layer.isValid())
f = next(created_layer.getFeatures())
self.assertEqual(f.geometry().asWkt(), 'Point (2 49)')
self.assertEqual(f.attributes(), [123, 'text1'])
self.assertEqual(f.id(), 123)


if __name__ == '__main__':
unittest.main()
@@ -1168,6 +1168,40 @@ def testWriteKMLAxisOrderIssueGDAL3(self):
f = next(created_layer.getFeatures(QgsFeatureRequest()))
self.assertEqual(f.geometry().asWkt(), 'PointZ (2 49 0)')

def testWriteGpkgWithFID(self):
"""Check writing a memory layer with a FID column takes it as FID"""

vl = QgsVectorLayer(
'Point?crs=epsg:4326&field=FID:integer(0)&field=name:string(20)',
'test',
'memory')

self.assertTrue(vl.isValid(), 'Provider not initialized')

ft = QgsFeature(vl.fields())
ft.setAttributes([123, 'text1'])
ft.setGeometry(QgsGeometry.fromWkt('Point(2 49)'))
myResult, myFeatures = vl.dataProvider().addFeatures([ft])
self.assertTrue(myResult)
self.assertTrue(myFeatures)

dest_file_name = os.path.join(str(QDir.tempPath()), 'testWriteGpkgWithFID.gpkg')
write_result, error_message = QgsVectorFileWriter.writeAsVectorFormat(
vl,
dest_file_name,
'utf-8',
vl.crs(),
'GPKG')
self.assertEqual(write_result, QgsVectorFileWriter.NoError, error_message)

# Open result and check
created_layer = QgsVectorLayer(dest_file_name, 'test', 'ogr')
self.assertTrue(created_layer.isValid())
f = next(created_layer.getFeatures(QgsFeatureRequest()))
self.assertEqual(f.geometry().asWkt(), 'Point (2 49)')
self.assertEqual(f.attributes(), [123, 'text1'])
self.assertEqual(f.id(), 123)


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

0 comments on commit b0fceef

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