[Backport release-3_40] Resurrecting SQL server testing on CI, many fixes

.ci/test_blocklist_qt5.txt

@@ -17,7 +17,3 @@ test_core_openclutils
 # Relies on a broken/unreliable 3rd party service
 test_core_layerdefinition
 
-# MSSQL requires the MSSQL docker
-PyQgsProviderConnectionMssql
-PyQgsStyleStorageMssql
-

.ci/test_flaky.txt

@@ -4,7 +4,3 @@ test_provider_wcsprovider
 PyQgsWFSProviderGUI
 # See https://github.com/qgis/QGIS/issues/48927
 test_core_tiledownloadmanager
-
-# Flaky, the ms odbc driver crashes a lot on the ubuntu docker image. Retest when
-# the docker base image is upgraded
-PyQgsMssqlProvider

.docker/docker-compose-testing-mssql.yml

@@ -0,0 +1,25 @@
+version: '3'
+
+services:
+  mssql:
+    image: mcr.microsoft.com/mssql/server:2022-latest
+    environment:
+      ACCEPT_EULA: Y
+      MSSQL_SA_PASSWORD: QGIStestSQLServer1234
+    ports:
+      - 1433:1433
+
+  qgis-deps:
+    tty: true
+    image: qgis3-build-deps-binary-image
+    volumes:
+      - ${QGIS_WORKSPACE}:/root/QGIS
+    links:
+      - mssql
+    env_file:
+      - docker-variables.env
+    environment:
+      - LANG=C.UTF-8
+      - LC_ALL=en_US.UTF-8
+    cap_add:
+      - NET_ADMIN

.docker/docker-compose-testing.yml

@@ -1,13 +1,6 @@
 version: '3'
 services:
 
-# Proving very fragile!
-#  mssql:
-#    image: microsoft/mssql-server-linux:2017-latest
-#    environment:
-#      ACCEPT_EULA: Y
-#      SA_PASSWORD: <YourStrong!Passw0rd>
-
   httpbin:
     image: kennethreitz/httpbin:latest
 
@@ -34,7 +27,6 @@ services:
       - ${QGIS_WORKSPACE}:/root/QGIS
       - ${QGIS_COMMON_GIT_DIR}:${QGIS_COMMON_GIT_DIR}
     links:
-    #  - mssql
       - webdav
       - minio
       - httpbin

.docker/docker-qgis-build.sh

@@ -87,6 +87,7 @@ cmake \
  -DENABLE_PGTEST=${WITH_QT5} \
  -DENABLE_SAGA_TESTS=${WITH_QT5} \
  -DENABLE_MSSQLTEST=${WITH_QT5} \
+ -DENABLE_MSSQLTEST_CPP=${WITH_QT5} \
  -DENABLE_HANATEST=${WITH_QT5} \
  -DENABLE_ORACLETEST=${WITH_QT5} \
  -DPUSH_TO_CDASH=${PUSH_TO_CDASH} \

.docker/docker-qgis-test.sh

@@ -178,12 +178,15 @@ if [ ${RUN_SQLSERVER:-"NO"} == "YES" ]; then
   # Restore SQL Server test data
   ##############################
 
+  echo "Wait a moment before loading SQL Server database."
+  sleep 15
+
   echo "Importing SQL Server test data..."
 
   export SQLUSER=sa
   export SQLHOST=mssql
   export SQLPORT=1433
-  export SQLPASSWORD='<YourStrong!Passw0rd>'
+  export SQLPASSWORD=QGIStestSQLServer1234
   export SQLDATABASE=qgis_test
 
   export PATH=$PATH:/opt/mssql-tools/bin
@@ -196,12 +199,14 @@ if [ ${RUN_SQLSERVER:-"NO"} == "YES" ]; then
 
   cat <<EOT > /etc/odbc.ini
 [ODBC Data Sources]
-testsqlserver = ODBC Driver 17 for SQL Server
+testsqlserver = ODBC Driver 18 for SQL Server
 
 [testsqlserver]
-Driver       = ODBC Driver 17 for SQL Server
+Driver       = ODBC Driver 18 for SQL Server
 Description  = Test SQL Server
 Server       = mssql
+Encrypt      = no
+AllowSelfSignedServerCert=1
 EOT
 
   echo "::endgroup::"

.docker/qgis3-qt5-build-deps.dockerfile

@@ -165,10 +165,12 @@ RUN curl -j -k -L -H "Cookie: eula_3_2_agreed=tools.hana.ondemand.com/developer-
 ENV PATH="/usr/sap/hdbclient:${PATH}"
 
 # MSSQL: client side
