[needs-docs][postgres] Correctly handle binary (bytea) fields

Previously these would be silently dropped from the layers, but we now have comprehensive support for binary fields and should expose them. Fix sponsored by WhereGroup
qgis · Jul 1, 2019 · aa1bc5d · aa1bc5d
1 parent ff77edc
commit aa1bc5d
Show file tree

Hide file tree

Showing 5 changed files with 149 additions and 4 deletions.
diff --git a/src/providers/postgres/qgspostgresfeatureiterator.cpp b/src/providers/postgres/qgspostgresfeatureiterator.cpp
@@ -811,7 +811,41 @@ void QgsPostgresFeatureIterator::getFeatureAttribute( int idx, QgsPostgresResult
     return;
 
   const QgsField fld = mSource->mFields.at( idx );
-  QVariant v = QgsPostgresProvider::convertValue( fld.type(), fld.subType(), queryResult.PQgetvalue( row, col ), fld.typeName() );
+
+  QVariant v;
+
+  switch ( fld.type() )
+  {
+    case QVariant::ByteArray:
+    {
+      //special handling for binary field values
+      if ( ::PQgetisnull( queryResult.result(), row, col ) )
+      {
+        v = QVariant( QVariant::ByteArray );
+      }
+      else
+      {
+        size_t returnedLength = 0;
+        const char *value = ::PQgetvalue( queryResult.result(), row, col );
+        unsigned char *data = ::PQunescapeBytea( reinterpret_cast<const unsigned char *>( value ), &returnedLength );
+        if ( returnedLength == 0 )
+        {
+          v = QVariant( QVariant::ByteArray );
+        }
+        else
+        {
+          v = QByteArray( reinterpret_cast<const char *>( data ), int( returnedLength ) );
+        }
+        ::PQfreemem( data );
+      }
+      break;
+    }
+    default:
+    {
+      v = QgsPostgresProvider::convertValue( fld.type(), fld.subType(), queryResult.PQgetvalue( row, col ), fld.typeName() );
+      break;
+    }
+  }
   feature.setAttribute( idx, v );
 
   col++;

diff --git a/src/providers/postgres/qgspostgresprovider.cpp b/src/providers/postgres/qgspostgresprovider.cpp
@@ -241,6 +241,9 @@ QgsPostgresProvider::QgsPostgresProvider( QString const &uri, const ProviderOpti
 
       // boolean
       << QgsVectorDataProvider::NativeType( tr( "Boolean" ), QStringLiteral( "bool" ), QVariant::Bool, -1, -1, -1, -1 )
+
+      // binary (bytea)
+      << QgsVectorDataProvider::NativeType( tr( "Binary object (bytea)" ), QStringLiteral( "bytea" ), QVariant::ByteArray, -1, -1, -1, -1 )
       ;
 
   if ( connectionRO()->pgVersion() >= 90200 )
@@ -371,6 +374,22 @@ void QgsPostgresProvider::disconnectDb()
   }
 }
 
+QString QgsPostgresProvider::quotedByteaValue( const QVariant &value )
+{
+  if ( value.isNull() )
+    return QStringLiteral( "NULL" );
+
+  const QByteArray ba = value.toByteArray();
+  const unsigned char *buf = reinterpret_cast< const unsigned char * >( ba.constData() );
+  QString param;
+  param.reserve( ba.length() * 4 );
+  for ( int i = 0; i < ba.length(); ++i )
+  {
+    param += QStringLiteral( "\\%1" ).arg( static_cast< int >( buf[i] ), 3, 8, QChar( '0' ) );
+  }
+  return QStringLiteral( "decode('%1','escape')" ).arg( param );
+}
+
 QString QgsPostgresProvider::storageType() const
 {
   return QStringLiteral( "PostgreSQL database with PostGIS extension" );
@@ -950,6 +969,11 @@ bool QgsPostgresProvider::loadFields()
         fieldType = QVariant::DateTime;
         fieldSize = -1;
       }
+      else if ( fieldTypeName == QLatin1String( "bytea" ) )
+      {
+        fieldType = QVariant::ByteArray;
+        fieldSize = -1;
+      }
       else if ( fieldTypeName == QLatin1String( "text" ) ||
                 fieldTypeName == QLatin1String( "geometry" ) ||
                 fieldTypeName == QLatin1String( "inet" ) ||
@@ -2188,11 +2212,15 @@ bool QgsPostgresProvider::addFeatures( QgsFeatureList &flist, Flags flags )
         }
         else if ( fieldTypeName == QLatin1String( "jsonb" ) )
         {
-          values += delim + quotedJsonValue( v ) + QLatin1String( "::jsonb" );
+          values += delim + quotedJsonValue( v ) + QStringLiteral( "::jsonb" );
         }
         else if ( fieldTypeName == QLatin1String( "json" ) )
         {
-          values += delim + quotedJsonValue( v ) + QLatin1String( "::json" );
+          values += delim + quotedJsonValue( v ) + QStringLiteral( "::json" );
+        }
+        else if ( fieldTypeName == QLatin1String( "bytea" ) )
+        {
+          values += delim + quotedByteaValue( v );
         }
         //TODO: convert arrays and hstore to native types
         else
@@ -2752,6 +2780,10 @@ bool QgsPostgresProvider::changeAttributeValues( const QgsChangedAttributesMap &
             sql += QStringLiteral( "%1::json" )
                    .arg( quotedJsonValue( siter.value() ) );
           }
