Merge pull request #54734 from elpaso/bugfix-gh54662-spatialite-multi…

…surface SPATIALITE: fix insert incompatible geometry types
qgis · Sep 30, 2023 · 4934029 · 4934029
2 parents 373d5f8 + 7bd457b
commit 4934029
Show file tree

Hide file tree

Showing 6 changed files with 111 additions and 40 deletions.
diff --git a/python/core/auto_generated/vector/qgsvectordataprovider.sip.in b/python/core/auto_generated/vector/qgsvectordataprovider.sip.in
@@ -704,6 +704,10 @@ For general debug information use :py:func:`QgsMessageLog.logMessage()` instead.
 Converts the geometry to the provider type if possible / necessary
 
 :return: the converted geometry or ``None`` if no conversion was necessary or possible
+
+.. note::
+
+   The default implementation simply calls the static version of this function.
 %End
 
     void setNativeTypes( const QList<QgsVectorDataProvider::NativeType> &nativeTypes );
@@ -721,6 +725,16 @@ Gets this providers encoding
 .. versionadded:: 3.0
 %End
 
+    static QgsGeometry convertToProviderType( const QgsGeometry &geometry,  Qgis::WkbType providerGeometryType );
+%Docstring
+Converts the ``geometry`` to the provider geometry type ``providerGeometryType`` if possible / necessary
+
+:return: the converted geometry or ``None`` if no conversion was necessary or possible
+
+.. versionadded:: 3.34
+%End
+
+
 };
 
 QFlags<QgsVectorDataProvider::Capability> operator|(QgsVectorDataProvider::Capability f1, QFlags<QgsVectorDataProvider::Capability> f2);

