Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Merge pull request #9071 from elpaso/bugfix-21100-ogrprovider-missing…
…-quoteidentifier

Quote field name identifiers in ORDER BY and MAX/MIN queries

Cherry-picked from master 3c9b6e5
  • Loading branch information
elpaso authored and nyalldawson committed Feb 6, 2019
1 parent b292edd commit 2831a02
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 4 deletions.
8 changes: 4 additions & 4 deletions src/providers/ogr/qgsogrprovider.cpp
Expand Up @@ -3486,7 +3486,7 @@ QSet<QVariant> QgsOgrProvider::uniqueValues( int index, int limit ) const
sql += " WHERE " + textEncoding()->fromUnicode( mSubsetString );
}

sql += " ORDER BY " + textEncoding()->fromUnicode( fld.name() ) + " ASC"; // quoting of fieldname produces a syntax error
sql += " ORDER BY " + quotedIdentifier( textEncoding()->fromUnicode( fld.name() ) ) + " ASC";

QgsDebugMsg( QStringLiteral( "SQL: %1" ).arg( textEncoding()->toUnicode( sql ) ) );
QgsOgrLayerUniquePtr l = mOgrLayer->ExecuteSQL( sql );
Expand Down Expand Up @@ -3531,7 +3531,7 @@ QStringList QgsOgrProvider::uniqueStringsMatching( int index, const QString &sub
sql += " AND (" + textEncoding()->fromUnicode( mSubsetString ) + ')';
}

sql += " ORDER BY " + textEncoding()->fromUnicode( fld.name() ) + " ASC"; // quoting of fieldname produces a syntax error
sql += " ORDER BY " + quotedIdentifier( textEncoding()->fromUnicode( fld.name() ) ) + " ASC";

QgsDebugMsg( QStringLiteral( "SQL: %1" ).arg( textEncoding()->toUnicode( sql ) ) );
QgsOgrLayerUniquePtr l = mOgrLayer->ExecuteSQL( sql );
Expand Down Expand Up @@ -3563,7 +3563,7 @@ QVariant QgsOgrProvider::minimumValue( int index ) const
QgsField fld = mAttributeFields.at( index );

// Don't quote column name (see https://trac.osgeo.org/gdal/ticket/5799#comment:9)
QByteArray sql = "SELECT MIN(" + textEncoding()->fromUnicode( fld.name() );
QByteArray sql = "SELECT MIN(" + quotedIdentifier( textEncoding()->fromUnicode( fld.name() ) );
sql += ") FROM " + quotedIdentifier( mOgrLayer->name() );

if ( !mSubsetString.isEmpty() )
Expand Down Expand Up @@ -3597,7 +3597,7 @@ QVariant QgsOgrProvider::maximumValue( int index ) const
QgsField fld = mAttributeFields.at( index );

// Don't quote column name (see https://trac.osgeo.org/gdal/ticket/5799#comment:9)
QByteArray sql = "SELECT MAX(" + textEncoding()->fromUnicode( fld.name() );
QByteArray sql = "SELECT MAX(" + quotedIdentifier( textEncoding()->fromUnicode( fld.name() ) );
sql += ") FROM " + quotedIdentifier( mOgrLayer->name() );

if ( !mSubsetString.isEmpty() )
Expand Down
73 changes: 73 additions & 0 deletions tests/src/python/test_provider_ogr_gpkg.py
Expand Up @@ -1230,6 +1230,79 @@ def testTransaction(self):
self.assertEqual(len([f for f in vl2_external.getFeatures(QgsFeatureRequest())]), 1)
del vl2_external

def testJson(self):
if int(gdal.VersionInfo('VERSION_NUM')) < GDAL_COMPUTE_VERSION(2, 4, 0):
return

tmpfile = os.path.join(self.basetestpath, 'test_json.gpkg')
testdata_path = unitTestDataPath('provider')
shutil.copy(os.path.join(unitTestDataPath('provider'), 'test_json.gpkg'), tmpfile)

