Merge pull request #45127 from m-kuhn/refGeomPg

[postgis] Expose secondary gometry columns as referenced geometries
qgis · Nov 24, 2021 · be17000 · be17000
2 parents c42e74c + 6d5d30b
commit be17000
Show file tree

Hide file tree

Showing 9 changed files with 246 additions and 84 deletions.
diff --git a/.github/workflows/macos-build.yml b/.github/workflows/macos-build.yml
@@ -58,10 +58,10 @@ jobs:
       # Qt caching
       - name: Cache Qt
         id: cache-qt
-        uses: pat-s/always-upload-cache@v2.1.5
+        uses: actions/cache@v2.1.6
         with:
           path: ${{ env.DEPS_CACHE_DIR }}/Qt/${{ env.QT_VERSION }}
-          key: mac-qt-v4-${{ env.QT_VERSION }}
+          key: mac-qt-${{ env.QT_VERSION }}
 
       - name: Restore Qt
         if: steps.cache-qt.outputs.cache-hit == 'true'
@@ -81,10 +81,10 @@ jobs:
       # QGIS-deps caching
       - name: Cache qgis-deps
         id: cache-deps
-        uses: pat-s/always-upload-cache@v2.1.5
+        uses: actions/cache@v2.1.6
         with:
           path: ${{ env.DEPS_CACHE_DIR }}/QGIS/qgis-deps-${{ env.QGIS_DEPS_VERSION }}.${{ env.QGIS_DEPS_PATCH_VERSION }}
-          key: mac-qgis-deps-v4-${{ env.QGIS_DEPS_VERSION }}.${{ env.QGIS_DEPS_PATCH_VERSION }}
+          key: mac-qgis-deps-${{ env.QGIS_DEPS_VERSION }}.${{ env.QGIS_DEPS_PATCH_VERSION }}
 
       - name: Restore qgis-deps
         if: steps.cache-deps.outputs.cache-hit == 'true'

diff --git a/doc/api_break.dox b/doc/api_break.dox
@@ -10,11 +10,23 @@ remove them nor change their semantics. Existing code should keep working when t
 to another minor version (e.g. from 2.0 to 2.2), so all extensions of existing classes should be done in a manner that
 third party developers do not need to adjust their code to work properly with newer QGIS releases.
 