+          else if ( fld.typeName() == QLatin1String( "bytea" ) )
+          {
+            sql += quotedByteaValue( siter.value() );
+          }
           else
           {
             sql += quotedValue( *siter );
@@ -3082,6 +3114,10 @@ bool QgsPostgresProvider::changeFeatures( const QgsChangedAttributesMap &attr_ma
             sql += QStringLiteral( "st_geographyfromewkt(%1)" )
                    .arg( quotedValue( siter->toString() ) );
           }
+          else if ( fld.typeName() == QLatin1String( "bytea" ) )
+          {
+            sql += quotedByteaValue( siter.value() );
+          }
           else
           {
             sql += quotedValue( *siter );

diff --git a/src/providers/postgres/qgspostgresprovider.h b/src/providers/postgres/qgspostgresprovider.h
@@ -451,6 +451,7 @@ class QgsPostgresProvider : public QgsVectorDataProvider
     static QString quotedIdentifier( const QString &ident ) { return QgsPostgresConn::quotedIdentifier( ident ); }
     static QString quotedValue( const QVariant &value ) { return QgsPostgresConn::quotedValue( value ); }
     static QString quotedJsonValue( const QVariant &value ) { return QgsPostgresConn::quotedJsonValue( value ); }
+    static QString quotedByteaValue( const QVariant &value );
 
     friend class QgsPostgresFeatureSource;
 

diff --git a/tests/src/python/test_provider_postgres.py b/tests/src/python/test_provider_postgres.py
@@ -45,7 +45,7 @@
     QgsGeometry
 )
 from qgis.gui import QgsGui, QgsAttributeForm
-from qgis.PyQt.QtCore import QDate, QTime, QDateTime, QVariant, QDir, QObject
+from qgis.PyQt.QtCore import QDate, QTime, QDateTime, QVariant, QDir, QObject, QByteArray
 from qgis.PyQt.QtWidgets import QLabel
 from qgis.testing import start_app, unittest
 from qgis.PyQt.QtXml import QDomDocument
@@ -167,6 +167,64 @@ def testBooleanType(self):
         }
         self.assertEqual(values, expected)
 
+    def testByteaType(self):
+        vl = QgsVectorLayer('{} table="qgis_test"."byte_a_table" sql='.format(self.dbconn), "testbytea", "postgres")
+        self.assertTrue(vl.isValid())
+
+        fields = vl.dataProvider().fields()
+        self.assertEqual(fields.at(fields.indexFromName('fld1')).type(), QVariant.ByteArray)
+
+        values = {feat['id']: feat['fld1'] for feat in vl.getFeatures()}
+        expected = {
+            1: QByteArray(b'YmludmFsdWU='),
+            2: QByteArray()
+        }
+        self.assertEqual(values, expected)
+
+        # editing binary values
+        self.execSQLCommand('DROP TABLE IF EXISTS qgis_test."byte_a_table_edit" CASCADE')
+        self.execSQLCommand(
+            'CREATE TABLE qgis_test."byte_a_table_edit" ( pk SERIAL NOT NULL PRIMARY KEY, blobby bytea)')
+        self.execSQLCommand("INSERT INTO qgis_test.\"byte_a_table_edit\" (pk, blobby) VALUES "
+                            "(1, encode('bbb', 'base64')::bytea)")
+        vl = QgsVectorLayer(
+            self.dbconn + ' sslmode=disable table="qgis_test"."byte_a_table_edit" sql=',
+            'test', 'postgres')
+        self.assertTrue(vl.isValid())
+        values = {feat['pk']: feat['blobby'] for feat in vl.getFeatures()}
+        expected = {
+            1: QByteArray(b'YmJi')
+        }
+        self.assertEqual(values, expected)
+
+        # change attribute value
+        self.assertTrue(vl.dataProvider().changeAttributeValues({1: {1: QByteArray(b'bbbvx')}}))
+        values = {feat['pk']: feat['blobby'] for feat in vl.getFeatures()}
+        expected = {
+            1: QByteArray(b'bbbvx')
+        }
+        self.assertEqual(values, expected)
+
+        # add feature
+        f = QgsFeature()
+        f.setAttributes([2, QByteArray(b'cccc')])
+        self.assertTrue(vl.dataProvider().addFeature(f))
+        values = {feat['pk']: feat['blobby'] for feat in vl.getFeatures()}
+        expected = {
+            1: QByteArray(b'bbbvx'),
+            2: QByteArray(b'cccc')
+        }
+        self.assertEqual(values, expected)
+
+        # change feature
+        self.assertTrue(vl.dataProvider().changeFeatures({2: {1: QByteArray(b'dddd')}}, {}))
+        values = {feat['pk']: feat['blobby'] for feat in vl.getFeatures()}
+        expected = {
+            1: QByteArray(b'bbbvx'),
+            2: QByteArray(b'dddd')
+        }
+        self.assertEqual(values, expected)
+
     def testQueryLayers(self):
         def test_query(dbconn, query, key):
             ql = QgsVectorLayer('%s srid=4326 table="%s" (geom) key=\'%s\' sql=' % (dbconn, query.replace('"', '\\"'), key), "testgeom", "postgres")

diff --git a/tests/testdata/provider/testdata_pg.sql b/tests/testdata/provider/testdata_pg.sql
@@ -515,6 +515,22 @@ INSERT INTO qgis_test.boolean_table VALUES
 (2, FALSE),
 (3, NULL);
 
+
+--------------------------------------
+-- Table for bytea
+--
+
+CREATE TABLE qgis_test.byte_a_table
+(
+  id int PRIMARY KEY,
+  fld1 bytea
+);
+
+INSERT INTO qgis_test.byte_a_table VALUES
+(1, encode('binvalue', 'base64')::bytea),
+(2, NULL);
+
+
 -----------------------------
 -- Table for constraint tests
 --