Use real field type instead of a forced conversion through string

for OGR provider minimum/maximum/unique value retrieval Avoids precision loss due to this string conversion, notably resulting in graduated ranges which don't quite encompass the full range of values present in a layer. Fixes #32667, #27420 (cherry picked from commit d65f218)
qgis · Nov 14, 2019 · 4033bd4 · 4033bd4
1 parent 68d7058
commit 4033bd4
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 13 deletions.
diff --git a/src/core/providers/ogr/qgsogrprovider.cpp b/src/core/providers/ogr/qgsogrprovider.cpp
@@ -3714,7 +3714,7 @@ QSet<QVariant> QgsOgrProvider::uniqueValues( int index, int limit ) const
   if ( !mValid || index < 0 || index >= mAttributeFields.count() )
     return uniqueValues;
 
-  QgsField fld = mAttributeFields.at( index );
+  const QgsField fld = mAttributeFields.at( index );
   if ( fld.name().isNull() )
   {
     return uniqueValues; //not a provider field
@@ -3751,9 +3751,13 @@ QSet<QVariant> QgsOgrProvider::uniqueValues( int index, int limit ) const
   }
 
   gdal::ogr_feature_unique_ptr f;
+  bool ok = false;
   while ( f.reset( l->GetNextFeature() ), f )
   {
-    uniqueValues << ( OGR_F_IsFieldSetAndNotNull( f.get(), 0 ) ? convertValue( fld.type(), textEncoding()->toUnicode( OGR_F_GetFieldAsString( f.get(), 0 ) ) ) : QVariant( fld.type() ) );
+    const QVariant res = QgsOgrUtils::getOgrFeatureAttribute( f.get(), fld, 0, textEncoding(), &ok );
+    if ( ok )
+      uniqueValues << res;
+
     if ( limit >= 0 && uniqueValues.size() >= limit )
       break;
   }
@@ -3813,7 +3817,7 @@ QVariant QgsOgrProvider::minimumValue( int index ) const
   {
     return QVariant();
   }
-  QgsField fld = mAttributeFields.at( 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(" + quotedIdentifier( textEncoding()->fromUnicode( fld.name() ) );
@@ -3837,8 +3841,11 @@ QVariant QgsOgrProvider::minimumValue( int index ) const
     return QVariant();
   }
 
-  QVariant value = OGR_F_IsFieldSetAndNotNull( f.get(), 0 ) ? convertValue( fld.type(), textEncoding()->toUnicode( OGR_F_GetFieldAsString( f.get(), 0 ) ) ) : QVariant( fld.type() );
-  return value;
+  bool ok = false;
+  const QVariant res = QgsOgrUtils::getOgrFeatureAttribute( f.get(), fld, 0, textEncoding(), &ok );
+  if ( !ok )
+    return QVariant();
+  return res;
 }
 
 QVariant QgsOgrProvider::maximumValue( int index ) const
@@ -3847,7 +3854,7 @@ QVariant QgsOgrProvider::maximumValue( int index ) const
   {
     return QVariant();
   }
-  QgsField fld = mAttributeFields.at( 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(" + quotedIdentifier( textEncoding()->fromUnicode( fld.name() ) );
@@ -3871,8 +3878,11 @@ QVariant QgsOgrProvider::maximumValue( int index ) const
     return QVariant();
   }
 
-  QVariant value = OGR_F_IsFieldSetAndNotNull( f.get(), 0 ) ? convertValue( fld.type(), textEncoding()->toUnicode( OGR_F_GetFieldAsString( f.get(), 0 ) ) ) : QVariant( fld.type() );
-  return value;
+  bool ok = false;
+  const QVariant res = QgsOgrUtils::getOgrFeatureAttribute( f.get(), fld, 0, textEncoding(), &ok );
+  if ( !ok )
+    return QVariant();
+  return res;
 }
 
 QByteArray QgsOgrProvider::quotedIdentifier( const QByteArray &field ) const