-# RUN curl https://packages.microsoft.com/keys/microsoft.asc | apt-key add -
-# RUN curl https://packages.microsoft.com/config/ubuntu/19.04/prod.list | tee /etc/apt/sources.list.d/msprod.list
-# RUN apt-get update
-# RUN ACCEPT_EULA=Y apt-get install -y --allow-unauthenticated msodbcsql17 mssql-tools
+RUN curl -sSL -O https://packages.microsoft.com/config/ubuntu/$(grep VERSION_ID /etc/os-release | cut -d '"' -f 2)/packages-microsoft-prod.deb
+RUN dpkg -i packages-microsoft-prod.deb
+RUN rm packages-microsoft-prod.deb
+RUN apt-get update
+RUN ACCEPT_EULA=Y apt-get install -y --allow-unauthenticated msodbcsql18 mssql-tools18
+ENV PATH="/opt/mssql-tools18/bin:${PATH}"
 
 FROM binary-only
 

.github/workflows/run-tests.yml

@@ -240,7 +240,7 @@ jobs:
     strategy:
       matrix:
         qt-version: [5, 6]
-        test-batch: [ALL_BUT_PROVIDERS, POSTGRES, HANA]
+        test-batch: [ALL_BUT_PROVIDERS, POSTGRES, HANA, SQLSERVER]
 
         include:
           - qt-version: 5
@@ -263,6 +263,9 @@ jobs:
           - qt-version: 6
             test-batch: POSTGRES
 
+          - qt-version: 6
+            test-batch: SQLSERVER
+
       fail-fast: false
 
     steps:
@@ -348,6 +351,7 @@ jobs:
         run: |
           DOCKERFILE=$( ( [[ ${{ matrix.test-batch }} == "ORACLE"   ]] && echo "docker-compose-testing-oracle.yml" ) \
                      || ( [[ ${{ matrix.test-batch }} == "POSTGRES" ]] && echo "docker-compose-testing-postgres.yml" ) \
+                     || ( [[ ${{ matrix.test-batch }} == "SQLSERVER" ]] && echo "docker-compose-testing-mssql.yml" ) \
                      || echo "docker-compose-testing.yml" )
           [[ ${{ matrix.test-batch }} == "ORACLE" ]] && sudo rm -rf /usr/share/dotnet/sdk
           echo "TEST_BATCH=$TEST_BATCH"

src/providers/mssql/qgsmssqlprovider.cpp

@@ -425,7 +425,7 @@ void QgsMssqlProvider::loadFields()
     }
   }
 
-  const QString sql3 { QStringLiteral( "exec sp_columns @table_name = N%1, @table_owner = %2" ).arg( quotedValue( mTableName ), quotedValue( mSchemaName ) ) };
+  const QString sql3 { QStringLiteral( "exec sp_columns @table_name = %1, @table_owner = %2" ).arg( quotedValue( mTableName ), quotedValue( mSchemaName ) ) };
   if ( !LoggedExec( query, sql3 ) )
   {
     pushError( query.lastError().text() );
@@ -528,7 +528,7 @@ void QgsMssqlProvider::loadFields()
   {
     query.clear();
     query.setForwardOnly( true );
-    const QString sql4 { QStringLiteral( "exec sp_pkeys @table_name = N%1, @table_owner = %2 " ).arg( quotedValue( mTableName ), quotedValue( mSchemaName ) ) };
+    const QString sql4 { QStringLiteral( "exec sp_pkeys @table_name = %1, @table_owner = %2 " ).arg( quotedValue( mTableName ), quotedValue( mSchemaName ) ) };
     if ( !LoggedExec( query, sql4 ) )
     {
       QgsDebugError( QStringLiteral( "SQL:%1\n  Error:%2" ).arg( query.lastQuery(), query.lastError().text() ) );
@@ -619,7 +619,7 @@ QString QgsMssqlProvider::quotedValue( const QVariant &value )
       if ( v.contains( '\\' ) )
         return v.replace( '\\', QLatin1String( "\\\\" ) ).prepend( "N'" ).append( '\'' );
       else
-        return v.prepend( '\'' ).append( '\'' );
+        return v.prepend( "N'" ).append( '\'' );
   }
 }
 
@@ -2502,6 +2502,13 @@ Qgis::VectorExportResult QgsMssqlProviderMetadata::createEmptyLayer( const QStri
   );
 }
 