-Sometimes, however, we may need to break the API as a result of some code changes. These cases should be only exceptions
-and they should happen only after consideration and agreement of the development team. Backwards incompatible changes
-with too big impact should be deferred to a major version release.
+Sometimes, however, we need to break the API as a result of code changes. These cases are exceptions
+and they happen only after consideration and agreement of the development team.
+Backwards incompatible changes with large impact are postponed to the next major release and tracked in
+https://github.com/qgis/qgis4.0_api/issues
+
+This page maintains a list of incompatible changes that happened in previous releases.
+
+QGIS 3.24        {#qgis_api_break_3_24}
+=========
+
+Additional geometry attributes
+------------------------------
+
+- If a postgis layer has more than one geometry, the additional geometry attributes are exposed as
+QgsReferencedGeometry. Previously, they were exposed as EWKT strings. EWKT strings are still supported
+for inserting and updating features.
 
-This page tries to maintain a list with incompatible changes that happened in previous releases.
 
 QGIS 3.22        {#qgis_api_break_3_22}
 =========

diff --git a/src/core/qgsfield.cpp b/src/core/qgsfield.cpp
@@ -19,6 +19,7 @@
 #include "qgis.h"
 #include "qgsapplication.h"
 #include "qgssettings.h"
+#include "qgsreferencedgeometry.h"
 
 #include <QDataStream>
 #include <QIcon>
@@ -258,6 +259,23 @@ QString QgsField::displayString( const QVariant &v ) const
     return QgsApplication::nullRepresentation();
   }
 
+  if ( v.userType() == QMetaType::type( "QgsReferencedGeometry" ) )
+  {
+    QgsReferencedGeometry geom = qvariant_cast<QgsReferencedGeometry>( v );
+    if ( geom.isNull() )
+      return QgsApplication::nullRepresentation();
+    else
+    {
+      QString wkt = geom.asWkt();
+      if ( wkt.length() >= 1050 )
+      {
+        wkt = wkt.left( 999 ) + QChar( 0x2026 );
+      }
+      QString formattedText = QStringLiteral( "%1 [%2]" ).arg( wkt, geom.crs().userFriendlyIdentifier() );
+      return formattedText;
+    }
+  }
+
   // Special treatment for numeric types if group separator is set or decimalPoint is not a dot
   if ( d->type == QVariant::Double )
   {

diff --git a/src/providers/postgres/qgspostgresfeatureiterator.cpp b/src/providers/postgres/qgspostgresfeatureiterator.cpp
@@ -875,7 +875,7 @@ bool QgsPostgresFeatureIterator::getFeature( QgsPostgresResult &queryResult, int
       int idx = mSource->mPrimaryKeyAttrs.at( 0 );
       QgsField fld = mSource->mFields.at( idx );
 
-      QVariant v = QgsPostgresProvider::convertValue( fld.type(), fld.subType(), QString::number( mConn->getBinaryInt( queryResult, row, col ) ), fld.typeName() );
+      QVariant v = QgsPostgresProvider::convertValue( fld.type(), fld.subType(), QString::number( mConn->getBinaryInt( queryResult, row, col ) ), fld.typeName(), mConn );
       pkVal << v;
 
       if ( !subsetOfAttributes || fetchAttributes.contains( idx ) )
@@ -900,11 +900,11 @@ bool QgsPostgresFeatureIterator::getFeature( QgsPostgresResult &queryResult, int
 
         if ( fld.type() == QVariant::LongLong )
         {
-          v = QgsPostgresProvider::convertValue( fld.type(), fld.subType(), QString::number( mConn->getBinaryInt( queryResult, row, col ) ), fld.typeName() );
+          v = QgsPostgresProvider::convertValue( fld.type(), fld.subType(), QString::number( mConn->getBinaryInt( queryResult, row, col ) ), fld.typeName(), mConn );
         }
         else
         {
-          v = QgsPostgresProvider::convertValue( fld.type(), fld.subType(), queryResult.PQgetvalue( row, col ), fld.typeName() );
+          v = QgsPostgresProvider::convertValue( fld.type(), fld.subType(), queryResult.PQgetvalue( row, col ), fld.typeName(), mConn );
         }
         primaryKeyVals << v;
 
@@ -986,13 +986,13 @@ void QgsPostgresFeatureIterator::getFeatureAttribute( int idx, QgsPostgresResult
       }
       else
       {
-        v = QgsPostgresProvider::convertValue( fld.type(), fld.subType(), QString::number( mConn->getBinaryInt( queryResult, row, col ) ), fld.typeName() );
+        v = QgsPostgresProvider::convertValue( fld.type(), fld.subType(), QString::number( mConn->getBinaryInt( queryResult, row, col ) ), fld.typeName(), mConn );
       }
       break;
     }
     default:
     {
-      v = QgsPostgresProvider::convertValue( fld.type(), fld.subType(), queryResult.PQgetvalue( row, col ), fld.typeName() );
+      v = QgsPostgresProvider::convertValue( fld.type(), fld.subType(), queryResult.PQgetvalue( row, col ), fld.typeName(), mConn );
       break;
     }
   }

diff --git a/src/providers/postgres/qgspostgresprovider.cpp b/src/providers/postgres/qgspostgresprovider.cpp
@@ -354,6 +354,106 @@ void QgsPostgresProvider::setTransaction( QgsTransaction *transaction )
   mTransaction = static_cast<QgsPostgresTransaction *>( transaction );
 }
 
+QgsReferencedGeometry QgsPostgresProvider::fromEwkt( const QString &ewkt, QgsPostgresConn *conn )
+{
+  thread_local const QRegularExpression regularExpressionSRID( "^SRID=(\\d+);" );
+
+  QRegularExpressionMatch regularExpressionMatch = regularExpressionSRID.match( ewkt );
+  if ( !regularExpressionMatch.hasMatch() )
+    return QgsReferencedGeometry();
+
+  QString wkt = ewkt.mid( regularExpressionMatch.captured( 0 ).size() );
+  int srid = regularExpressionMatch.captured( 1 ).toInt();
+
+
+  QgsGeometry geom = QgsGeometry::fromWkt( wkt );
+  return QgsReferencedGeometry( geom, sridToCrs( srid, conn ) );
+}
+
+QString QgsPostgresProvider::toEwkt( const QgsReferencedGeometry &geom, QgsPostgresConn *conn )
+{
+  if ( !geom.isNull() )
+    return QStringLiteral( "SRID=%1;%2" ).arg( QString::number( crsToSrid( geom.crs(), conn ) ), geom.asWkt() );
+  else
+    return QString();
+}
+
+QString QgsPostgresProvider::geomAttrToString( const QVariant &attr, QgsPostgresConn *conn )
+{
+  if ( attr.type() == QVariant::String )
+    return attr.toString();
+  else
+    return toEwkt( attr.value<QgsReferencedGeometry>(), conn );
+}
+
+static QMutex sMutex;
+static QMap<int, QgsCoordinateReferenceSystem> sCrsCache;
+
+int QgsPostgresProvider::crsToSrid( const QgsCoordinateReferenceSystem &crs, QgsPostgresConn *conn )
+{
+  QMutexLocker locker( &sMutex );
+  int srid = sCrsCache.key( crs );
+
+  if ( srid > -1 )
+    return srid;
+  else
+  {
+    if ( conn )
+    {
+      QStringList authParts = crs.authid().split( ':' );
+      if ( authParts.size() != 2 )
+        return -1;
+      const QString authName = authParts.first();
+      const QString authId = authParts.last();
+      QgsPostgresResult result( conn->PQexec( QStringLiteral( "SELECT srid FROM spatial_ref_sys WHERE auth_name='%1' AND auth_srid=%2" ).arg( authName, authId ) ) );
+
+      if ( result.PQresultStatus() == PGRES_TUPLES_OK )
+      {
+        int srid = result.PQgetvalue( 0, 0 ).toInt();
+        sCrsCache.insert( srid, crs );
+        return srid;
+      }
+    }
+  }
+
+  return -1;
+}
+
+QgsCoordinateReferenceSystem QgsPostgresProvider::sridToCrs( int srid, QgsPostgresConn *conn )
+{
+  QgsCoordinateReferenceSystem crs;
+
+  QMutexLocker locker( &sMutex );
+  if ( sCrsCache.contains( srid ) )
+    crs = sCrsCache.value( srid );
+  else
+  {
+    if ( conn )
+    {
+      QgsPostgresResult result( conn->PQexec( QStringLiteral( "SELECT auth_name, auth_srid, srtext, proj4text FROM spatial_ref_sys WHERE srid=%1" ).arg( srid ) ) );
+      if ( result.PQresultStatus() == PGRES_TUPLES_OK )
+      {
+        const QString authName = result.PQgetvalue( 0, 0 );
+        const QString authSRID = result.PQgetvalue( 0, 1 );
+        const QString srText = result.PQgetvalue( 0, 2 );
+        bool ok = false;
+        if ( authName == QLatin1String( "EPSG" ) || authName == QLatin1String( "ESRI" ) )
+        {
+          ok = crs.createFromUserInput( authName + ':' + authSRID );
+        }
+        if ( !ok && !srText.isEmpty() )
+        {
+          ok = crs.createFromUserInput( srText );
+        }
+        if ( !ok )
+          crs = QgsCoordinateReferenceSystem::fromProj( result.PQgetvalue( 0, 3 ) );
+        sCrsCache.insert( srid, crs );
+      }
+    }
+  }
+  return crs;
+}
+
 void QgsPostgresProvider::disconnectDb()
 {
   if ( mConnectionRO )
@@ -1078,7 +1178,6 @@ bool QgsPostgresProvider::loadFields()
       }
       else if ( fieldTypeName == QLatin1String( "text" ) ||
                 fieldTypeName == QLatin1String( "citext" ) ||
-                fieldTypeName == QLatin1String( "geometry" ) ||
                 fieldTypeName == QLatin1String( "geography" ) ||
                 fieldTypeName == QLatin1String( "inet" ) ||
                 fieldTypeName == QLatin1String( "cidr" ) ||
@@ -1094,6 +1193,11 @@ bool QgsPostgresProvider::loadFields()
         fieldType = QVariant::String;
         fieldSize = -1;
       }
+      else if ( fieldTypeName == QLatin1String( "geometry" ) )
+      {
+        fieldType = QVariant::UserType;
+        fieldSize = -1;
+      }
       else if ( fieldTypeName == QLatin1String( "bpchar" ) )
       {
         // although postgres internally uses "bpchar", this is exposed to users as character in postgres
@@ -2448,10 +2552,11 @@ bool QgsPostgresProvider::addFeatures( QgsFeatureList &flist, Flags flags )
         }
         else if ( fieldTypeName == QLatin1String( "geometry" ) )
         {
+          QString val = geomAttrToString( v, connectionRO() );
           values += QStringLiteral( "%1%2(%3)" )
                     .arg( delim,
                           connectionRO()->majorVersion() < 2 ? "geomfromewkt" : "st_geomfromewkt",
-                          quotedValue( v.toString() ) );
+                          quotedValue( val ) );
         }
         else if ( fieldTypeName == QLatin1String( "geography" ) )
         {
@@ -3054,9 +3159,11 @@ bool QgsPostgresProvider::changeAttributeValues( const QgsChangedAttributesMap &
           }
           else if ( fld.typeName() == QLatin1String( "geometry" ) )
           {
+            QString val = geomAttrToString( siter.value(), connectionRO() );
+
             sql += QStringLiteral( "%1(%2)" )
                    .arg( connectionRO()->majorVersion() < 2 ? "geomfromewkt" : "st_geomfromewkt",
-                         quotedValue( siter->toString() ) );
+                         quotedValue( val ) );
           }
           else if ( fld.typeName() == QLatin1String( "geography" ) )
           {
@@ -3418,9 +3525,10 @@ bool QgsPostgresProvider::changeFeatures( const QgsChangedAttributesMap &attr_ma
 
           if ( fld.typeName() == QLatin1String( "geometry" ) )
           {
+            QString val = geomAttrToString( siter.value(), connectionRO() ) ;
             sql += QStringLiteral( "%1(%2)" )
                    .arg( connectionRO()->majorVersion() < 2 ? "geomfromewkt" : "st_geomfromewkt",
-                         quotedValue( siter->toString() ) );
+                         quotedValue( val ) );
           }
           else if ( fld.typeName() == QLatin1String( "geography" ) )
           {
@@ -4693,40 +4801,8 @@ QgsCoordinateReferenceSystem QgsPostgresProvider::crs() const
   QgsCoordinateReferenceSystem srs;
   int srid = mRequestedSrid.isEmpty() ? mDetectedSrid.toInt() : mRequestedSrid.toInt();
 
-  {
-    static QMutex sMutex;
-    QMutexLocker locker( &sMutex );
-    static QMap<int, QgsCoordinateReferenceSystem> sCrsCache;
-    if ( sCrsCache.contains( srid ) )
-      srs = sCrsCache.value( srid );
-    else
-    {
-      QgsPostgresConn *conn = connectionRO();
-      if ( conn )
-      {
-        QgsPostgresResult result( conn->PQexec( QStringLiteral( "SELECT auth_name, auth_srid, srtext, proj4text FROM spatial_ref_sys WHERE srid=%1" ).arg( srid ) ) );
-        if ( result.PQresultStatus() == PGRES_TUPLES_OK )
-        {
-          const QString authName = result.PQgetvalue( 0, 0 );
-          const QString authSRID = result.PQgetvalue( 0, 1 );
-          const QString srText = result.PQgetvalue( 0, 2 );
-          bool ok = false;
-          if ( authName == QLatin1String( "EPSG" ) || authName == QLatin1String( "ESRI" ) )
-          {
-            ok = srs.createFromUserInput( authName + ':' + authSRID );
-          }
-          if ( !ok && !srText.isEmpty() )
-          {
-            ok = srs.createFromUserInput( srText );
-          }
-          if ( !ok )
-            srs = QgsCoordinateReferenceSystem::fromProj( result.PQgetvalue( 0, 3 ) );
-          sCrsCache.insert( srid, srs );
-        }
-      }
-    }
-  }
-  return srs;
+  return sridToCrs( srid, connectionRO() );
+
 }
 
 QString QgsPostgresProvider::subsetString() const
@@ -4847,7 +4923,7 @@ QVariant QgsPostgresProvider::parseJson( const QString &txt )
   return QgsJsonUtils::parseJson( txt );
 }
 
