Fix PG raster views and filter setting

- adds a warning if the filter could not be set because resulted in 0 rows and metadata could not be retrieved - fix views loading Fixes #50841
qgis · Nov 11, 2022 · 6b891c7 · 6b891c7
1 parent 5c6c63d
commit 6b891c7
Show file tree

Hide file tree

Showing 4 changed files with 129 additions and 46 deletions.
diff --git a/src/app/qgisapp.cpp b/src/app/qgisapp.cpp
@@ -11363,6 +11363,11 @@ void QgisApp::layerSubsetString( QgsMapLayer *mapLayer )
                 mLayerTreeView->refreshLayerSymbology( rlayer->id() );
                 activateDeactivateLayerRelatedActions( rlayer );
               }
+              else
+              {
+                QMessageBox::warning( this, tr( "Error Setting Filter" ),
+                                      tr( "The filtered layer returned no rows. The PostgreSQL raster provider requires at least one row in order to extract the information required to create a valid layer." ) );
+              }
             }
           }
         }

diff --git a/src/providers/postgres/raster/qgspostgresrasterprovider.cpp b/src/providers/postgres/raster/qgspostgresrasterprovider.cpp
@@ -1000,6 +1000,8 @@ bool QgsPostgresRasterProvider::init()
 
   // WARNING: multiple failure and return points!
 
+  mOverViews.clear();
+
   if ( !determinePrimaryKey() )
   {
     QgsMessageLog::logMessage( tr( "PostgreSQL raster layer has no primary key." ), tr( "PostGIS" ) );
@@ -1273,9 +1275,12 @@ bool QgsPostgresRasterProvider::init()
 
   // Get the full raster and extract information
   // Note: this can be very slow
-  // Use oveviews if we can, even if they are probably missing for unconstrained tables
-
-  findOverviews();
+  // Use oveviews if we can, even if they are probably missing for unconstrained tables.
+  // Overviews are useless if there is a filter.
+  if ( subsetString().isEmpty() )
+  {
+    findOverviews();
+  }
 
   QString tableToQuery { mQuery };
 
@@ -1340,7 +1345,7 @@ bool QgsPostgresRasterProvider::init()
 
     mExtent = p.boundingBox();
 
-    // Tile size (in this path the raster is considered untiled, so this is actually the whole size
+    // Tile size (in this path the raster is considered untiled, so this is actually the whole size)
     mTileWidth = result.PQgetvalue( 0, 3 ).toInt( &ok );
 
     if ( ! ok )
@@ -2058,6 +2063,11 @@ QgsPostgresProvider::Relkind QgsPostgresRasterProvider::relkind() const
 bool QgsPostgresRasterProvider::determinePrimaryKey()
 {
 
+  if ( !loadFields() )
+  {
+    return false;
+  }
+
   // check to see if there is an unique index on the relation, which
   // can be used as a key into the table. Primary keys are always
   // unique indices, so we catch them as well.
@@ -2110,7 +2120,9 @@ bool QgsPostgresRasterProvider::determinePrimaryKey()
             // Could warn the user here that performance will suffer if
             // attribute isn't indexed (and that they may want to add a
             // primary key to the table)
-            mPrimaryKeyAttrs << res.PQgetvalue( 0, 0 );
+            const QString keyName { res.PQgetvalue( 0, 0 ) };
+            Q_ASSERT( mAttributeFields.indexFromName( keyName ) >= 0 );
+            mPrimaryKeyAttrs << mAttributeFields.indexFromName( keyName );
           }
         }
 
@@ -2126,7 +2138,7 @@ bool QgsPostgresRasterProvider::determinePrimaryKey()
             // oid isn't indexed (and that they may want to add a
             // primary key to the table)
             mPrimaryKeyType = PktOid;
-            mPrimaryKeyAttrs << QStringLiteral( "oid" );
+            mPrimaryKeyAttrs.clear();
           }
         }
 
@@ -2140,7 +2152,7 @@ bool QgsPostgresRasterProvider::determinePrimaryKey()
             mPrimaryKeyType = PktTid;
             QgsMessageLog::logMessage( tr( "Primary key is ctid - changing of existing features disabled (%1; %2)" ).arg( mRasterColumn, mQuery ) );
             // TODO: set capabilities to RO when writing will be implemented
-            mPrimaryKeyAttrs << QStringLiteral( "ctid" );
+            mPrimaryKeyAttrs.clear();
           }
         }
 
@@ -2212,12 +2224,13 @@ bool QgsPostgresRasterProvider::determinePrimaryKey()
         }
         // Always use PktFidMap for multi-field keys
         mPrimaryKeyType = i ? QgsPostgresPrimaryKeyType::PktFidMap : pkType;