+
+QString buildfTableCatalogClause( const QgsDataSourceUri &dsUri )
+{
+  return QStringLiteral( "f_table_catalog%1" ).arg( dsUri.database().isEmpty() ? QStringLiteral( " IS NULL" ) : QStringLiteral( "=%1" ).arg( QgsMssqlProvider::quotedValue( dsUri.database() ) ) );
+}
+
+
 bool QgsMssqlProviderMetadata::styleExists( const QString &uri, const QString &styleId, QString &errorCause )
 {
   errorCause.clear();
@@ -2535,12 +2542,12 @@ bool QgsMssqlProviderMetadata::styleExists( const QString &uri, const QString &s
   query.setForwardOnly( true );
   const QString checkQuery = QString( "SELECT styleName"
                                       " FROM layer_styles"
-                                      " WHERE f_table_catalog=%1"
+                                      " WHERE %1"
                                       " AND f_table_schema=%2"
                                       " AND f_table_name=%3"
                                       " AND f_geometry_column=%4"
                                       " AND styleName=%5" )
-                               .arg( QgsMssqlProvider::quotedValue( dsUri.database() ) )
+                               .arg( buildfTableCatalogClause( dsUri ) )
                                .arg( QgsMssqlProvider::quotedValue( dsUri.schema() ) )
                                .arg( QgsMssqlProvider::quotedValue( dsUri.table() ) )
                                .arg( QgsMssqlProvider::quotedValue( dsUri.geometryColumn() ) )
@@ -2650,12 +2657,12 @@ bool QgsMssqlProviderMetadata::saveStyle( const QString &uri, const QString &qml
 
   const QString checkQuery = QStringLiteral( "SELECT styleName"
                                              " FROM layer_styles"
-                                             " WHERE f_table_catalog=%1"
+                                             " WHERE %1"
                                              " AND f_table_schema=%2"
                                              " AND f_table_name=%3"
                                              " AND f_geometry_column=%4"
                                              " AND styleName=%5" )
-                               .arg( QgsMssqlProvider::quotedValue( dsUri.database() ) )
+                               .arg( buildfTableCatalogClause( dsUri ) )
                                .arg( QgsMssqlProvider::quotedValue( dsUri.schema() ) )
                                .arg( QgsMssqlProvider::quotedValue( dsUri.table() ) )
                                .arg( QgsMssqlProvider::quotedValue( dsUri.geometryColumn() ) )
@@ -2676,7 +2683,7 @@ bool QgsMssqlProviderMetadata::saveStyle( const QString &uri, const QString &qml
                    ",styleSLD=%3"
                    ",description=%4"
                    ",owner=%5"
-                   " WHERE f_table_catalog=%6"
+                   " WHERE %6"
                    " AND f_table_schema=%7"
                    " AND f_table_name=%8"
                    " AND f_geometry_column=%9"
@@ -2686,7 +2693,7 @@ bool QgsMssqlProviderMetadata::saveStyle( const QString &uri, const QString &qml
             .arg( QgsMssqlProvider::quotedValue( sldStyle ) )
             .arg( QgsMssqlProvider::quotedValue( styleDescription.isEmpty() ? QDateTime::currentDateTime().toString() : styleDescription ) )
             .arg( QgsMssqlProvider::quotedValue( dsUri.username() ) )
-            .arg( QgsMssqlProvider::quotedValue( dsUri.database() ) )
+            .arg( buildfTableCatalogClause( dsUri ) )
             .arg( QgsMssqlProvider::quotedValue( dsUri.schema() ) )
             .arg( QgsMssqlProvider::quotedValue( dsUri.table() ) )
             .arg( QgsMssqlProvider::quotedValue( dsUri.geometryColumn() ) )
@@ -2696,11 +2703,11 @@ bool QgsMssqlProviderMetadata::saveStyle( const QString &uri, const QString &qml
   {
     const QString removeDefaultSql = QString( "UPDATE layer_styles "
                                               " SET useAsDefault=0"
-                                              " WHERE f_table_catalog=%1"
+                                              " WHERE %1"
                                               " AND f_table_schema=%2"
                                               " AND f_table_name=%3"
                                               " AND f_geometry_column=%4" )
-                                       .arg( QgsMssqlProvider::quotedValue( dsUri.database() ) )
+                                       .arg( buildfTableCatalogClause( dsUri ) )
                                        .arg( QgsMssqlProvider::quotedValue( dsUri.schema() ) )
                                        .arg( QgsMssqlProvider::quotedValue( dsUri.table() ) )
                                        .arg( QgsMssqlProvider::quotedValue( dsUri.geometryColumn() ) );
@@ -2764,12 +2771,12 @@ QString QgsMssqlProviderMetadata::loadStoredStyle( const QString &uri, QString &
 
   const QString selectQmlQuery = QString( "SELECT top 1 styleName, styleQML"
                                           " FROM layer_styles"
-                                          " WHERE f_table_catalog=%1"
+                                          " WHERE %1"
                                           " AND f_table_schema=%2"
                                           " AND f_table_name=%3"
                                           " AND f_geometry_column=%4"
                                           " ORDER BY useAsDefault desc" )
-                                   .arg( QgsMssqlProvider::quotedValue( dsUri.database() ) )
+                                   .arg( buildfTableCatalogClause( dsUri ) )
                                    .arg( QgsMssqlProvider::quotedValue( dsUri.schema() ) )
                                    .arg( QgsMssqlProvider::quotedValue( dsUri.table() ) )
                                    .arg( QgsMssqlProvider::quotedValue( dsUri.geometryColumn() ) );
@@ -2823,14 +2830,16 @@ int QgsMssqlProviderMetadata::listStyles( const QString &uri, QStringList &ids,
     return -1;
   }
 
+  const QString fTableCatalogClause = buildfTableCatalogClause( dsUri );
+
   const QString selectRelatedQuery = QString( "SELECT id,styleName,description"
                                               " FROM layer_styles "
-                                              " WHERE f_table_catalog=%1"
+                                              " WHERE %1"
                                               " AND f_table_schema=%2"
                                               " AND f_table_name=%3"
                                               " AND f_geometry_column=%4"
                                               " ORDER BY useasdefault DESC, update_time DESC" )
-                                       .arg( QgsMssqlProvider::quotedValue( dsUri.database() ) )
+                                       .arg( fTableCatalogClause )
                                        .arg( QgsMssqlProvider::quotedValue( dsUri.schema() ) )
                                        .arg( QgsMssqlProvider::quotedValue( dsUri.table() ) )
                                        .arg( QgsMssqlProvider::quotedValue( dsUri.geometryColumn() ) );
@@ -2853,9 +2862,9 @@ int QgsMssqlProviderMetadata::listStyles( const QString &uri, QStringList &ids,
   }
   const QString selectOthersQuery = QString( "SELECT id,styleName,description"
                                              " FROM layer_styles "
-                                             " WHERE NOT (f_table_catalog=%1 AND f_table_schema=%2 AND f_table_name=%3 AND f_geometry_column=%4)"
+                                             " WHERE NOT (%1 AND f_table_schema=%2 AND f_table_name=%3 AND f_geometry_column=%4)"
                                              " ORDER BY update_time DESC" )
-                                      .arg( QgsMssqlProvider::quotedValue( dsUri.database() ) )
+                                      .arg( fTableCatalogClause )
                                       .arg( QgsMssqlProvider::quotedValue( dsUri.schema() ) )
                                       .arg( QgsMssqlProvider::quotedValue( dsUri.table() ) )
                                       .arg( QgsMssqlProvider::quotedValue( dsUri.geometryColumn() ) );

src/providers/mssql/qgsmssqlproviderconnection.cpp

@@ -117,14 +117,14 @@ void QgsMssqlProviderConnection::dropTablePrivate( const QString &schema, const
   DECLARE @table nvarchar(50)
   DECLARE @schema nvarchar(50)
 
-  set @database = N%1
-  set @table = N%2
-  set @schema = N%3
+  set @database = %1
+  set @table = %2
+  set @schema = %3
 
   DECLARE @sql nvarchar(255)
   WHILE EXISTS(select * from INFORMATION_SCHEMA.TABLE_CONSTRAINTS where CONSTRAINT_CATALOG = @database and TABLE_NAME = @table AND TABLE_SCHEMA = @schema )
   BEGIN
-      select    @sql = 'ALTER TABLE ' + @table + ' DROP CONSTRAINT ' + CONSTRAINT_NAME
+      select    @sql = 'ALTER TABLE [' + @schema + '].[' + @table + '] DROP CONSTRAINT [' + CONSTRAINT_NAME + ']'
       from    INFORMATION_SCHEMA.TABLE_CONSTRAINTS
       where    constraint_catalog = @database and
               table_name = @table and table_schema = @schema
@@ -359,7 +359,7 @@ QList<QgsMssqlProviderConnection::TableProperty> QgsMssqlProviderConnection::tab
 
   if ( useGeometryColumnsOnly )
   {
-    query += QStringLiteral( "f_table_schema, f_table_name, f_geometry_column, srid, geometry_type, 0 FROM geometry_columns WHERE f_table_schema = N%1" )
+    query += QStringLiteral( "f_table_schema, f_table_name, f_geometry_column, srid, geometry_type, 0 FROM geometry_columns WHERE f_table_schema = %1" )
                .arg( QgsMssqlProvider::quotedValue( schema ) );
   }
   else
@@ -374,7 +374,7 @@ QList<QgsMssqlProviderConnection::TableProperty> QgsMssqlProviderConnection::tab
                                 JOIN sys.schemas
                                   ON sys.objects.schema_id = sys.schemas.schema_id
                                 WHERE
-                                  sys.schemas.name = N%1
+                                  sys.schemas.name = %1
                                   AND (sys.types.name = 'geometry' OR sys.types.name = 'geography')
                                   AND (sys.objects.type = 'U' OR sys.objects.type = 'V')
                              )raw" )
@@ -390,7 +390,7 @@ QList<QgsMssqlProviderConnection::TableProperty> QgsMssqlProviderConnection::tab
                                JOIN sys.schemas
                                   ON sys.objects.schema_id = sys.schemas.schema_id
                              WHERE
-                               sys.schemas.name = N%1
+                               sys.schemas.name = %1
                                AND NOT EXISTS
                                 (SELECT *
                                   FROM sys.columns sc1 JOIN sys.types ON sc1.system_type_id = sys.types.system_type_id

tests/src/providers/CMakeLists.txt

@@ -46,7 +46,7 @@ endif()
 
 set(ENABLE_MSSQLTEST_CPP FALSE CACHE BOOL "Enable MSSQL provider C++ tests")
 if (ENABLE_MSSQLTEST_CPP)
-  add_qgis_test(testqgsmssqlprovider.cpp MODULE provider LINKEDLIBRARIES provider_mssql_a qgis_core LABELS "MSSQL")
+  add_qgis_test(testqgsmssqlprovider.cpp MODULE provider LINKEDLIBRARIES provider_mssql_a qgis_core LABELS "SQLSERVER")
   # also depend on dynamic lib, so that building this test will also build the dynamic lib
   add_dependencies(test_provider_mssqlprovider provider_mssql)
 endif()

tests/src/providers/testqgsmssqlprovider.cpp

@@ -65,7 +65,7 @@ void TestQgsMssqlProvider::initTestCase()
   QgsApplication::init();
   QgsApplication::initQgis();
 
-  mDbConn = qEnvironmentVariable( "QGIS_MSSQLTEST_DB", "service='testsqlserver' user=sa password='<YourStrong!Passw0rd>' " );
+  mDbConn = qEnvironmentVariable( "QGIS_MSSQLTEST_DB", "service='testsqlserver' user=sa password='QGIStestSQLServer1234' " );
 
   mSomeDataWktGeom << QStringLiteral( "Point (-70.33199999999999363 66.32999999999999829)" )
                    << QStringLiteral( "Point (-68.20000000000000284 70.79999999999999716)" )

tests/src/python/CMakeLists.txt

@@ -596,7 +596,7 @@ if (ENABLE_MSSQLTEST)
   ADD_PYTHON_TEST(PyQgsMssqlProvider test_provider_mssql.py)
   ADD_PYTHON_TEST(PyQgsProviderConnectionMssql test_qgsproviderconnection_mssql.py)
   ADD_PYTHON_TEST(PyQgsStyleStorageMssql test_stylestorage_mssql.py)
-  SET_TESTS_PROPERTIES(PyQgsMssqlProvider PyQgsProviderConnectionMssql PyQgsStyleStorageMssql PROPERTIES LABELS "MSSQL")
+  SET_TESTS_PROPERTIES(PyQgsMssqlProvider PyQgsProviderConnectionMssql PyQgsStyleStorageMssql PROPERTIES LABELS "SQLSERVER")
 endif()
 
 if (ENABLE_ORACLETEST)

tests/src/python/stylestoragebase.py

@@ -137,7 +137,9 @@ def testMultipleStyles(self):
         vl.saveStyleToDatabase("style3", "style3", False, None)
         num, ids, names, desc, err = vl.listStylesInDatabase()
 
-        self.assertTrue({"style2", "style3", "style1"}.issubset(set(names)))
+        self.assertIn("style1", names)
+        self.assertIn("style2", names)
+        self.assertIn("style3", names)
 
         del vl
         vl = QgsVectorLayer(uri, "vl", self.providerKey)