diff --git a/src/core/qgsogrutils.cpp b/src/core/qgsogrutils.cpp
@@ -169,9 +169,23 @@ QgsFields QgsOgrUtils::readOgrFields( OGRFeatureH ogrFet, QTextCodec *encoding )
   return fields;
 }
 
+
 QVariant QgsOgrUtils::getOgrFeatureAttribute( OGRFeatureH ogrFet, const QgsFields &fields, int attIndex, QTextCodec *encoding, bool *ok )
 {
-  if ( !ogrFet || attIndex < 0 || attIndex >= fields.count() )
+  if ( attIndex < 0 || attIndex >= fields.count() )
+  {
+    if ( ok )
+      *ok = false;
+    return QVariant();
+  }
+
+  const QgsField field = fields.at( attIndex );
+  return getOgrFeatureAttribute( ogrFet, field, attIndex, encoding, ok );
+}
+
+QVariant QgsOgrUtils::getOgrFeatureAttribute( OGRFeatureH ogrFet, const QgsField &field, int attIndex, QTextCodec *encoding, bool *ok )
+{
+  if ( !ogrFet || attIndex < 0 )
   {
     if ( ok )
       *ok = false;
@@ -196,7 +210,7 @@ QVariant QgsOgrUtils::getOgrFeatureAttribute( OGRFeatureH ogrFet, const QgsField
 
   if ( OGR_F_IsFieldSetAndNotNull( ogrFet, attIndex ) )
   {
-    switch ( fields.at( attIndex ).type() )
+    switch ( field.type() )
     {
       case QVariant::String:
       {
@@ -225,9 +239,9 @@ QVariant QgsOgrUtils::getOgrFeatureAttribute( OGRFeatureH ogrFet, const QgsField
         int year, month, day, hour, minute, second, tzf;
 
         OGR_F_GetFieldAsDateTime( ogrFet, attIndex, &year, &month, &day, &hour, &minute, &second, &tzf );
-        if ( fields.at( attIndex ).type() == QVariant::Date )
+        if ( field.type() == QVariant::Date )
           value = QDate( year, month, day );
-        else if ( fields.at( attIndex ).type() == QVariant::Time )
+        else if ( field.type() == QVariant::Time )
           value = QTime( hour, minute, second );
         else
           value = QDateTime( QDate( year, month, day ), QTime( hour, minute, second ) );
@@ -250,7 +264,7 @@ QVariant QgsOgrUtils::getOgrFeatureAttribute( OGRFeatureH ogrFet, const QgsField
 
       case QVariant::List:
       {
-        if ( fields.at( attIndex ).subType() == QVariant::String )
+        if ( field.subType() == QVariant::String )
         {
           QStringList list;
           char **lst = OGR_F_GetFieldAsStringList( ogrFet, attIndex );

diff --git a/src/core/qgsogrutils.h b/src/core/qgsogrutils.h
@@ -191,6 +191,20 @@ class CORE_EXPORT QgsOgrUtils
      */
     static QVariant getOgrFeatureAttribute( OGRFeatureH ogrFet, const QgsFields &fields, int attIndex, QTextCodec *encoding, bool *ok = nullptr );
 
+    /**
+     * Retrieves an attribute value from an OGR feature, using a provided \a field definition.
+     * \param ogrFet OGR feature handle
+     * \param field definition of corresponding field
+     * \param attIndex index of attribute to retrieve from \a ogrFet
+     * \param encoding text encoding
+     * \param ok optional storage for success of retrieval
+     * \returns attribute converted to a QVariant object
+     * \see readOgrFeatureAttributes()
+     *
+     * \since QGIS 3.10.1
+     */
+    static QVariant getOgrFeatureAttribute( OGRFeatureH ogrFet, const QgsField &field, int attIndex, QTextCodec *encoding, bool *ok = nullptr );
+
     /**
      * Reads all attributes from an OGR feature into a QgsFeature.
      * \param ogrFet OGR feature handle