vl = QgsVectorLayer('{}|layerid=0'.format(tmpfile, 'foo', 'ogr'))
self.assertTrue(vl.isValid())

fields = vl.dataProvider().fields()
self.assertEqual(fields.at(fields.indexFromName('json_content')).type(), QVariant.Map)

fi = vl.getFeatures(QgsFeatureRequest())
f = QgsFeature()

#test reading dict value from attribute
while fi.nextFeature(f):
if f['fid'] == 1:
self.assertIsInstance(f['json_content'], dict)
self.assertEqual(f['json_content'], {'foo': 'bar'})
#test changing dict value in attribute
f['json_content'] = {'foo': 'baz'}
self.assertEqual(f['json_content'], {'foo': 'baz'})
#test changint dict to list
f['json_content'] = ['eins', 'zwei', 'drei']
self.assertEqual(f['json_content'], ['eins', 'zwei', 'drei'])
#test changing list value in attribute
f['json_content'] = ['eins', 'zwei', 'drei', 4]
self.assertEqual(f['json_content'], ['eins', 'zwei', 'drei', 4])
#test changing to complex json structure
f['json_content'] = {'name': 'Lily', 'age': '0', 'cars': {'car1': ['fiat tipo', 'fiat punto', 'davoser schlitten'], 'car2': 'bobbycar', 'car3': 'tesla'}}
self.assertEqual(f['json_content'], {'name': 'Lily', 'age': '0', 'cars': {'car1': ['fiat tipo', 'fiat punto', 'davoser schlitten'], 'car2': 'bobbycar', 'car3': 'tesla'}})

#test adding attribute
vl.startEditing()
self.assertTrue(vl.addAttribute(QgsField('json_content2', QVariant.Map, "JSON", 60, 0, 'no comment', QVariant.String)))
self.assertTrue(vl.commitChanges())

vl.startEditing()
self.assertTrue(vl.addAttribute(QgsField('json_content3', QVariant.Map, "JSON", 60, 0, 'no comment', QVariant.String)))
self.assertTrue(vl.commitChanges())

#test setting values to new attributes
while fi.nextFeature(f):
if f['fid'] == 2:
f['json_content'] = {'uno': 'foo'}
f['json_content2'] = ['uno', 'due', 'tre']
f['json_content3'] = {'uno': ['uno', 'due', 'tre']}
self.assertEqual(f['json_content'], {'foo': 'baz'})
self.assertEqual(f['json_content2'], ['uno', 'due', 'tre'])
self.assertEqual(f['json_content3'], {'uno': ['uno', 'due', 'tre']})

#test deleting attribute
vl.startEditing()
self.assertTrue(vl.deleteAttribute(vl.fields().indexFromName('json_content3')))
self.assertTrue(vl.commitChanges())

#test if index of existent field is not -1 and the one of the deleted is -1
self.assertNotEqual(vl.fields().indexFromName('json_content2'), -1)
self.assertEqual(vl.fields().indexFromName('json_content3'), -1)

def test_quote_identifier(self):
"""Regression #21100"""

tmpfile = os.path.join(self.basetestpath, 'bug21100-wierd_field_names.gpkg') # spellok
shutil.copy(os.path.join(unitTestDataPath(''), 'bug21100-wierd_field_names.gpkg'), tmpfile) # spellok
vl = QgsVectorLayer('{}|layerid=0'.format(tmpfile), 'foo', 'ogr')
self.assertTrue(vl.isValid())
for i in range(1, len(vl.fields())):
self.assertEqual(vl.uniqueValues(i), {'a', 'b', 'c'})


if __name__ == '__main__':
unittest.main()
Binary file added tests/testdata/bug21100-wierd_field_names.gpkg
Binary file not shown.

0 comments on commit 2831a02

Please sign in to comment.