Fix query layers from SQL window trailing semicolon

Fix #56993
qgis · Jan 22, 2025 · 1de194a · 1de194a
1 parent ef0dd03
commit 1de194a
Show file tree

Hide file tree

Showing 10 changed files with 32 additions and 6 deletions.
diff --git a/python/PyQt6/core/auto_generated/providers/qgsabstractdatabaseproviderconnection.sip.in b/python/PyQt6/core/auto_generated/providers/qgsabstractdatabaseproviderconnection.sip.in
@@ -1033,6 +1033,8 @@ Checks if ``capability`` is supported.
 
 
 
+
+
 };
 
 QFlags<QgsAbstractDatabaseProviderConnection::Capability> operator|(QgsAbstractDatabaseProviderConnection::Capability f1, QFlags<QgsAbstractDatabaseProviderConnection::Capability> f2);

diff --git a/python/core/auto_generated/providers/qgsabstractdatabaseproviderconnection.sip.in b/python/core/auto_generated/providers/qgsabstractdatabaseproviderconnection.sip.in
@@ -1033,6 +1033,8 @@ Checks if ``capability`` is supported.
 
 
 
+
+
 };
 
 QFlags<QgsAbstractDatabaseProviderConnection::Capability> operator|(QgsAbstractDatabaseProviderConnection::Capability f1, QFlags<QgsAbstractDatabaseProviderConnection::Capability> f2);

diff --git a/src/core/providers/ogr/qgsogrproviderconnection.cpp b/src/core/providers/ogr/qgsogrproviderconnection.cpp
@@ -285,7 +285,7 @@ QgsVectorLayer *QgsOgrProviderConnection::createSqlVectorLayer( const QgsAbstrac
   QgsProviderMetadata *providerMetadata { QgsProviderRegistry::instance()->providerMetadata( QStringLiteral( "ogr" ) ) };
   Q_ASSERT( providerMetadata );
   QMap<QString, QVariant> decoded = providerMetadata->decodeUri( uri() );
-  decoded[ QStringLiteral( "subset" ) ] = options.sql;
+  decoded[ QStringLiteral( "subset" ) ] = sanitizeSqlForQueryLayer( options.sql ) ;
   return new QgsVectorLayer( providerMetadata->encodeUri( decoded ), options.layerName.isEmpty() ? QStringLiteral( "QueryLayer" ) : options.layerName, providerKey() );
 }
 

diff --git a/src/core/providers/qgsabstractdatabaseproviderconnection.cpp b/src/core/providers/qgsabstractdatabaseproviderconnection.cpp
@@ -84,6 +84,16 @@ void QgsAbstractDatabaseProviderConnection::checkCapability( Qgis::DatabaseProvi
   }
 }
 
+QString QgsAbstractDatabaseProviderConnection::sanitizeSqlForQueryLayer( const QString &sql ) const
+{
+  QString sanitizedSql { sql.trimmed() };
+  while ( sanitizedSql.endsWith( ';' ) )
+  {
+    sanitizedSql.chop( 1 );
+  }
+  return sanitizedSql;
+}
+
 ///@endcond
 
 QString QgsAbstractDatabaseProviderConnection::providerKey() const

diff --git a/src/core/providers/qgsabstractdatabaseproviderconnection.h b/src/core/providers/qgsabstractdatabaseproviderconnection.h
@@ -1143,6 +1143,10 @@ class CORE_EXPORT QgsAbstractDatabaseProviderConnection : public QgsAbstractProv
      * \throws QgsProviderConnectionException if the capability is not supported
      */
     void checkCapability( Qgis::DatabaseProviderConnectionCapability2 capability ) const;
+
+    //! Trim and remove any trailing semicolon
+    QString sanitizeSqlForQueryLayer( const QString &sql ) const SIP_SKIP;
+
 ///@endcond
 
     Capabilities mCapabilities = Capabilities() SIP_SKIP;