-QVariant QgsPostgresProvider::parseOtherArray( const QString &txt, QVariant::Type subType, const QString &typeName )
+QVariant QgsPostgresProvider::parseOtherArray( const QString &txt, QVariant::Type subType, const QString &typeName, QgsPostgresConn *conn )
 {
   int i = 0;
   QVariantList result;
@@ -4859,7 +4935,7 @@ QVariant QgsPostgresProvider::parseOtherArray( const QString &txt, QVariant::Typ
       QgsMessageLog::logMessage( tr( "Error parsing array: %1" ).arg( txt ), tr( "PostGIS" ) );
       break;
     }
-    result.append( QgsPostgresProvider::convertValue( subType, QVariant::Invalid, value, typeName ) );
+    result.append( convertValue( subType, QVariant::Invalid, value, typeName, conn ) );
   }
   return result;
 }
@@ -4919,7 +4995,7 @@ QVariant QgsPostgresProvider::parseMultidimensionalArray( const QString &txt )
 
 }
 
-QVariant QgsPostgresProvider::parseArray( const QString &txt, QVariant::Type type, QVariant::Type subType, const QString &typeName )
+QVariant QgsPostgresProvider::parseArray( const QString &txt, QVariant::Type type, QVariant::Type subType, const QString &typeName, QgsPostgresConn *conn )
 {
   if ( !txt.startsWith( '{' ) || !txt.endsWith( '}' ) )
   {
@@ -4933,10 +5009,15 @@ QVariant QgsPostgresProvider::parseArray( const QString &txt, QVariant::Type typ
   else if ( type == QVariant::StringList )
     return parseStringArray( inner );
   else
-    return parseOtherArray( inner, subType, typeName );
+    return parseOtherArray( inner, subType, typeName, conn );
+}
+
+QVariant QgsPostgresProvider::convertValue( QVariant::Type type, QVariant::Type subType, const QString &value, const QString &typeName ) const
+{
+  return convertValue( type, subType, value, typeName, connectionRO() );
 }
 
-QVariant QgsPostgresProvider::convertValue( QVariant::Type type, QVariant::Type subType, const QString &value, const QString &typeName )
+QVariant QgsPostgresProvider::convertValue( QVariant::Type type, QVariant::Type subType, const QString &value, const QString &typeName, QgsPostgresConn *conn )
 {
   QVariant result;
   switch ( type )
@@ -4949,7 +5030,7 @@ QVariant QgsPostgresProvider::convertValue( QVariant::Type type, QVariant::Type
       break;
     case QVariant::StringList:
     case QVariant::List:
-      result = parseArray( value, type, subType, typeName );
+      result = parseArray( value, type, subType, typeName, conn );
       break;
     case QVariant::Bool:
       if ( value == QChar( 't' ) )
@@ -4959,6 +5040,10 @@ QVariant QgsPostgresProvider::convertValue( QVariant::Type type, QVariant::Type
       else
         result = QVariant( type );
       break;
+    case QVariant::UserType:
+      result = fromEwkt( value, conn );
+      break;
+
     default:
       result = value;
       if ( !result.convert( type ) || value.isNull() )