-        mPrimaryKeyAttrs << name;
+        Q_ASSERT( mAttributeFields.indexFromName( name ) >= 0 );
+        mPrimaryKeyAttrs << mAttributeFields.indexFromName( name );
       }
 
       if ( mightBeNull || isParentTable )
       {
-        QgsMessageLog::logMessage( tr( "Ignoring key candidate because of NULL values or inherited table" ), tr( "PostGIS" ) );
+        QgsMessageLog::logMessage( tr( "Ignoring key candidate because of NULL values or inherited table" ), tr( "PostGIS" ), Qgis::MessageLevel::Info );
         mPrimaryKeyType = PktUnknown;
         mPrimaryKeyAttrs.clear();
       }
@@ -2228,60 +2241,97 @@ bool QgsPostgresRasterProvider::determinePrimaryKey()
     determinePrimaryKeyFromUriKeyColumn();
   }
 
-  if ( mPrimaryKeyAttrs.size() == 0 )
-  {
-    QgsMessageLog::logMessage( tr( "Could not find a primary key for PostGIS raster table %1" ).arg( mQuery ), tr( "PostGIS" ) );
-    mPrimaryKeyType = PktUnknown;
-  }
-
   return mPrimaryKeyType != PktUnknown;
 }
 
-
-
 void QgsPostgresRasterProvider::determinePrimaryKeyFromUriKeyColumn()
 {
-  mPrimaryKeyAttrs.clear();
-  const QString keyCandidate {  mUri.keyColumn() };
-  QgsPostgresPrimaryKeyType pkType { QgsPostgresPrimaryKeyType::PktUnknown };
-  const QString sql = QStringLiteral( "SELECT data_type FROM information_schema.columns "
-                                      "WHERE column_name = %1 AND table_name = %2 AND table_schema = %3" )
-                      .arg( keyCandidate, mTableName,  mSchemaName );
-  QgsPostgresResult result( connectionRO()->PQexec( sql ) );
-  if ( PGRES_TUPLES_OK == result.PQresultStatus() )
+  QString primaryKey = mUri.keyColumn();
+  mPrimaryKeyType = PktUnknown;
+
+  if ( !primaryKey.isEmpty() )
   {
-    const QString fieldTypeName { result.PQgetvalue( 0, 0 ) };
+    const QStringList cols = parseUriKey( primaryKey );
 
-    if ( fieldTypeName == QLatin1String( "oid" ) )
+    primaryKey.clear();
+    QString del;
+    for ( const QString &col : cols )
     {
-      pkType = QgsPostgresPrimaryKeyType::PktOid;
+      primaryKey += del + quotedIdentifier( col );
+      del = QStringLiteral( "," );
     }
-    else if ( fieldTypeName == QLatin1String( "integer" ) )
+
+    for ( const QString &col : cols )
     {
-      pkType = QgsPostgresPrimaryKeyType::PktInt;
+      int idx = fields().lookupField( col );
+      if ( idx < 0 )
+      {
+        QgsMessageLog::logMessage( tr( "Key field '%1' for view/query not found." ).arg( col ), tr( "PostGIS" ) );
+        mPrimaryKeyAttrs.clear();
+        break;
+      }
+
+      mPrimaryKeyAttrs << idx;
     }
-    else if ( fieldTypeName == QLatin1String( "bigint" ) )
+
+    if ( !mPrimaryKeyAttrs.isEmpty() )
     {
-      pkType = QgsPostgresPrimaryKeyType::PktUint64;
+
+      if ( mUseEstimatedMetadata )
+      {
+        mPrimaryKeyType = PktFidMap; // Map by default
+        if ( mPrimaryKeyAttrs.size() == 1 )
+        {
+          QgsField fld = mAttributeFields.at( mPrimaryKeyAttrs.at( 0 ) );
+          mPrimaryKeyType = pkType( fld );
+        }
+      }
+      else
+      {
+        QgsMessageLog::logMessage( tr( "Primary key field '%1' for view/query not unique." ).arg( primaryKey ), tr( "PostGIS" ) );
+      }
+    }
+    else
+    {
+      QgsMessageLog::logMessage( tr( "Keys for view/query undefined." ), tr( "PostGIS" ) );
     }
-    mPrimaryKeyAttrs.push_back( mUri.keyColumn() );
-    mPrimaryKeyType = pkType;
+  }
+  else
+  {
+    QgsMessageLog::logMessage( tr( "No key field for view/query given." ), tr( "PostGIS" ) );
   }
 }
 
