Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge pull request #6173 from elpaso/bugfix-17863-wrong-ogr-extent
[bugfix][ogr][spatialite] Update extent when subsetstring is set in the ctor
  • Loading branch information
elpaso committed Jan 26, 2018
2 parents b1a97e1 + 698befa commit 2997c3b
Show file tree
Hide file tree
Showing 6 changed files with 165 additions and 10 deletions.
19 changes: 9 additions & 10 deletions src/providers/ogr/qgsogrprovider.cpp
Expand Up @@ -3909,10 +3909,10 @@ void QgsOgrProvider::open( OpenMode mode )
QgsDebugMsg( QString( "Trying %1 syntax, mFilePath= %2" ).arg( vsiPrefix, mFilePath ) );
}

QgsDebugMsg( "mFilePath: " + mFilePath );
QgsDebugMsg( "mLayerIndex: " + QString::number( mLayerIndex ) );
QgsDebugMsg( "mLayerName: " + mLayerName );
QgsDebugMsg( "mSubsetString: " + mSubsetString );
QgsDebugMsgLevel( "mFilePath: " + mFilePath, 3 );
QgsDebugMsgLevel( "mLayerIndex: " + QString::number( mLayerIndex ), 3 );
QgsDebugMsgLevel( "mLayerName: " + mLayerName, 3 );
QgsDebugMsgLevel( "mSubsetString: " + mSubsetString, 3 );
CPLSetConfigOption( "OGR_ORGANIZE_POLYGONS", "ONLY_CCW" ); // "SKIP" returns MULTIPOLYGONs for multiringed POLYGONs
CPLSetConfigOption( "GPX_ELE_AS_25D", "YES" ); // use GPX elevation as z values

Expand Down Expand Up @@ -4053,7 +4053,11 @@ void QgsOgrProvider::open( OpenMode mode )
int featuresCountedBackup = mFeaturesCounted;
mFeaturesCounted = -1;
// Do not update capabilities here
mValid = _setSubsetString( mSubsetString, false, false );
// but ensure subset is set (setSubsetString does nothing if the passed sql subset string is equal to
// mSubsetString, which is the case when reloading the dataset)
QString origSubsetString = mSubsetString;
mSubsetString.clear();
mValid = _setSubsetString( origSubsetString, false, false );
mFeaturesCounted = featuresCountedBackup;
}
}
Expand Down Expand Up @@ -5254,7 +5258,6 @@ QGISEXTERN bool saveStyle( const QString &uri, const QString &qmlStyle, const QS
if ( !hLayer )
{
errCause = QObject::tr( "Unable to save layer style. It's not possible to create the destination table on the database." );
mutex->unlock();
return false;
}
bool ok = true;
Expand Down Expand Up @@ -5318,7 +5321,6 @@ QGISEXTERN bool saveStyle( const QString &uri, const QString &qmlStyle, const QS
if ( !ok )
{
errCause = QObject::tr( "Unable to save layer style. It's not possible to create the destination table on the database." );
mutex->unlock();
return false;
}
}
Expand Down Expand Up @@ -5375,7 +5377,6 @@ QGISEXTERN bool saveStyle( const QString &uri, const QString &qmlStyle, const QS
QMessageBox::Yes | QMessageBox::No ) == QMessageBox::No ) )
{
errCause = QObject::tr( "Operation aborted" );
mutex->unlock();
return false;
}
bNew = false;
Expand Down Expand Up @@ -5427,8 +5428,6 @@ QGISEXTERN bool saveStyle( const QString &uri, const QString &qmlStyle, const QS
else
bFeatureOK = OGR_L_SetFeature( hLayer, hFeature.get() ) == OGRERR_NONE;

mutex->unlock();

if ( !bFeatureOK )
{
QgsMessageLog::logMessage( QObject::tr( "Error updating style" ) );
Expand Down
4 changes: 4 additions & 0 deletions src/providers/spatialite/qgsspatialiteprovider.cpp
Expand Up @@ -602,6 +602,10 @@ QgsSpatiaLiteProvider::QgsSpatiaLiteProvider( QString const &uri )
<< QgsVectorDataProvider::NativeType( tr( "Array of decimal numbers (double)" ), SPATIALITE_ARRAY_PREFIX.toUpper() + "REAL" + SPATIALITE_ARRAY_SUFFIX.toUpper(), QVariant::List, 0, 0, 0, 0, QVariant::Double )
<< QgsVectorDataProvider::NativeType( tr( "Array of whole numbers (integer)" ), SPATIALITE_ARRAY_PREFIX.toUpper() + "INTEGER" + SPATIALITE_ARRAY_SUFFIX.toUpper(), QVariant::List, 0, 0, 0, 0, QVariant::LongLong )
);
// Update extent and feature count
if ( ! mSubsetString.isEmpty() )
setSubsetString( mSubsetString, true );

mValid = true;
}

