Merge pull request OSGeo#4978 from rouault/cmake_mssqlodbc_linux

CMake: add Linux support for mssqlodbc driver and fix a few bugs in the driver

.github/workflows/cmake_builds.yml

@@ -37,6 +37,12 @@ jobs:
            python3-dev libpython3-dev libpython3.8-dev python3.8-dev python3-numpy python3-lxml pyflakes python3-setuptools python3-pip python3-venv \
            python3-pytest swig doxygen texlive-latex-base make cppcheck ccache g++ \
            libpq-dev libpqtypes-dev postgresql-12 postgresql-12-postgis-3 postgresql-client-12 postgresql-12-postgis-3-scripts
+        # MSSQL: client side
+        curl https://packages.microsoft.com/keys/microsoft.asc | sudo apt-key add -
+        curl https://packages.microsoft.com/config/ubuntu/20.04/prod.list | sudo tee /etc/apt/sources.list.d/msprod.list
+        sudo apt-get update
+        sudo ACCEPT_EULA=Y apt-get install -y msodbcsql17 unixodbc-dev
+        #
         python3 -m pip install -U pip wheel setuptools numpy
         python3 -m pip install -r $GITHUB_WORKSPACE/autotest/requirements.txt
     - name: Configure

autotest/ogr/ogr_mssqlspatial.py

@@ -162,8 +162,10 @@ def test_ogr_mssqlspatial_3():
                                           max_error=0.001) == 0)
 
         for fld in range(3):
-            assert orig_feat.GetField(fld) == read_feat.GetField(fld), \
-                ('Attribute %d does not match' % fld)
+            if orig_feat.GetField(fld) != read_feat.GetField(fld):
+                orig_feat.DumpReadable()
+                read_feat.DumpReadable()
+                assert False, ('Attribute %d does not match' % fld)
         assert read_feat.GetField('INT64') == 1234567890123
 
         read_feat = None
@@ -191,6 +193,12 @@ def test_ogr_mssqlspatial_4():
     if gdaltest.mssqlspatial_has_z_m:
         wkt_list.append('3d_1')
 
+    use_bcp = 'BCP' in gdal.GetDriverByName('MSSQLSpatial').GetMetadataItem(gdal.DMD_LONGNAME)
+    if use_bcp:
+        MSSQLSPATIAL_USE_BCP = gdal.GetConfigOption('MSSQLSPATIAL_USE_BCP', 'YES')
+        if MSSQLSPATIAL_USE_BCP.upper() in ('NO', 'OFF', 'FALSE'):
+            use_bcp = False
+
     for item in wkt_list:
         wkt_filename = 'data/wkb_wkt/' + item + '.wkt'
         wkt = open(wkt_filename).read()
@@ -207,11 +215,12 @@ def test_ogr_mssqlspatial_4():
                                  'from file "' + wkt_filename + '"')
 
         ######################################################################
-        # Before reading back the record, verify that the newly added feature 
+        # Before reading back the record, verify that the newly added feature
         # is returned from the CreateFeature method with a newly assigned FID.
-
-        assert dst_feat.GetFID() != -1, \
-            'Assigned FID was not returned in the new feature'
+
+        if not use_bcp:
+            assert dst_feat.GetFID() != -1, \
+                'Assigned FID was not returned in the new feature'
 
         ######################################################################
         # Read back the feature and get the geometry.
@@ -275,14 +284,8 @@ def test_ogr_mssqlspatial_create_feature_in_unregistered_table():
 
     # Create a new MSSQLSpatial data source, one that will find the table just
     # created and make it available via GetLayerByName()