+
 QString QgsPostgresRasterProvider::pkSql()
 {
-  Q_ASSERT_X( ! mPrimaryKeyAttrs.isEmpty(), "QgsPostgresRasterProvider::pkSql()",  "No PK is defined!" );
-  if ( mPrimaryKeyAttrs.count( ) > 1 )
+  switch ( mPrimaryKeyType )
   {
-    QStringList pkeys;
-    for ( const QString &k : std::as_const( mPrimaryKeyAttrs ) )
+    case QgsPostgresPrimaryKeyType::PktOid:
+      return QStringLiteral( "oid" );
+    case QgsPostgresPrimaryKeyType::PktTid:
+      return QStringLiteral( "ctid" );
+    default:
     {
-      pkeys.push_back( quotedIdentifier( k ) );
+      if ( mPrimaryKeyAttrs.count( ) > 1 )
+      {
+        QStringList pkeys;
+        for ( const int &keyIndex : std::as_const( mPrimaryKeyAttrs ) )
+        {
+          if ( mAttributeFields.exists( keyIndex ) )
+          {
+            pkeys.push_back( quotedIdentifier( mAttributeFields.at( keyIndex ).name() ) );
+          }
+          else
+          {
+            QgsDebugMsg( QStringLiteral( "Attribute not found %1" ).arg( keyIndex ) );
+          }
+        }
+        return pkeys.join( ',' ).prepend( '(' ).append( ')' );
+      }
+      return mAttributeFields.exists( mPrimaryKeyAttrs.first() ) ? quotedIdentifier( mAttributeFields.at( mPrimaryKeyAttrs.first() ).name() ) : QString();
     }
-    return pkeys.join( ',' ).prepend( '(' ).append( ')' );
   }
-  return quotedIdentifier( mPrimaryKeyAttrs.first() );
 }
 
 QString QgsPostgresRasterProvider::dataComment() const
@@ -2338,6 +2388,26 @@ int QgsPostgresRasterProvider::ySize() const
   return static_cast<int>( mHeight );
 }
 
+QgsPostgresPrimaryKeyType QgsPostgresRasterProvider::pkType( const QgsField &fld )
+{
+  switch ( fld.type() )
+  {
+    case QVariant::LongLong:
+      // PostgreSQL doesn't have native "unsigned" types.
+      // Unsigned primary keys are emulated by the serial/bigserial
+      // pseudo-types, in which autogenerated values are always > 0;
+      // however, the database accepts manually inserted 0 and negative values
+      // in these fields.
+      return PktInt64;
+
+    case QVariant::Int:
+      return PktInt;
+
+    default:
+      return PktFidMap;
+  }
+}
+
 Qgis::DataType QgsPostgresRasterProvider::sourceDataType( int bandNo ) const
 {
   if ( bandNo <= mBandCount && static_cast<unsigned long>( bandNo ) <= mDataTypes.size() )

diff --git a/src/providers/postgres/raster/qgspostgresrasterprovider.h b/src/providers/postgres/raster/qgspostgresrasterprovider.h
@@ -77,6 +77,18 @@ class QgsPostgresRasterProvider : public QgsRasterDataProvider
     static const QString PG_RASTER_PROVIDER_KEY;
     static const QString PG_RASTER_PROVIDER_DESCRIPTION;
 
+    /**
+     * Returns the type of primary key for a PK field
+     *
+     * \param fld the field to determine PK type of
+     * \returns the PrimaryKeyType
+     *
+     * \note that this only makes sense for single-field primary keys,
+     *       whereas multi-field keys always need the PktFidMap
+     *       primary key type.
+     */
+    static QgsPostgresPrimaryKeyType pkType( const QgsField &fld );
+
   private:
 
     bool mValid = false;
@@ -156,7 +168,7 @@ class QgsPostgresRasterProvider : public QgsRasterDataProvider
     /**
      * List of primary key attributes for fetching features.
      */
-    QList<QString> mPrimaryKeyAttrs;
+    QList<int> mPrimaryKeyAttrs;
 
     //! Mutable data shared between provider and feature sources
     std::shared_ptr<QgsPostgresRasterSharedData> mShared;

diff --git a/tests/src/python/test_provider_postgresraster.py b/tests/src/python/test_provider_postgresraster.py
@@ -109,10 +109,6 @@ def gdal_block_compare(self, rlayer, band, extent, width, height, value):
         self.assertEqual(value, gdal_rl.dataProvider().block(
             band, self.rl.extent(), 6, 5).data().toHex())
 
-    @classmethod
-    def tearDownClass(cls):
-        """Run after all tests"""
-
     def testExtent(self):
         extent = self.rl.extent()
         self.assertEqual(extent, QgsRectangle(