Expand Down
37 changes: 37 additions & 0 deletions tests/src/python/test_provider_ogr_gpkg.py
Expand Up @@ -13,6 +13,7 @@
__revision__ = '$Format:%H$'

import os
import re
import shutil
import sys
import tempfile
Expand Down Expand Up @@ -860,6 +861,42 @@ def testSubSetStringEditable_bug17795(self):
vl.setSubsetString('')
self.assertTrue(vl.dataProvider().capabilities() & isEditable)

def testSubsetStringExtent_bug17863(self):
"""Check that the extent is correct when applied in the ctor and when
modified after a subset string is set """

def _lessdigits(s):
return re.sub(r'(\d+\.\d{3})\d+', r'\1', s)

testPath = TEST_DATA_DIR + '/' + 'provider/bug_17795.gpkg|layername=bug_17795'
subSetString = '"name" = \'int\''
subSet = '|layername=bug_17795|subset=%s' % subSetString

# unfiltered
vl = QgsVectorLayer(testPath, 'test', 'ogr')
self.assertTrue(vl.isValid())
unfiltered_extent = _lessdigits(vl.extent().toString())
del(vl)

# filter after construction ...
subSet_vl2 = QgsVectorLayer(testPath, 'test', 'ogr')
self.assertEqual(_lessdigits(subSet_vl2.extent().toString()), unfiltered_extent)
# ... apply filter now!
subSet_vl2.setSubsetString(subSetString)
self.assertEqual(subSet_vl2.subsetString(), subSetString)
self.assertNotEqual(_lessdigits(subSet_vl2.extent().toString()), unfiltered_extent)
filtered_extent = _lessdigits(subSet_vl2.extent().toString())
del(subSet_vl2)

# filtered in constructor
subSet_vl = QgsVectorLayer(testPath + subSet, 'subset_test', 'ogr')
self.assertEqual(subSet_vl.subsetString(), subSetString)
self.assertTrue(subSet_vl.isValid())

# This was failing in bug 17863
self.assertEqual(_lessdigits(subSet_vl.extent().toString()), filtered_extent)
self.assertNotEqual(_lessdigits(subSet_vl.extent().toString()), unfiltered_extent)


if __name__ == '__main__':
unittest.main()
37 changes: 37 additions & 0 deletions tests/src/python/test_provider_shapefile.py
Expand Up @@ -13,6 +13,7 @@
__revision__ = '$Format:%H$'

import os
import re
import tempfile
import shutil
import glob
Expand Down Expand Up @@ -635,6 +636,42 @@ def testSubSetStringEditable_bug17795(self):
vl.setSubsetString('')
self.assertTrue(vl.dataProvider().capabilities() & isEditable)

def testSubsetStringExtent_bug17863(self):
"""Check that the extent is correct when applied in the ctor and when
modified after a subset string is set """

def _lessdigits(s):
return re.sub(r'(\d+\.\d{3})\d+', r'\1', s)

testPath = TEST_DATA_DIR + '/' + 'points.shp'
subSetString = '"Class" = \'Biplane\''
subSet = '|layerid=0|subset=%s' % subSetString

# unfiltered
vl = QgsVectorLayer(testPath, 'test', 'ogr')
self.assertTrue(vl.isValid())
unfiltered_extent = _lessdigits(vl.extent().toString())
del(vl)

# filter after construction ...
subSet_vl2 = QgsVectorLayer(testPath, 'test', 'ogr')
self.assertEqual(_lessdigits(subSet_vl2.extent().toString()), unfiltered_extent)
# ... apply filter now!
subSet_vl2.setSubsetString(subSetString)
self.assertEqual(subSet_vl2.subsetString(), subSetString)
self.assertNotEqual(_lessdigits(subSet_vl2.extent().toString()), unfiltered_extent)
filtered_extent = _lessdigits(subSet_vl2.extent().toString())
del(subSet_vl2)

# filtered in constructor
subSet_vl = QgsVectorLayer(testPath + subSet, 'subset_test', 'ogr')
self.assertEqual(subSet_vl.subsetString(), subSetString)
self.assertTrue(subSet_vl.isValid())

# This was failing in bug 17863
self.assertEqual(_lessdigits(subSet_vl.extent().toString()), filtered_extent)
self.assertNotEqual(_lessdigits(subSet_vl.extent().toString()), unfiltered_extent)


if __name__ == '__main__':
unittest.main()
78 changes: 78 additions & 0 deletions tests/src/python/test_provider_spatialite.py
Expand Up @@ -15,6 +15,7 @@
import qgis # NOQA

