[mssql] use nullable info from db to set field ConstraintNotNull

qgis · Jun 10, 2021 · 59be24f · 59be24f
1 parent be23718
commit 59be24f
Show file tree

Hide file tree

Showing 3 changed files with 183 additions and 133 deletions.
diff --git a/src/providers/mssql/qgsmssqlprovider.cpp b/src/providers/mssql/qgsmssqlprovider.cpp
@@ -364,175 +364,175 @@ void QgsMssqlProvider::loadFields()
     return;
   }
 
-  if ( query.isActive() )
+  while ( query.next() )
   {
-    while ( query.next() )
-    {
-      mComputedColumns.append( query.value( 0 ).toString() );
-    }
+    mComputedColumns.append( query.value( 0 ).toString() );
   }
 
   if ( !query.exec( QStringLiteral( "exec sp_columns @table_name = N%1, @table_owner = %2" ).arg( quotedValue( mTableName ), quotedValue( mSchemaName ) ) ) )
   {
     pushError( query.lastError().text() );
     return;
   }
-  if ( query.isActive() )
+
+  int i = 0;
+  QStringList pkCandidates;
+  while ( query.next() )
   {
-    int i = 0;
-    QStringList pkCandidates;
-    while ( query.next() )
-    {
-      const QString colName = query.value( 3 ).toString();
-      const QString sqlTypeName = query.value( 5 ).toString();
+    const QString colName = query.value( QStringLiteral( "COLUMN_NAME" ) ).toString();
+    const QString sqlTypeName = query.value( QStringLiteral( "TYPE_NAME" ) ).toString();
 
-      // if we don't have an explicitly set geometry column name, and this is a geometry column, then use it
-      // but if we DO have an explicitly set geometry column name, then load the other information if this is that column
-      if ( ( mGeometryColName.isEmpty() && ( sqlTypeName == QLatin1String( "geometry" ) || sqlTypeName == QLatin1String( "geography" ) ) )
-           || colName == mGeometryColName )
+    // if we don't have an explicitly set geometry column name, and this is a geometry column, then use it
+    // but if we DO have an explicitly set geometry column name, then load the other information if this is that column
+    if ( ( mGeometryColName.isEmpty() && ( sqlTypeName == QLatin1String( "geometry" ) || sqlTypeName == QLatin1String( "geography" ) ) )
+         || colName == mGeometryColName )
+    {
+      mGeometryColName = colName;
+      mGeometryColType = sqlTypeName;
+      mParser.mIsGeography = sqlTypeName == QLatin1String( "geography" );
+    }
+    else
+    {
+      QVariant::Type sqlType = DecodeSqlType( sqlTypeName );
+      if ( sqlTypeName == QLatin1String( "int identity" ) || sqlTypeName == QLatin1String( "bigint identity" ) )
       {
-        mGeometryColName = colName;
-        mGeometryColType = sqlTypeName;
-        mParser.mIsGeography = sqlTypeName == QLatin1String( "geography" );
+        mPrimaryKeyType = PktInt;
+        mPrimaryKeyAttrs << mAttributeFields.size();
+        isIdentity = true;
       }
-      else
+      else if ( sqlTypeName == QLatin1String( "int" ) || sqlTypeName == QLatin1String( "bigint" ) )
       {
-        QVariant::Type sqlType = DecodeSqlType( sqlTypeName );
-        if ( sqlTypeName == QLatin1String( "int identity" ) || sqlTypeName == QLatin1String( "bigint identity" ) )
-        {
-          mPrimaryKeyType = PktInt;
-          mPrimaryKeyAttrs << mAttributeFields.size();
-          isIdentity = true;
-        }
-        else if ( sqlTypeName == QLatin1String( "int" ) || sqlTypeName == QLatin1String( "bigint" ) )
-        {
-          pkCandidates << query.value( 3 ).toString();
-        }
+        pkCandidates << query.value( 3 ).toString();
+      }
 
-        if ( sqlType == QVariant::String )
-        {
-          // Field length in chars is column 7 ("Length") of the sp_columns output,
-          // except for uniqueidentifiers which must use column 6 ("Precision").
-          int length = query.value( sqlTypeName.startsWith( QLatin1String( "uniqueidentifier" ), Qt::CaseInsensitive ) ? 6 : 7 ).toInt();
-          if ( sqlTypeName.startsWith( QLatin1Char( 'n' ) ) )
-          {
-            length = length / 2;
-          }
-          mAttributeFields.append(
-            QgsField(
-              colName, sqlType,
-              sqlTypeName,
-              length ) );
-        }
-        else if ( sqlType == QVariant::Double )
-        {
-          mAttributeFields.append(
-            QgsField(
-              colName, sqlType,
-              sqlTypeName,
-              query.value( 6 ).toInt(),
-              sqlTypeName == QLatin1String( "decimal" ) ? query.value( 8 ).toInt() : -1 ) );
-        }
-        else if ( sqlType == QVariant::Date || sqlType == QVariant::DateTime || sqlType == QVariant::Time )
-        {
-          mAttributeFields.append(
-            QgsField(
-              colName, sqlType,
-              sqlTypeName,
-              -1,
-              -1 ) );
-        }
-        else
+      QgsField field;
+      if ( sqlType == QVariant::String )
+      {
+        // Field length in chars is column 7 ("Length") of the sp_columns output,
+        // except for uniqueidentifiers which must use column 6 ("Precision").
+        int length = query.value( sqlTypeName.startsWith( QLatin1String( "uniqueidentifier" ), Qt::CaseInsensitive ) ? 6 : 7 ).toInt();
+        if ( sqlTypeName.startsWith( QLatin1Char( 'n' ) ) )
         {
-          mAttributeFields.append(
-            QgsField(
-              colName, sqlType,
-              sqlTypeName ) );
+          length = length / 2;
         }
+        field = QgsField( colName,
+                          sqlType,
+                          sqlTypeName,
+                          length );
+      }
+      else if ( sqlType == QVariant::Double )
+      {
+        field = QgsField( colName,
+                          sqlType,
+                          sqlTypeName,
+                          query.value( 6 ).toInt(),
+                          sqlTypeName == QLatin1String( "decimal" ) ? query.value( 8 ).toInt() : -1 );
+      }
+      else if ( sqlType == QVariant::Date || sqlType == QVariant::DateTime || sqlType == QVariant::Time )
+      {
+        field = QgsField( colName,
+                          sqlType,
+                          sqlTypeName,
+                          -1,
+                          -1 );
+      }
+      else
+      {
+        field = QgsField( colName,
+                          sqlType,
+                          sqlTypeName );
+      }
 
-        //COLUMN_DEF
-        if ( !query.value( 12 ).isNull() )
-        {
-          mDefaultValues.insert( i, query.value( 12 ).toString() );
-        }
+      // Field nullable
+      const bool nullable = query.value( QStringLiteral( "NULLABLE" ) ).toBool();
+      QgsFieldConstraints constraints;
+      if ( !nullable )
+        constraints.setConstraint( QgsFieldConstraints::ConstraintNotNull, QgsFieldConstraints::ConstraintOriginProvider );
+      field.setConstraints( constraints );
 
-        ++i;
+      mAttributeFields.append( field );
+
+      //COLUMN_DEF
+      if ( !query.value( 12 ).isNull() )
+      {
+        mDefaultValues.insert( i, query.value( 12 ).toString() );
       }
+
+      ++i;
     }
-    // get primary key
-    if ( mPrimaryKeyAttrs.isEmpty() )
+  }
+  // get primary key
+  if ( mPrimaryKeyAttrs.isEmpty() )
+  {
+    query.clear();
+    query.setForwardOnly( true );
+    if ( !query.exec( QStringLiteral( "exec sp_pkeys @table_name = N%1, @table_owner = %2 " ).arg( quotedValue( mTableName ), quotedValue( mSchemaName ) ) ) )
     {
-      query.clear();
-      query.setForwardOnly( true );
-      if ( !query.exec( QStringLiteral( "exec sp_pkeys @table_name = N%1, @table_owner = %2 " ).arg( quotedValue( mTableName ), quotedValue( mSchemaName ) ) ) )
-      {
-        QgsDebugMsg( QStringLiteral( "SQL:%1\n  Error:%2" ).arg( query.lastQuery(), query.lastError().text() ) );
-      }
+      QgsDebugMsg( QStringLiteral( "SQL:%1\n  Error:%2" ).arg( query.lastQuery(), query.lastError().text() ) );
+    }
 
-      if ( query.isActive() )
-      {
-        mPrimaryKeyType = PktInt;
+    if ( query.isActive() )
+    {
+      mPrimaryKeyType = PktInt;
 
-        while ( query.next() )
-        {
-          QString fidColName = query.value( 3 ).toString();
-          int idx = mAttributeFields.indexFromName( fidColName );
-          const QgsField &fld = mAttributeFields.at( idx );
+      while ( query.next() )
+      {
+        QString fidColName = query.value( 3 ).toString();
+        int idx = mAttributeFields.indexFromName( fidColName );
+        const QgsField &fld = mAttributeFields.at( idx );
 
-          if ( !mPrimaryKeyAttrs.isEmpty() ||
-               ( fld.type() != QVariant::Int &&
-                 fld.type() != QVariant::LongLong &&
-                 ( fld.type() != QVariant::Double || fld.precision() != 0 ) ) )
-            mPrimaryKeyType = PktFidMap;
+        if ( !mPrimaryKeyAttrs.isEmpty() ||
+             ( fld.type() != QVariant::Int &&
+               fld.type() != QVariant::LongLong &&
+               ( fld.type() != QVariant::Double || fld.precision() != 0 ) ) )
+          mPrimaryKeyType = PktFidMap;
 
-          mPrimaryKeyAttrs << idx;
-        }
+        mPrimaryKeyAttrs << idx;
+      }
 
-        if ( mPrimaryKeyAttrs.isEmpty() )
-        {
-          mPrimaryKeyType = PktUnknown;
-        }
+      if ( mPrimaryKeyAttrs.isEmpty() )
+      {
+        mPrimaryKeyType = PktUnknown;
       }
     }
+  }
 
-    if ( mPrimaryKeyAttrs.isEmpty() )
+  if ( mPrimaryKeyAttrs.isEmpty() )
+  {
+    const auto constPkCandidates = pkCandidates;
+    for ( const QString &pk : constPkCandidates )
     {
-      const auto constPkCandidates = pkCandidates;
-      for ( const QString &pk : constPkCandidates )
+      query.clear();
+      query.setForwardOnly( true );
+      if ( !query.exec( QStringLiteral( "select count(distinct [%1]), count([%1]) from [%2].[%3]" )
+                        .arg( pk, mSchemaName, mTableName ) ) )
       {
-        query.clear();
-        query.setForwardOnly( true );
-        if ( !query.exec( QStringLiteral( "select count(distinct [%1]), count([%1]) from [%2].[%3]" )
-                          .arg( pk, mSchemaName, mTableName ) ) )
-        {
-          QgsDebugMsg( QStringLiteral( "SQL:%1\n  Error:%2" ).arg( query.lastQuery(), query.lastError().text() ) );
-        }
+        QgsDebugMsg( QStringLiteral( "SQL:%1\n  Error:%2" ).arg( query.lastQuery(), query.lastError().text() ) );
+      }
 
-        if ( query.isActive() && query.next() && query.value( 0 ).toInt() == query.value( 1 ).toInt() )
-        {
-          mPrimaryKeyType = PktInt;
-          mPrimaryKeyAttrs << mAttributeFields.indexFromName( pk );
-          return;
-        }
+      if ( query.isActive() && query.next() && query.value( 0 ).toInt() == query.value( 1 ).toInt() )
+      {
+        mPrimaryKeyType = PktInt;
+        mPrimaryKeyAttrs << mAttributeFields.indexFromName( pk );
+        return;
       }
     }
+  }
 
-    if ( mPrimaryKeyAttrs.isEmpty() )
-    {
-      QString error = QStringLiteral( "No primary key could be found on table %1" ).arg( mTableName );
-      QgsDebugMsg( error );
-      mValid = false;
-      setLastError( error );
-    }
+  if ( mPrimaryKeyAttrs.isEmpty() )
+  {
+    QString error = QStringLiteral( "No primary key could be found on table %1" ).arg( mTableName );
+    QgsDebugMsg( error );
+    mValid = false;
+    setLastError( error );
+  }
 
-    if ( mPrimaryKeyAttrs.size() == 1 && !isIdentity )
-    {
-      // primary key has not null, unique constraints
-      QgsFieldConstraints constraints = mAttributeFields.at( mPrimaryKeyAttrs[0] ).constraints();
-      constraints.setConstraint( QgsFieldConstraints::ConstraintNotNull, QgsFieldConstraints::ConstraintOriginProvider );
-      constraints.setConstraint( QgsFieldConstraints::ConstraintUnique, QgsFieldConstraints::ConstraintOriginProvider );
-      mAttributeFields[ mPrimaryKeyAttrs[0] ].setConstraints( constraints );
-    }
+  if ( mPrimaryKeyAttrs.size() == 1 && !isIdentity )
+  {
+    // primary key has unique constraints
+    QgsFieldConstraints constraints = mAttributeFields.at( mPrimaryKeyAttrs[0] ).constraints();
+    constraints.setConstraint( QgsFieldConstraints::ConstraintUnique, QgsFieldConstraints::ConstraintOriginProvider );
+    mAttributeFields[ mPrimaryKeyAttrs[0] ].setConstraints( constraints );
   }
 }
 

diff --git a/tests/src/python/test_provider_mssql.py b/tests/src/python/test_provider_mssql.py
@@ -20,6 +20,7 @@
                        QgsFeature,
                        QgsField,
                        QgsFields,
+                       QgsFieldConstraints,
                        QgsDataSourceUri,
                        QgsWkbTypes,
                        QgsGeometry,
@@ -671,6 +672,42 @@ def testPktCompositeFloat(self):
 
         self.assertFalse(got_feature)
 
+    def testNotNullConstraint(self):
+        vl = QgsVectorLayer('%s table="qgis_test"."constraints" sql=' %
+                            (self.dbconn), "testdatetimes", "mssql")
+        self.assertTrue(vl.isValid())
+        self.assertEqual(len(vl.fields()), 4)
+
+        # test some bad field indexes
+        self.assertEqual(vl.dataProvider().fieldConstraints(-1),
+                         QgsFieldConstraints.Constraints())
+        self.assertEqual(vl.dataProvider().fieldConstraints(
+            1001), QgsFieldConstraints.Constraints())
+
+        self.assertTrue(vl.dataProvider().fieldConstraints(0) &
+                        QgsFieldConstraints.ConstraintNotNull)
+        self.assertFalse(vl.dataProvider().fieldConstraints(1)
+                         & QgsFieldConstraints.ConstraintNotNull)
+        self.assertTrue(vl.dataProvider().fieldConstraints(2) &
+                        QgsFieldConstraints.ConstraintNotNull)
+        self.assertFalse(vl.dataProvider().fieldConstraints(3)
+                         & QgsFieldConstraints.ConstraintNotNull)
+
+        # test that constraints have been saved to fields correctly
+        fields = vl.fields()
+        self.assertTrue(fields.at(0).constraints().constraints()
+                        & QgsFieldConstraints.ConstraintNotNull)
+        self.assertEqual(fields.at(0).constraints().constraintOrigin(QgsFieldConstraints.ConstraintNotNull),
+                         QgsFieldConstraints.ConstraintOriginProvider)
+        self.assertFalse(fields.at(1).constraints().constraints()
+                         & QgsFieldConstraints.ConstraintNotNull)
+        self.assertTrue(fields.at(2).constraints().constraints()
+                        & QgsFieldConstraints.ConstraintNotNull)
+        self.assertEqual(fields.at(2).constraints().constraintOrigin(QgsFieldConstraints.ConstraintNotNull),
+                         QgsFieldConstraints.ConstraintOriginProvider)
+        self.assertFalse(fields.at(3).constraints().constraints()
+                         & QgsFieldConstraints.ConstraintNotNull)
+
     def getSubsetString(self):
         return '[cnt] > 100 and [cnt] < 410'
 

diff --git a/tests/testdata/provider/testdata_mssql.sql b/tests/testdata/provider/testdata_mssql.sql
@@ -39,6 +39,9 @@ GO
 DROP TABLE IF EXISTS qgis_test.[tb_test_composite_float_pk];
 GO
 
+DROP TABLE IF EXISTS qgis_test.[constraints];
+GO
+
 DROP SCHEMA qgis_test;
 GO
 
@@ -285,3 +288,13 @@ INSERT INTO [qgis_test].[tb_test_composite_float_pk] (pk1, pk2, pk3, value, geom
     (2, 2, 2.718281828, 'test 3', geometry::STGeomFromText('POINT(-47.902 -15.763)', 4326)),
     (2, 2, 1.0,         'test 4', geometry::STGeomFromText('POINT(-47.952 -15.781)', 4326));
 GO
+
+-- Table for constraint tests
+CREATE TABLE [qgis_test].[constraints]
+(
+  gid integer PRIMARY KEY,
+  val int,
+  name text NOT NULL,
+  description text
+);
+GO