diff --git a/src/providers/hana/qgshanaproviderconnection.cpp b/src/providers/hana/qgshanaproviderconnection.cpp
@@ -496,7 +496,7 @@ QgsVectorLayer *QgsHanaProviderConnection::createSqlVectorLayer( const SqlVector
   if ( !options.primaryKeyColumns.isEmpty() )
   {
     tUri.setKeyColumn( QgsHanaPrimaryKeyUtils::buildUriKey( options.primaryKeyColumns ) );
-    tUri.setTable( QStringLiteral( "(%1)" ).arg( options.sql ) );
+    tUri.setTable( QStringLiteral( "(%1)" ).arg( sanitizeSqlForQueryLayer( options.sql ) ) );
   }
   else
   {

diff --git a/src/providers/oracle/qgsoracleproviderconnection.cpp b/src/providers/oracle/qgsoracleproviderconnection.cpp
@@ -173,7 +173,7 @@ QgsVectorLayer *QgsOracleProviderConnection::createSqlVectorLayer( const QgsAbst
   if ( !options.primaryKeyColumns.isEmpty() )
   {
     tUri.setKeyColumn( options.primaryKeyColumns.join( ',' ) );
-    tUri.setTable( QStringLiteral( "(%1)" ).arg( options.sql ) );
+    tUri.setTable( QStringLiteral( "(%1)" ).arg( sanitizeSqlForQueryLayer( options.sql ) ) );
   }
   else
   {

diff --git a/src/providers/postgres/qgspostgresproviderconnection.cpp b/src/providers/postgres/qgspostgresproviderconnection.cpp
@@ -876,7 +876,7 @@ QgsVectorLayer *QgsPostgresProviderConnection::createSqlVectorLayer( const SqlVe
   if ( !options.primaryKeyColumns.isEmpty() )
   {
     tUri.setKeyColumn( options.primaryKeyColumns.join( ',' ) );
-    tUri.setTable( QStringLiteral( "(%1)" ).arg( options.sql ) );
+    tUri.setTable( QStringLiteral( "(%1)" ).arg( sanitizeSqlForQueryLayer( options.sql ) ) );
   }
   else
   {
@@ -892,7 +892,7 @@ QgsVectorLayer *QgsPostgresProviderConnection::createSqlVectorLayer( const SqlVe
     {
       sqlId++;
     }
-    tUri.setTable( QStringLiteral( "(SELECT row_number() over () AS _uid%1_, * FROM (%2\n) AS _subq_%3_\n)" ).arg( QString::number( pkId ), options.sql, QString::number( sqlId ) ) );
+    tUri.setTable( QStringLiteral( "(SELECT row_number() over () AS _uid%1_, * FROM (%2\n) AS _subq_%3_\n)" ).arg( QString::number( pkId ), sanitizeSqlForQueryLayer( options.sql ), QString::number( sqlId ) ) );
   }
 
   if ( !options.geometryColumn.isEmpty() )

diff --git a/src/providers/spatialite/qgsspatialiteproviderconnection.cpp b/src/providers/spatialite/qgsspatialiteproviderconnection.cpp
@@ -134,7 +134,7 @@ QgsVectorLayer *QgsSpatiaLiteProviderConnection::createSqlVectorLayer( const Qgs
   QgsDataSourceUri tUri( uri() );
 
   tUri.setSql( options.filter );
-  tUri.setTable( '(' + options.sql + ')' );
+  tUri.setTable( '(' + sanitizeSqlForQueryLayer( options.sql ) + ')' );
 
   if ( !options.geometryColumn.isEmpty() )
   {

diff --git a/tests/src/python/test_qgsproviderconnection_base.py b/tests/src/python/test_qgsproviderconnection_base.py
@@ -783,6 +783,14 @@ def testCreateSqlVectorLayer(self):
         self.assertEqual(vl.name(), options.layerName)
         self.assertEqual(vl.sourceCrs().authid(), crs)
 
+        # Test issue https://github.com/qgis/QGIS/issues/56993
+        options.sql += ';;;'
+        vl2 = conn.createSqlVectorLayer(options)
+        self.assertTrue(vl2.isValid())
+        self.assertTrue(vl2.isSqlQuery())
+        self.assertTrue(vl2.isSpatial())
+        del vl2
+
         # Test that a database connection can be retrieved from an existing layer
         vlconn = QgsMapLayerUtils.databaseConnection(vl)
         self.assertIsNotNone(vlconn)
Original file line number	Diff line number	Diff line change
Expand Up		@@ -1033,6 +1033,8 @@ Checks if ``capability`` is supported.





		};

		QFlags<QgsAbstractDatabaseProviderConnection::Capability> operator\|(QgsAbstractDatabaseProviderConnection::Capability f1, QFlags<QgsAbstractDatabaseProviderConnection::Capability> f2);
Expand Down