diff --git a/src/core/vector/qgsvectordataprovider.cpp b/src/core/vector/qgsvectordataprovider.cpp
@@ -898,31 +898,52 @@ QgsGeometry QgsVectorDataProvider::convertToProviderType( const QgsGeometry &geo
 {
   QGIS_PROTECT_QOBJECT_THREAD_ACCESS
 
-  if ( geom.isNull() )
+  // Call the static version
+  return QgsVectorDataProvider::convertToProviderType( geom, wkbType() );
+
+}
+
+void QgsVectorDataProvider::setNativeTypes( const QList<NativeType> &nativeTypes )
+{
+  QGIS_PROTECT_QOBJECT_THREAD_ACCESS
+
+  mNativeTypes = nativeTypes;
+}
+
+QTextCodec *QgsVectorDataProvider::textEncoding() const
+{
+  // non fatal for now -- the "rasterize" processing algorithm is not thread safe and calls this
+  QGIS_PROTECT_QOBJECT_THREAD_ACCESS_NON_FATAL
+
+  return mEncoding;
+}
+
+QgsGeometry QgsVectorDataProvider::convertToProviderType( const QgsGeometry &geometry, Qgis::WkbType providerGeometryType )
+{
+  if ( geometry.isNull() )
   {
     return QgsGeometry();
   }
 
-  const QgsAbstractGeometry *geometry = geom.constGet();
-  if ( !geometry )
+  const QgsAbstractGeometry *convertedGeometry = geometry.constGet();
+  if ( !convertedGeometry )
   {
     return QgsGeometry();
   }
 
-  const Qgis::WkbType providerGeomType = wkbType();
 
   //geom is already in the provider geometry type
-  if ( geometry->wkbType() == providerGeomType )
+  if ( convertedGeometry->wkbType() == providerGeometryType )
   {
     return QgsGeometry();
   }
 
   std::unique_ptr< QgsAbstractGeometry > outputGeom;
 
   //convert compoundcurve to circularstring (possible if compoundcurve consists of one circular string)
-  if ( QgsWkbTypes::flatType( providerGeomType ) == Qgis::WkbType::CircularString )
+  if ( QgsWkbTypes::flatType( providerGeometryType ) == Qgis::WkbType::CircularString )
   {
-    QgsCompoundCurve *compoundCurve = qgsgeometry_cast<QgsCompoundCurve *>( geometry );
+    QgsCompoundCurve *compoundCurve = qgsgeometry_cast<QgsCompoundCurve *>( convertedGeometry );
     if ( compoundCurve )
     {
       if ( compoundCurve->nCurves() == 1 )
@@ -937,49 +958,49 @@ QgsGeometry QgsVectorDataProvider::convertToProviderType( const QgsGeometry &geo
   }
 
   //convert to curved type if necessary
-  if ( !QgsWkbTypes::isCurvedType( geometry->wkbType() ) && QgsWkbTypes::isCurvedType( providerGeomType ) )
+  if ( !QgsWkbTypes::isCurvedType( convertedGeometry->wkbType() ) && QgsWkbTypes::isCurvedType( providerGeometryType ) )
   {
-    QgsAbstractGeometry *curveGeom = outputGeom ? outputGeom->toCurveType() : geometry->toCurveType();
+    QgsAbstractGeometry *curveGeom = outputGeom ? outputGeom->toCurveType() : convertedGeometry->toCurveType();
     if ( curveGeom )
     {
       outputGeom.reset( curveGeom );
     }
   }
 
   //convert to linear type from curved type
-  if ( QgsWkbTypes::isCurvedType( geometry->wkbType() ) && !QgsWkbTypes::isCurvedType( providerGeomType ) )
+  if ( QgsWkbTypes::isCurvedType( convertedGeometry->wkbType() ) && !QgsWkbTypes::isCurvedType( providerGeometryType ) )
   {
-    QgsAbstractGeometry *segmentizedGeom = outputGeom ? outputGeom->segmentize() : geometry->segmentize();
+    QgsAbstractGeometry *segmentizedGeom = outputGeom ? outputGeom->segmentize() : convertedGeometry->segmentize();
     if ( segmentizedGeom )
     {
       outputGeom.reset( segmentizedGeom );
     }
   }
 
   //convert to multitype if necessary
-  if ( QgsWkbTypes::isMultiType( providerGeomType ) && !QgsWkbTypes::isMultiType( geometry->wkbType() ) )
+  if ( QgsWkbTypes::isMultiType( providerGeometryType ) && !QgsWkbTypes::isMultiType( convertedGeometry->wkbType() ) )
   {
-    std::unique_ptr< QgsAbstractGeometry > collGeom( QgsGeometryFactory::geomFromWkbType( providerGeomType ) );
+    std::unique_ptr< QgsAbstractGeometry > collGeom( QgsGeometryFactory::geomFromWkbType( providerGeometryType ) );
     QgsGeometryCollection *geomCollection = qgsgeometry_cast<QgsGeometryCollection *>( collGeom.get() );
     if ( geomCollection )
     {
-      if ( geomCollection->addGeometry( outputGeom ? outputGeom->clone() : geometry->clone() ) )
+      if ( geomCollection->addGeometry( outputGeom ? outputGeom->clone() : convertedGeometry->clone() ) )
       {
         outputGeom.reset( collGeom.release() );
       }
     }
   }
 
   //convert to single type if there's a single part of compatible type
-  if ( !QgsWkbTypes::isMultiType( providerGeomType ) && QgsWkbTypes::isMultiType( geometry->wkbType() ) )
+  if ( !QgsWkbTypes::isMultiType( providerGeometryType ) && QgsWkbTypes::isMultiType( convertedGeometry->wkbType() ) )
   {
-    const QgsGeometryCollection *collection = qgsgeometry_cast<const QgsGeometryCollection *>( geometry );
+    const QgsGeometryCollection *collection = qgsgeometry_cast<const QgsGeometryCollection *>( convertedGeometry );
     if ( collection )
     {
       if ( collection->numGeometries() == 1 )
       {
         const QgsAbstractGeometry *firstGeom = collection->geometryN( 0 );
-        if ( firstGeom && firstGeom->wkbType() == providerGeomType )
+        if ( firstGeom && firstGeom->wkbType() == providerGeometryType )
         {
           outputGeom.reset( firstGeom->clone() );
         }
@@ -988,20 +1009,20 @@ QgsGeometry QgsVectorDataProvider::convertToProviderType( const QgsGeometry &geo
   }
 
   //set z/m types
-  if ( QgsWkbTypes::hasZ( providerGeomType ) )
+  if ( QgsWkbTypes::hasZ( providerGeometryType ) )
   {
     if ( !outputGeom )
     {
-      outputGeom.reset( geometry->clone() );
+      outputGeom.reset( convertedGeometry->clone() );
     }
     outputGeom->addZValue();
   }
 
-  if ( QgsWkbTypes::hasM( providerGeomType ) )
+  if ( QgsWkbTypes::hasM( providerGeometryType ) )
   {
     if ( !outputGeom )
     {
-      outputGeom.reset( geometry->clone() );
+      outputGeom.reset( convertedGeometry->clone() );
     }
     outputGeom->addMValue();
   }
@@ -1014,21 +1035,6 @@ QgsGeometry QgsVectorDataProvider::convertToProviderType( const QgsGeometry &geo
   return QgsGeometry();
 }
 
-void QgsVectorDataProvider::setNativeTypes( const QList<NativeType> &nativeTypes )
-{
-  QGIS_PROTECT_QOBJECT_THREAD_ACCESS
-
-  mNativeTypes = nativeTypes;
-}
-
-QTextCodec *QgsVectorDataProvider::textEncoding() const
-{
-  // non fatal for now -- the "rasterize" processing algorithm is not thread safe and calls this
-  QGIS_PROTECT_QOBJECT_THREAD_ACCESS_NON_FATAL
-
-  return mEncoding;
-}
-
 bool QgsVectorDataProvider::cancelReload()
 {
   QGIS_PROTECT_QOBJECT_THREAD_ACCESS

diff --git a/src/core/vector/qgsvectordataprovider.h b/src/core/vector/qgsvectordataprovider.h
@@ -681,6 +681,7 @@ class CORE_EXPORT QgsVectorDataProvider : public QgsDataProvider, public QgsFeat
     /**
      * Converts the geometry to the provider type if possible / necessary
      * \returns the converted geometry or NULLPTR if no conversion was necessary or possible
+     * \note The default implementation simply calls the static version of this function.
      */
     QgsGeometry convertToProviderType( const QgsGeometry &geom ) const;
 
@@ -699,6 +700,14 @@ class CORE_EXPORT QgsVectorDataProvider : public QgsDataProvider, public QgsFeat
      */
     QTextCodec *textEncoding() const;
 
+    /**
+     * Converts the \a geometry to the provider geometry type \a providerGeometryType if possible / necessary
+     * \returns the converted geometry or NULLPTR if no conversion was necessary or possible
+     * \since QGIS 3.34
+     */
+    static QgsGeometry convertToProviderType( const QgsGeometry &geometry,  Qgis::WkbType providerGeometryType );
+
+
   private:
     mutable bool mCacheMinMaxDirty = true;
     mutable QMap<int, QVariant> mCacheMinValues, mCacheMaxValues;

diff --git a/src/providers/postgres/qgspostgresprovider.cpp b/src/providers/postgres/qgspostgresprovider.cpp
@@ -3312,7 +3312,7 @@ void QgsPostgresProvider::appendGeomParam( const QgsGeometry &geom, QStringList
 
   QString param;
 
-  QgsGeometry convertedGeom( convertToProviderType( geom ) );
+  const QgsGeometry convertedGeom( convertToProviderType( geom, wkbType() ) );
   QByteArray wkb( !convertedGeom.isNull() ? convertedGeom.asWkb() : geom.asWkb() );
   const unsigned char *buf = reinterpret_cast< const unsigned char * >( wkb.constData() );
   int wkbSize = wkb.length();

diff --git a/src/providers/spatialite/qgsspatialiteprovider.cpp b/src/providers/spatialite/qgsspatialiteprovider.cpp
@@ -128,6 +128,7 @@ bool QgsSpatiaLiteProvider::convertField( QgsField &field )
 }
 
 
+
 Qgis::VectorExportResult QgsSpatiaLiteProvider::createEmptyLayer( const QString &uri,
     const QgsFields &fields,
     Qgis::WkbType wkbType,
@@ -4230,7 +4231,8 @@ bool QgsSpatiaLiteProvider::addFeatures( QgsFeatureList &flist, Flags flags )
           {
             unsigned char *wkb = nullptr;
             int wkb_size;
-            QByteArray featureWkb = feature->geometry().asWkb();
+            const QgsGeometry convertedGeom( QgsVectorDataProvider::convertToProviderType( feature->geometry(), wkbType() ) );
+            const QByteArray featureWkb{ !convertedGeom.isNull() ? convertedGeom.asWkb() : feature->geometry().asWkb() };
             convertFromGeosWKB( reinterpret_cast<const unsigned char *>( featureWkb.constData() ),
                                 featureWkb.length(),
                                 &wkb, &wkb_size, nDims );
@@ -4840,7 +4842,8 @@ bool QgsSpatiaLiteProvider::changeGeometryValues( const QgsGeometryMap &geometry
       // binding GEOMETRY to Prepared Statement
       unsigned char *wkb = nullptr;
       int wkb_size;
-      QByteArray iterWkb = iter->asWkb();
+      const QgsGeometry convertedGeom( convertToProviderType( *iter ) );
+      const QByteArray iterWkb{ !convertedGeom.isNull() ? convertedGeom.asWkb() : iter->asWkb() };
       convertFromGeosWKB( reinterpret_cast<const unsigned char *>( iterWkb.constData() ), iterWkb.length(), &wkb, &wkb_size, nDims );
       if ( !wkb )
         sqlite3_bind_null( stmt, 1 );

diff --git a/tests/src/python/test_provider_spatialite.py b/tests/src/python/test_provider_spatialite.py
@@ -45,7 +45,7 @@
 from qgis.utils import spatialite_connect
 
 from providertestbase import ProviderTestCase
-from utilities import unitTestDataPath
+from utilities import unitTestDataPath, compareWkt
 
 # Pass no_exit=True: for some reason this crashes sometimes on exit on Travis
 start_app(True)
@@ -1885,6 +1885,45 @@ def test_absolute_relative_uri(self):
         self.assertEqual(meta.absoluteToRelativeUri(absolute_uri, context), relative_uri)
         self.assertEqual(meta.relativeToAbsoluteUri(relative_uri, context), absolute_uri)
 
+    def testRegression54622Multisurface(self):
+
+        con = spatialite_connect(self.dbname, isolation_level=None)
+        cur = con.cursor()
+        cur.execute("BEGIN")
+        sql = sql = """CREATE TABLE table54622 (
+            _id INTEGER PRIMARY KEY AUTOINCREMENT)"""
+        cur.execute(sql)
+        sql = "SELECT AddGeometryColumn('table54622', 'geometry', 25832, 'MULTIPOLYGON', 'XY', 0)"
+        cur.execute(sql)
+        cur.execute("COMMIT")
+        con.close()
+
+        def _check_feature():
+            layer = QgsVectorLayer(
+                'dbname=\'{}\' table="table54622" (geometry) sql='.format(self.dbname), 'test', 'spatialite')
+            feature = next(layer.getFeatures())
+            self.assertFalse(feature.geometry().isNull())
+            self.assertTrue(compareWkt(feature.geometry().asWkt(), 'MultiPolygon (((-0.886 0.135, -0.886 -0.038, -0.448 -0.070, -0.426 0.143, -0.886 0.135)))', 0.01))
+
+        layer = QgsVectorLayer(
+            'dbname=\'{}\' table="table54622" (geometry) sql='.format(self.dbname), 'test', 'spatialite')
+
+        self.assertTrue(layer.isValid())
+        feature = QgsFeature(layer.fields())
+        geom = QgsGeometry.fromWkt('MULTISURFACE(CURVEPOLYGON(COMPOUNDCURVE((-0.886 0.135,-0.886 -0.038,-0.448 -0.070,-0.427 0.144,-0.886 0.135))))')
+        feature.setGeometry(geom)
+        self.assertTrue(layer.dataProvider().addFeatures([feature]))
+
+        _check_feature()
+
+        self.assertTrue(layer.dataProvider().changeFeatures({}, {feature.id(): geom}))
+
+        _check_feature()
+
+        self.assertTrue(layer.dataProvider().changeGeometryValues({feature.id(): geom}))
+
+        _check_feature()
+
 
 if __name__ == '__main__':
     unittest.main()