-    use_geometry_columns = gdal.GetConfigOption(
-        'MSSQLSPATIAL_USE_GEOMETRY_COLUMNS')
-    gdal.SetConfigOption('MSSQLSPATIAL_USE_GEOMETRY_COLUMNS', 'NO')
-
-    test_ds = ogr.Open(gdaltest.mssqlspatial_dsname, update=1)
-
-    gdal.SetConfigOption('MSSQLSPATIAL_USE_GEOMETRY_COLUMNS',
-                         use_geometry_columns)
+    with gdaltest.config_option('MSSQLSPATIAL_USE_GEOMETRY_COLUMNS', 'NO'):
+        test_ds = ogr.Open(gdaltest.mssqlspatial_dsname, update=1)
 
     assert test_ds is not None, 'cannot open data source'
 

cmake/modules/packages/FindMSSQL_ODBC.cmake

@@ -32,38 +32,62 @@ This module defines the following variables:
 
 #]=======================================================================]
 
-if(NOT WIN32)
-    return()
-endif()
+if(WIN32)
+    if(NOT DEFINED MSSQL_ODBC_VERSION)
+        set(MSSQL_ODBC_VERSION_CANDIDATES 17 13)
+        foreach(_vers IN LISTS MSSQL_ODBC_VERSION_CANDIDATES)
+          set(_dir "C:/Program Files/Microsoft SQL Server/Client SDK/ODBC/${_vers}0/SDK")
+          if(EXISTS "${_dir}")
+            set(MSSQL_ODBC_VERSION "${_vers}")
+            break()
+          endif()
+        endforeach()
+    endif()
 
-if(NOT DEFINED MSSQL_ODBC_VERSION)
-    set(MSSQL_ODBC_VERSION_CANDIDATES 17 13)
-    foreach(_vers IN LISTS MSSQL_ODBC_VERSION_CANDIDATES)
-      set(_dir "C:/Program Files/Microsoft SQL Server/Client SDK/ODBC/${_vers}0/SDK")
-      if(EXISTS "${_dir}")
-        set(MSSQL_ODBC_VERSION "${_vers}")
-        break()
-      endif()
-    endforeach()
-endif()
+    if(NOT DEFINED MSSQL_ODBC_ROOT)
+        set(MSSQL_ODBC_ROOT "C:/Program Files/Microsoft SQL Server/Client SDK/ODBC/${MSSQL_ODBC_VERSION}0/SDK")
+    endif()
 
-if(NOT DEFINED MSSQL_ODBC_ROOT)
-    set(MSSQL_ODBC_ROOT "C:/Program Files/Microsoft SQL Server/Client SDK/ODBC/${MSSQL_ODBC_VERSION}0/SDK")
-endif()
+    find_path(MSSQL_ODBC_INCLUDE_DIR NAMES msodbcsql.h
+              PATHS "${MSSQL_ODBC_ROOT}/Include")
+    mark_as_advanced(MSSQL_ODBC_INCLUDE_DIR)
 
-find_path(MSSQL_ODBC_INCLUDE_DIR NAMES msodbcsql.h
-          PATHS "${MSSQL_ODBC_ROOT}/Include")
-mark_as_advanced(MSSQL_ODBC_INCLUDE_DIR)
+    if("${CMAKE_SYSTEM_PROCESSOR}" MATCHES "AMD64")
+        set(MSSQL_ODBC_DIR_ARCH x64)
+    else()
+        set(MSSQL_ODBC_DIR_ARCH x86)
+    endif()
 
-if("${CMAKE_SYSTEM_PROCESSOR}" MATCHES "AMD64")
-    set(MSSQL_ODBC_DIR_ARCH x64)
+    find_library(MSSQL_ODBC_LIBRARY NAMES "msodbcsql${MSSQL_ODBC_VERSION}.lib"
+                 PATHS "${MSSQL_ODBC_ROOT}/Lib/${MSSQL_ODBC_DIR_ARCH}")
+    mark_as_advanced(MSSQL_ODBC_LIBRARY)
 else()