import os
import re
import sys
import shutil
import tempfile
Expand Down Expand Up @@ -187,6 +188,37 @@ def setUpClass(cls):
sql = "CREATE TABLE test_defaults (id INTEGER NOT NULL PRIMARY KEY, name TEXT DEFAULT 'qgis ''is good', number INTEGER DEFAULT 5, number2 REAL DEFAULT 5.7, no_default REAL)"
cur.execute(sql)

# simple table with catgorized points
sql = "CREATE TABLE test_filter (id INTEGER NOT NULL PRIMARY KEY, name TEXT NOT NULL)"
cur.execute(sql)
sql = "SELECT AddGeometryColumn('test_filter', 'geometry', 4326, 'POINT', 'XY')"
cur.execute(sql)
sql = "INSERT INTO test_filter (id, name, geometry) "
sql += "VALUES (1, 'ext', GeomFromText('POINT(0 0)', 4326))"
cur.execute(sql)
sql = "INSERT INTO test_filter (id, name, geometry) "
sql += "VALUES (2, 'ext', GeomFromText('POINT(0 3)', 4326))"
cur.execute(sql)
sql = "INSERT INTO test_filter (id, name, geometry) "
sql += "VALUES (3, 'ext', GeomFromText('POINT(3 3)', 4326))"
cur.execute(sql)
sql = "INSERT INTO test_filter (id, name, geometry) "
sql += "VALUES (4, 'ext', GeomFromText('POINT(3 0)', 4326))"
cur.execute(sql)

sql = "INSERT INTO test_filter (id, name, geometry) "
sql += "VALUES (5, 'int', GeomFromText('POINT(1 1)', 4326))"
cur.execute(sql)
sql = "INSERT INTO test_filter (id, name, geometry) "
sql += "VALUES (6, 'int', GeomFromText('POINT(1 2)', 4326))"
cur.execute(sql)
sql = "INSERT INTO test_filter (id, name, geometry) "
sql += "VALUES (7, 'int', GeomFromText('POINT(2 2)', 4326))"
cur.execute(sql)
sql = "INSERT INTO test_filter (id, name, geometry) "
sql += "VALUES (8, 'int', GeomFromText('POINT(2 1)', 4326))"
cur.execute(sql)

cur.execute("COMMIT")
con.close()

Expand Down Expand Up @@ -663,6 +695,52 @@ def testCreateAttributeIndex(self):
self.assertEqual(set(indexed_columns), set(['name', 'number']))
con.close()

def testSubsetStringExtent_bug17863(self):
"""Check that the extent is correct when applied in the ctor and when
modified after a subset string is set """

def _lessdigits(s):
return re.sub(r'(\d+\.\d{3})\d+', r'\1', s)

testPath = "dbname=%s table='test_filter' (geometry) key='id'" % self.dbname

subSetString = '"name" = \'int\''
subSet = ' sql=%s' % subSetString

# unfiltered
vl = QgsVectorLayer(testPath, 'test', 'spatialite')
self.assertTrue(vl.isValid())
self.assertEqual(vl.featureCount(), 8)
unfiltered_extent = _lessdigits(vl.extent().toString())
self.assertNotEqual('Empty', unfiltered_extent)
del(vl)

# filter after construction ...
subSet_vl2 = QgsVectorLayer(testPath, 'test', 'spatialite')
self.assertEqual(_lessdigits(subSet_vl2.extent().toString()), unfiltered_extent)
self.assertEqual(subSet_vl2.featureCount(), 8)
# ... apply filter now!
subSet_vl2.setSubsetString(subSetString)
self.assertEqual(subSet_vl2.featureCount(), 4)
self.assertEqual(subSet_vl2.subsetString(), subSetString)
self.assertNotEqual(_lessdigits(subSet_vl2.extent().toString()), unfiltered_extent)
filtered_extent = _lessdigits(subSet_vl2.extent().toString())
del(subSet_vl2)

# filtered in constructor
subSet_vl = QgsVectorLayer(testPath + subSet, 'subset_test', 'spatialite')
self.assertEqual(subSet_vl.subsetString(), subSetString)
self.assertTrue(subSet_vl.isValid())

# This was failing in bug 17863
self.assertEqual(subSet_vl.featureCount(), 4)
self.assertEqual(_lessdigits(subSet_vl.extent().toString()), filtered_extent)
self.assertNotEqual(_lessdigits(subSet_vl.extent().toString()), unfiltered_extent)

self.assertTrue(subSet_vl.setSubsetString(''))
self.assertEqual(subSet_vl.featureCount(), 8)
self.assertEqual(_lessdigits(subSet_vl.extent().toString()), unfiltered_extent)


if __name__ == '__main__':
unittest.main()
Binary file modified tests/testdata/provider/bug_17795.gpkg
Binary file not shown.

0 comments on commit 2997c3b

Please sign in to comment.