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

SPATIALITE: fix insert incompatible geometry types

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);

src/core/vector/qgsvectordataprovider.cpp

@@ -898,120 +898,9 @@ QgsGeometry QgsVectorDataProvider::convertToProviderType( const QgsGeometry &geo
 {
   QGIS_PROTECT_QOBJECT_THREAD_ACCESS
 
-  if ( geom.isNull() )
-  {
-    return QgsGeometry();
-  }
+  // Call the static version
+  return QgsVectorDataProvider::convertToProviderType( geom, wkbType() );
 
-  const QgsAbstractGeometry *geometry = geom.constGet();
-  if ( !geometry )
-  {
-    return QgsGeometry();
-  }
-
-  const Qgis::WkbType providerGeomType = wkbType();
-
-  //geom is already in the provider geometry type
-  if ( geometry->wkbType() == providerGeomType )
-  {
-    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 )
-  {
-    QgsCompoundCurve *compoundCurve = qgsgeometry_cast<QgsCompoundCurve *>( geometry );
-    if ( compoundCurve )
-    {
-      if ( compoundCurve->nCurves() == 1 )
-      {
-        const QgsCircularString *circularString = qgsgeometry_cast<const QgsCircularString *>( compoundCurve->curveAt( 0 ) );
-        if ( circularString )
-        {
-          outputGeom.reset( circularString->clone() );
-        }
-      }
-    }
-  }
-
-  //convert to curved type if necessary
-  if ( !QgsWkbTypes::isCurvedType( geometry->wkbType() ) && QgsWkbTypes::isCurvedType( providerGeomType ) )
-  {
-    QgsAbstractGeometry *curveGeom = outputGeom ? outputGeom->toCurveType() : geometry->toCurveType();
-    if ( curveGeom )
-    {
-      outputGeom.reset( curveGeom );
-    }
-  }
-
-  //convert to linear type from curved type
-  if ( QgsWkbTypes::isCurvedType( geometry->wkbType() ) && !QgsWkbTypes::isCurvedType( providerGeomType ) )
-  {
-    QgsAbstractGeometry *segmentizedGeom = outputGeom ? outputGeom->segmentize() : geometry->segmentize();
-    if ( segmentizedGeom )
-    {
-      outputGeom.reset( segmentizedGeom );
-    }
-  }
-
-  //convert to multitype if necessary
-  if ( QgsWkbTypes::isMultiType( providerGeomType ) && !QgsWkbTypes::isMultiType( geometry->wkbType() ) )
-  {
-    std::unique_ptr< QgsAbstractGeometry > collGeom( QgsGeometryFactory::geomFromWkbType( providerGeomType ) );
-    QgsGeometryCollection *geomCollection = qgsgeometry_cast<QgsGeometryCollection *>( collGeom.get() );
-    if ( geomCollection )
-    {
-      if ( geomCollection->addGeometry( outputGeom ? outputGeom->clone() : geometry->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() ) )
-  {
-    const QgsGeometryCollection *collection = qgsgeometry_cast<const QgsGeometryCollection *>( geometry );
-    if ( collection )
-    {
-      if ( collection->numGeometries() == 1 )
-      {
-        const QgsAbstractGeometry *firstGeom = collection->geometryN( 0 );
-        if ( firstGeom && firstGeom->wkbType() == providerGeomType )
-        {
-          outputGeom.reset( firstGeom->clone() );
-        }
-      }
-    }
-  }
-
-  //set z/m types
-  if ( QgsWkbTypes::hasZ( providerGeomType ) )
-  {
-    if ( !outputGeom )
-    {
-      outputGeom.reset( geometry->clone() );
-    }
-    outputGeom->addZValue();
-  }
-
-  if ( QgsWkbTypes::hasM( providerGeomType ) )
-  {
-    if ( !outputGeom )
-    {
-      outputGeom.reset( geometry->clone() );
-    }
-    outputGeom->addMValue();
-  }
-
-  if ( outputGeom )
-  {
-    return QgsGeometry( outputGeom.release() );
-  }
-
-  return QgsGeometry();
 }
 
 void QgsVectorDataProvider::setNativeTypes( const QList<NativeType> &nativeTypes )
@@ -1029,6 +918,123 @@ QTextCodec *QgsVectorDataProvider::textEncoding() const
   return mEncoding;
 }
 
+QgsGeometry QgsVectorDataProvider::convertToProviderType( const QgsGeometry &geometry, Qgis::WkbType providerGeometryType )
+{
+  if ( geometry.isNull() )
+  {
+    return QgsGeometry();
+  }
+
+  const QgsAbstractGeometry *convertedGeometry = geometry.constGet();
+  if ( !convertedGeometry )
+  {
+    return QgsGeometry();
+  }
+
+
+  //geom is already in the provider geometry type
+  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( providerGeometryType ) == Qgis::WkbType::CircularString )
+  {
+    QgsCompoundCurve *compoundCurve = qgsgeometry_cast<QgsCompoundCurve *>( convertedGeometry );
+    if ( compoundCurve )
+    {
+      if ( compoundCurve->nCurves() == 1 )
+      {
+        const QgsCircularString *circularString = qgsgeometry_cast<const QgsCircularString *>( compoundCurve->curveAt( 0 ) );
+        if ( circularString )
+        {
+          outputGeom.reset( circularString->clone() );
+        }
+      }
+    }
+  }
+
+  //convert to curved type if necessary
+  if ( !QgsWkbTypes::isCurvedType( convertedGeometry->wkbType() ) && QgsWkbTypes::isCurvedType( providerGeometryType ) )
+  {
+    QgsAbstractGeometry *curveGeom = outputGeom ? outputGeom->toCurveType() : convertedGeometry->toCurveType();
+    if ( curveGeom )
+    {
+      outputGeom.reset( curveGeom );
+    }
+  }
+
+  //convert to linear type from curved type
+  if ( QgsWkbTypes::isCurvedType( convertedGeometry->wkbType() ) && !QgsWkbTypes::isCurvedType( providerGeometryType ) )
+  {
+    QgsAbstractGeometry *segmentizedGeom = outputGeom ? outputGeom->segmentize() : convertedGeometry->segmentize();
+    if ( segmentizedGeom )
+    {
+      outputGeom.reset( segmentizedGeom );
+    }
+  }
+
+  //convert to multitype if necessary
+  if ( QgsWkbTypes::isMultiType( providerGeometryType ) && !QgsWkbTypes::isMultiType( convertedGeometry->wkbType() ) )
+  {
+    std::unique_ptr< QgsAbstractGeometry > collGeom( QgsGeometryFactory::geomFromWkbType( providerGeometryType ) );
+    QgsGeometryCollection *geomCollection = qgsgeometry_cast<QgsGeometryCollection *>( collGeom.get() );
+    if ( geomCollection )
+    {
+      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( providerGeometryType ) && QgsWkbTypes::isMultiType( convertedGeometry->wkbType() ) )
+  {
+    const QgsGeometryCollection *collection = qgsgeometry_cast<const QgsGeometryCollection *>( convertedGeometry );
+    if ( collection )
+    {
+      if ( collection->numGeometries() == 1 )
+      {
+        const QgsAbstractGeometry *firstGeom = collection->geometryN( 0 );
+        if ( firstGeom && firstGeom->wkbType() == providerGeometryType )
+        {
+          outputGeom.reset( firstGeom->clone() );
+        }
+      }
+    }
+  }
+
+  //set z/m types
+  if ( QgsWkbTypes::hasZ( providerGeometryType ) )
+  {
+    if ( !outputGeom )
+    {
+      outputGeom.reset( convertedGeometry->clone() );
+    }
+    outputGeom->addZValue();
+  }
+
+  if ( QgsWkbTypes::hasM( providerGeometryType ) )
+  {
+    if ( !outputGeom )
+    {
+      outputGeom.reset( convertedGeometry->clone() );
+    }
+    outputGeom->addMValue();
+  }
+
+  if ( outputGeom )
+  {
+    return QgsGeometry( outputGeom.release() );
+  }
+
+  return QgsGeometry();
+}
+
 bool QgsVectorDataProvider::cancelReload()
 {
   QGIS_PROTECT_QOBJECT_THREAD_ACCESS

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;

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();

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 );

tests/src/python/test_provider_spatialite.py

@@ -45,7 +45,7 @@ from qgis.testing import start_app, QgisTestCase
 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 @@ class TestQgsSpatialiteProvider(QgisTestCase, ProviderTestCase):
         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()