-    set(MSSQL_ODBC_DIR_ARCH x86)
-endif()
+    if(NOT DEFINED MSSQL_ODBC_VERSION)
+        set(MSSQL_ODBC_VERSION_CANDIDATES 17 13)
+        foreach(_vers IN LISTS MSSQL_ODBC_VERSION_CANDIDATES)
+          set(_dir "/opt/microsoft/msodbcsql${_vers}")
+          if(EXISTS "${_dir}")
+            set(MSSQL_ODBC_VERSION "${_vers}")
+            break()
+          endif()
+        endforeach()
+    endif()
+
+    if(NOT DEFINED MSSQL_ODBC_ROOT)
+        set(MSSQL_ODBC_ROOT "/opt/microsoft/msodbcsql${MSSQL_ODBC_VERSION}")
+    endif()
 
-find_library(MSSQL_ODBC_LIBRARY NAMES "msodbcsql${MSSQL_ODBC_VERSION}.lib"
-             PATHS "${MSSQL_ODBC_ROOT}/Lib/${MSSQL_ODBC_DIR_ARCH}")
-mark_as_advanced(MSSQL_ODBC_LIBRARY)
+    find_path(MSSQL_ODBC_INCLUDE_DIR NAMES msodbcsql.h
+              PATHS "${MSSQL_ODBC_ROOT}/include")
+    mark_as_advanced(MSSQL_ODBC_INCLUDE_DIR)
+
+    file(GLOB _libs "${MSSQL_ODBC_ROOT}/lib*/libmsodbc*")
+    list(LENGTH _libs _libs_length)
+    if(_libs_length EQUAL "1")
+        set(MSSQL_ODBC_LIBRARY ${_libs})
+    endif()
+    mark_as_advanced(MSSQL_ODBC_LIBRARY)
+endif()
 
 include(FindPackageHandleStandardArgs)
 find_package_handle_standard_args(MSSQL_ODBC

ogr/ogrsf_frmts/mssqlspatial/ogr_mssqlspatial.h

@@ -292,13 +292,16 @@ class OGRMSSQLSpatialLayer CPL_NON_FINAL: public OGRLayer
     char               *pszFIDColumn = nullptr;
     int                nFIDColumnIndex = -1;
 
+    // UUID doesn't work for now in bulk copy mode
+    bool               m_bHasUUIDColumn = false;
+
     int                bIsIdentityFid = FALSE;
 
     int                nLayerStatus = MSSQLLAYERSTATUS_ORIGINAL;
 
     int                *panFieldOrdinals = nullptr;
 
-    CPLErr              BuildFeatureDefn( const char *pszLayerName,
+    void               BuildFeatureDefn( const char *pszLayerName,
                                           CPLODBCStatement *poStmt );
 
     virtual CPLODBCStatement *  GetStatement() { return poStmt; }
@@ -428,6 +431,7 @@ class OGRMSSQLSpatialTableLayer final: public OGRMSSQLSpatialLayer
     virtual const char* GetName() override;
 
     virtual OGRErr      SetAttributeFilter( const char * ) override;
+    virtual OGRFeature *GetNextFeature() override;
 
     virtual OGRErr      ISetFeature( OGRFeature *poFeature ) override;
     virtual OGRErr      DeleteFeature( GIntBig nFID ) override;

ogr/ogrsf_frmts/mssqlspatial/ogrmssqlspatialdatasource.cpp

@@ -280,9 +280,6 @@ OGRErr OGRMSSQLSpatialDataSource::DeleteLayer( int iLayer )
              sizeof(void *) * (nLayers - iLayer - 1) );
     nLayers--;
 
-    if ( strlen(pszTableName) == 0 )
-        return OGRERR_NONE;
-
 /* -------------------------------------------------------------------- */
 /*      Remove from the database.                                       */
 /* -------------------------------------------------------------------- */

ogr/ogrsf_frmts/mssqlspatial/ogrmssqlspatiallayer.cpp

@@ -75,7 +75,7 @@ OGRMSSQLSpatialLayer::~OGRMSSQLSpatialLayer()
 /*      set on a statement.  Sift out geometry and FID fields.          */
 /************************************************************************/
 
-CPLErr OGRMSSQLSpatialLayer::BuildFeatureDefn( const char *pszLayerName,
+void OGRMSSQLSpatialLayer::BuildFeatureDefn( const char *pszLayerName,
                                     CPLODBCStatement *poStmtIn )
 
 {
@@ -242,6 +242,7 @@ CPLErr OGRMSSQLSpatialLayer::BuildFeatureDefn( const char *pszLayerName,
                 break;
 
             case SQL_C_GUID:
+                m_bHasUUIDColumn = true;
                 oField.SetType( OFTString );
                 oField.SetSubType( OFSTUUID );
                 break;
@@ -310,8 +311,6 @@ CPLErr OGRMSSQLSpatialLayer::BuildFeatureDefn( const char *pszLayerName,
     else
         CPLDebug( "OGR_MSSQLSpatial", "Table %s has no identified FID column.",
                   poFeatureDefn->GetName() );
-
-    return CE_None;
 }
 
 /************************************************************************/

ogr/ogrsf_frmts/mssqlspatial/ogrmssqlspatialtablelayer.cpp

@@ -141,6 +141,7 @@ OGRFeatureDefn* OGRMSSQLSpatialTableLayer::GetLayerDefn()
     if( oGetKey.GetPrimaryKeys( pszTableName, poDS->GetCatalog(), pszSchemaName )
         && oGetKey.Fetch() )
     {
+        CPLFree(pszFIDColumn);
         pszFIDColumn = CPLStrdup(oGetKey.GetColData( 3 ));
 
         if( oGetKey.Fetch() ) // more than one field in key!
@@ -158,14 +159,15 @@ OGRFeatureDefn* OGRMSSQLSpatialTableLayer::GetLayerDefn()
 /*      Get the column definitions for this table.                      */
 /* -------------------------------------------------------------------- */
     CPLODBCStatement oGetCol( poSession );
-    CPLErr eErr;
 
     if( !oGetCol.GetColumns( pszTableName, poDS->GetCatalog(), pszSchemaName ) )
-        return nullptr;
+    {
+        poFeatureDefn = new OGRFeatureDefn();
+        poFeatureDefn->Reference();
+        return poFeatureDefn;
+    }
 
-    eErr = BuildFeatureDefn( pszLayerName, &oGetCol );
-    if( eErr != CE_None )
-        return nullptr;
+    BuildFeatureDefn( pszLayerName, &oGetCol );
 
     if (eGeomType != wkbNone)
         poFeatureDefn->SetGeomType(eGeomType);
@@ -179,7 +181,7 @@ OGRFeatureDefn* OGRMSSQLSpatialTableLayer::GetLayerDefn()
         CPLError( CE_Failure, CPLE_AppDefined,
                   "No column definitions found for table '%s', layer not usable.",
                   pszLayerName );
-        return nullptr;
+        return poFeatureDefn;
     }
 
 /* -------------------------------------------------------------------- */
@@ -672,6 +674,8 @@ OGRFeature *OGRMSSQLSpatialTableLayer::GetFeature( GIntBig nFeatureId )
 
 OGRErr OGRMSSQLSpatialTableLayer::GetExtent(int iGeomField, OGREnvelope *psExtent, int bForce)
 {
+    GetLayerDefn();
+
     // Make sure we have a geometry field:
     if (iGeomField < 0 || iGeomField >= poFeatureDefn->GetGeomFieldCount() ||
         poFeatureDefn->GetGeomFieldDefn(iGeomField)->GetType() == wkbNone)
@@ -778,6 +782,16 @@ OGRErr OGRMSSQLSpatialTableLayer::SetAttributeFilter( const char *pszQueryIn )
     return OGRERR_NONE;
 }
 
+/************************************************************************/
+/*                           GetNextFeature()                           */
+/************************************************************************/
+
+OGRFeature *OGRMSSQLSpatialTableLayer::GetNextFeature()
+{
+    poDS->EndCopy();
+    return OGRMSSQLSpatialLayer::GetNextFeature();
+}
+
 /************************************************************************/
 /*                           TestCapability()                           */
 /************************************************************************/
@@ -925,7 +939,10 @@ OGRErr OGRMSSQLSpatialTableLayer::CreateField( OGRFieldDefn *poFieldIn,
     else if( oField.GetType() == OFTString )
     {
         if( oField.GetSubType() == OGRFieldSubType::OFSTUUID)
+        {
+            m_bHasUUIDColumn = true;
             strcpy( szFieldType, "uniqueidentifier" );
+        }
         else if( oField.GetWidth() == 0 || oField.GetWidth() > 4000 || !bPreservePrecision )
             strcpy( szFieldType, "nvarchar(MAX)" );
         else
@@ -1669,11 +1686,11 @@ OGRErr OGRMSSQLSpatialTableLayer::CreateFeatureBCP( OGRFeature *poFeature )
     {
         if (iCol == nGeomColumnIndex)
         {
-            if (poFeature->GetGeometryRef())
+            OGRGeometry *poGeom = poFeature->GetGeometryRef();
+            if (poGeom != nullptr)
             {
                 /* prepare geometry */
-                OGRGeometry *poGeom = poFeature->GetGeometryRef();
-                if (bUseGeometryValidation  && poGeom != nullptr)
+                if (bUseGeometryValidation)
                 {
                     OGRMSSQLGeometryValidator oValidator(poGeom, nGeomColumnType);
                     if (!oValidator.IsValid())
@@ -1684,7 +1701,17 @@ OGRErr OGRMSSQLSpatialTableLayer::CreateFeatureBCP( OGRFeature *poFeature )
                     }
                 }
 
-                OGRMSSQLGeometryWriter poWriter(poGeom, nGeomColumnType, nSRSId);
+                int nOutgoingSRSId = 0;
+                // Use the SRID specified by the provided feature's geometry, if
+                // its spatial-reference system is known; otherwise, use the SRID
+                // associated with the table
+                OGRSpatialReference *poFeatureSRS = poGeom->getSpatialReference();
+                if (poFeatureSRS)
+                    nOutgoingSRSId = poDS->FetchSRSId(poFeatureSRS);
+                if (nOutgoingSRSId <= 0)
+                    nOutgoingSRSId = nSRSId;
+
+                OGRMSSQLGeometryWriter poWriter(poGeom, nGeomColumnType, nOutgoingSRSId);
                 papstBindBuffer[iCol]->RawData.nSize = poWriter.GetDataLen();
                 papstBindBuffer[iCol]->RawData.pData = (GByte *) CPLMalloc(papstBindBuffer[iCol]->RawData.nSize + 1);
 
@@ -1798,9 +1825,17 @@ OGRErr OGRMSSQLSpatialTableLayer::CreateFeatureBCP( OGRFeature *poFeature )
                     else
                     {
 
-                        papstBindBuffer[iCol]->VarChar.nSize = (SQLLEN)CPLStrlenUTF8(poFeature->GetFieldAsString(iField)) * 2;
                         wchar_t* buffer = CPLRecodeToWChar( poFeature->GetFieldAsString(iField), CPL_ENC_UTF8, CPL_ENC_UCS2);
-                        memcpy(papstBindBuffer[iCol]->VarChar.pData, buffer, papstBindBuffer[iCol]->VarChar.nSize + 2);
+                        const auto nLen = wcslen(buffer);
+                        papstBindBuffer[iCol]->VarChar.nSize = (SQLLEN)nLen * sizeof(GUInt16);
+#if WCHAR_MAX > 0xFFFFu
+                        // Shorten each character to a two-byte value, as expected by
+                        // the ODBC driver
+                        GUInt16 *panBuffer = reinterpret_cast<GUInt16 *>(buffer);
+                        for( unsigned int nIndex = 1; nIndex <= nLen; nIndex += 1 )
+                            panBuffer[nIndex] = static_cast<GUInt16>(buffer[nIndex]);
+#endif
+                        memcpy(papstBindBuffer[iCol]->VarChar.pData, buffer, papstBindBuffer[iCol]->VarChar.nSize + sizeof(GUInt16));
                         CPLFree(buffer);
 
                         if (Failed2( bcp_collen( hDBCBCP, (DBINT)papstBindBuffer[iCol]->VarChar.nSize, iCol + 1) ))
@@ -1978,10 +2013,19 @@ OGRErr OGRMSSQLSpatialTableLayer::CreateFeatureBCP( OGRFeature *poFeature )
                 {
                     if (poFeature->IsFieldSetAndNotNull( iField ))
                     {
-                        papstBindBuffer[iCol]->VarChar.nSize = (SQLLEN)CPLStrlenUTF8(poFeature->GetFieldAsString(iField)) * 2;
-                        if (papstBindBuffer[iCol]->VarChar.nSize > 0)
+                        const char* pszStr = poFeature->GetFieldAsString(iField);
+                        if (pszStr[0] != 0)
                         {
                             wchar_t* buffer = CPLRecodeToWChar( poFeature->GetFieldAsString(iField), CPL_ENC_UTF8, CPL_ENC_UCS2);
+                            const auto nLen = wcslen(buffer);
+                            papstBindBuffer[iCol]->VarChar.nSize = (SQLLEN)nLen * sizeof(GUInt16);
+#if WCHAR_MAX > 0xFFFFu
+                            // Shorten each character to a two-byte value, as expected by
+                            // the ODBC driver
+                            GUInt16 *panBuffer = reinterpret_cast<GUInt16 *>(buffer);
+                            for( unsigned int nIndex = 1; nIndex <= nLen; nIndex += 1 )
+                                panBuffer[nIndex] = static_cast<GUInt16>(buffer[nIndex]);
+#endif
                             if (Failed2( bcp_moretext( hDBCBCP,
                                 (DBINT)papstBindBuffer[iCol]->VarChar.nSize,
                                 (LPCBYTE)buffer ) ))
@@ -2075,7 +2119,7 @@ OGRErr OGRMSSQLSpatialTableLayer::ICreateFeature( OGRFeature *poFeature )
     }
 
 #if (ODBCVER >= 0x0300) && defined(MSSQL_BCP_SUPPORTED)
-    if (bUseCopy)
+    if (bUseCopy && !m_bHasUUIDColumn)
     {
         return CreateFeatureBCP( poFeature );
     }
@@ -2211,9 +2255,9 @@ OGRErr OGRMSSQLSpatialTableLayer::ICreateFeature( OGRFeature *poFeature )
                     if ((!poSession->Failed( SQLBindParameter(oStatement.GetStatement(), (SQLUSMALLINT)(bind_num + 1),
                         SQL_PARAM_INPUT, SQL_C_BINARY, SQL_SS_UDT,
                         SQL_SS_LENGTH_UNLIMITED, 0, (SQLPOINTER)pabyData, bind_datalen[bind_num], (SQLLEN*)&bind_datalen[bind_num])))
-                        && (!poSession->Failed(SQLGetStmtAttr(oStatement.GetStatement(), SQL_ATTR_IMP_PARAM_DESC, &ipd, 0, 0)))
+                        && (!poSession->Failed(SQLGetStmtAttr(oStatement.GetStatement(), SQL_ATTR_IMP_PARAM_DESC, &ipd, 0, nullptr)))
                         && (!poSession->Failed(SQLSetDescField(ipd, 1, SQL_CA_SS_UDT_TYPE_NAME,
-                            (nGeomColumnType == MSSQLCOLTYPE_GEOGRAPHY? "geography" : "geometry"), SQL_NTS))))
+                            const_cast<char*>(nGeomColumnType == MSSQLCOLTYPE_GEOGRAPHY? "geography" : "geometry"), SQL_NTS))))
                     {
                         oStatement.Append( "?" );
                         bind_buffer[bind_num] = pabyData;