Opened 9 years ago

Closed 8 years ago

Last modified 8 years ago

#6191 closed enhancement (fixed)

Add IBM DB2 spatial support to OGR

Reported by: dadler Owned by: dadler
Priority: normal Milestone: 2.1.0
Component: OGR_SF Version: svn-trunk
Severity: normal Keywords: IBM DB2 OGR
Cc:

Description (last modified by dadler)

Many relational databases with spatial support have GDAL/OGR drivers but one doesn't currently exist for the IBM DB2 database.

A customer has requested this support and in response we have created a first version of this which support the DB2 LUW and DB2 for z/OS products.

So far this has been tested with, test_ogrsf, ogrinfo, ogr2ogr to import a shapefile to DB2, ogr2ogr to export a shapefile from DB2, and a GDAL autotest Python script

The attached file contains an 'svn diff' of the code changes and additions. All of the DB2 driver code should go in ogr/ogrsf_frmts/db2.

The following files outside the DB2 driver were changed:

apps/test_ogrsf.cpp

  • add code to check for DB2 driver
  • change HUGE_COORDS from 1e300 to 1e70 as the largest floating pt number on z/OS is around 1e75. If this change isn't acceptable, it will just result in the huge coords test failing

ogr/ogrsf_frmts/generic/makefile.vc

  • add DB2_ENABLED format variable

ogr/ogrsf_frmts/makefile.vc

  • add DB2 support

ogr/ogrsf_frmts/generic/ogrregisterall.cpp

  • add call to register DB2 driver

ogr/ogrsf_frmts/ogrsf_frmts.h

  • add DB2 register function

The current driver is in db2.patch. Ignore db2.zip (I don't know how to delete this)

A DB2 test for autotest is in db2_test.patch.

Attachments (3)

db2.zip (28.9 KB ) - added by dadler 9 years ago.
db2_test.patch (13.7 KB ) - added by dadler 8 years ago.
DB2 autotest and database setup script
db2.patch (169.6 KB ) - added by dadler 8 years ago.
DB2 driver as of 2015-11-15

Download all attachments as: .zip

Change History (40)

by dadler, 9 years ago

Attachment: db2.zip added

comment:1 by dadler, 9 years ago

Description: modified (diff)

comment:2 by dadler, 9 years ago

Description: modified (diff)

comment:3 by dadler, 8 years ago

Description: modified (diff)

comment:4 by dadler, 8 years ago

Description: modified (diff)

comment:5 by dadler, 8 years ago

Description: modified (diff)

comment:6 by Mateusz Łoskot, 8 years ago

Note regarding the HUGE_COORDS change, I guess it might be not accepted. Instead, it might be possible to replace the compile-time define with value based on driver (e.g. 1e300 as default, while for DB2 driver it would be 1e70).

Still, not ideal but better than changing current test which worked.

in reply to:  6 comment:7 by Even Rouault, 8 years ago

Replying to mloskot:

Note regarding the HUGE_COORDS change, I guess it might be not accepted. Instead, it might be possible to replace the compile-time define with value based on driver (e.g. 1e300 as default, while for DB2 driver it would be 1e70).

Still, not ideal but better than changing current test which worked.

Or clamp values in the implementation of SetSpatialFilter() in the DB2 driver. The purpose of 1e300 was to detect buffer overflows of the kind 'char szbuffer[80]; sprintf(szBuffer, "%f", x)'

comment:8 by Mateusz Łoskot, 8 years ago

I have done brief review of DB2 driver, a walk through ReviewerChecklist and rfc8_devguide but without any attempt to build or run it.


  1. Whitespaces

Use spaces instead of hard tab characters in source code. There is mixture of both in the submitted files. Also, hard tabs affect indentation.

I'm not sure how hard is the "Use 4 character indentation levels." rule of RFC8 but there are places where indentation could be correct, e.g.:

+    virtual void        ResetReading();
+    virtual GIntBig         GetFeatureCount( int );

Also, there are trailing spaces which shouldn't be there, like after declaration of AppendFieldValue in OGRDB2TableLayer class.

  1. drv_db2.html
  • Not linked from ogr/ogrsf_frtms/ogr_formats.html
  • AFAIU, currently the driver is Windows-only, it should be mentioned.
  • It may be good to add note on installation/building and third-party software required, if any.
  1. I would avoid unnecessary #define's in ogr_db2.h and move to .cpp file, especially that DB2LAYERSTATUS_* seems to be only used in ogrdb2datasource.cpp
  1. The convention for variable names is to capitalize each word in a variable name.

There are places that that need to be corrected, e.g.

+    void                AppendFieldValue(CPLODBCStatement *poStatement,
+                                       OGRFeature* poFeature, int i, int *bind_num, void **bind_buffer);
  1. Perhaps, int used for boolean variables could be changed to C++ bool where possible (i.e. not part of public interface), then it would save Kurt Schwehr's on refactoring work.
  1. See my previous comment to HUGE_COORDS change and Even's answer.
  1. Try to keep lines to 79 characters or less.

There are lines exceeding even 160 characters.

  1. Avoid leading underscore in preprocessor #define's.

For example here, leading underscore is C/C++ reserved it does not conform ot C/C++ standard http://www.gnu.org/software/libc/manual/html_node/Reserved-Names.html Not sure how GDAL is strong about this, but _OGR_DB2_H_INCLUDED better reads e.g. GDAL_OGR_DB2_H_INCLUDED

  1. ogr/ogrsf_frmts/generic/makefile.vc

I understand that the driver does not depend on any thirdpardy software, hence the compilation is enabled by default.

  1. Is OGRDB2Driver class needed at all?
  1. End-of-line characters should be the Unix way (\n)

AFAICT, there are CRLF, so it should be replaced with LF.

  1. db2_test.py

The driver is Windows-only, so perhaps it should be skipped with simple check if os.name != 'nt', though I don't know what is the recommended way to do that in autotest.


I have no more comments. If I have a chance to build/run it, I will let you know. Meanwhile, I see no reason not to accept it.

comment:9 by Mateusz Łoskot, 8 years ago

David,

Also, since GDAL AppVeyor setup builds with Visual C++ flag /WX, it is important to ensure the code compiles warnings-free.

I will do my best to build it myself too, once updated patch is attached.

comment:10 by dadler, 8 years ago

Thank you for the review and comments.

I will update the patch.

Did you use some tools to check the tabs, line ending and line lengths? Are there some tools to fix this? I can certainly write a Perl script to fix it but don't need to recreate the wheel.

Is there a preferred or most common Linux distribution that is used for GDAL?

in reply to:  8 comment:11 by dadler, 8 years ago

Replying to mloskot:

Regarding the leading underscore, all the OGR .h files appear to use this approach, although I can certainly make the change.

I have done brief review of DB2 driver, a walk through ReviewerChecklist and rfc8_devguide but without any attempt to build or run it.


  1. Whitespaces

Use spaces instead of hard tab characters in source code. There is mixture of both in the submitted files. Also, hard tabs affect indentation.

I'm not sure how hard is the "Use 4 character indentation levels." rule of RFC8 but there are places where indentation could be correct, e.g.:

+    virtual void        ResetReading();
+    virtual GIntBig         GetFeatureCount( int );

Also, there are trailing spaces which shouldn't be there, like after declaration of AppendFieldValue in OGRDB2TableLayer class.

  1. drv_db2.html
  • Not linked from ogr/ogrsf_frtms/ogr_formats.html
  • AFAIU, currently the driver is Windows-only, it should be mentioned.
  • It may be good to add note on installation/building and third-party software required, if any.
  1. I would avoid unnecessary #define's in ogr_db2.h and move to .cpp file, especially that DB2LAYERSTATUS_* seems to be only used in ogrdb2datasource.cpp
  1. The convention for variable names is to capitalize each word in a variable name.

There are places that that need to be corrected, e.g.

+    void                AppendFieldValue(CPLODBCStatement *poStatement,
+                                       OGRFeature* poFeature, int i, int *bind_num, void **bind_buffer);
  1. Perhaps, int used for boolean variables could be changed to C++ bool where possible (i.e. not part of public interface), then it would save Kurt Schwehr's on refactoring work.
  1. See my previous comment to HUGE_COORDS change and Even's answer.
  1. Try to keep lines to 79 characters or less.

There are lines exceeding even 160 characters.

  1. Avoid leading underscore in preprocessor #define's.

For example here, leading underscore is C/C++ reserved it does not conform ot C/C++ standard http://www.gnu.org/software/libc/manual/html_node/Reserved-Names.html Not sure how GDAL is strong about this, but _OGR_DB2_H_INCLUDED better reads e.g. GDAL_OGR_DB2_H_INCLUDED

  1. ogr/ogrsf_frmts/generic/makefile.vc

I understand that the driver does not depend on any thirdpardy software, hence the compilation is enabled by default.

  1. Is OGRDB2Driver class needed at all?
  1. End-of-line characters should be the Unix way (\n)

AFAICT, there are CRLF, so it should be replaced with LF.

  1. db2_test.py

The driver is Windows-only, so perhaps it should be skipped with simple check if os.name != 'nt', though I don't know what is the recommended way to do that in autotest.


I have no more comments. If I have a chance to build/run it, I will let you know. Meanwhile, I see no reason not to accept it.

in reply to:  10 comment:12 by Mateusz Łoskot, 8 years ago

Replying to dadler:

Did you use some tools to check the tabs, line ending and line lengths?

Since you are targeting the driver for Windows first, I assume you are developing with Visual Studio. And, since the number of source files is not large, you an simply use Untabify selected lines command.

The trailing whitespaces can be auto-removed with Trailing Whitespace Visualizer extension.

Finally, change VS options to Insert Spaces for C/C++ language.

I hope it helps.

Is there a preferred or most common Linux distribution that is used for GDAL?

Possibly, simply, Ubuntu.

Replying to dadler:

Regarding the leading underscore, all the OGR .h files appear to use this approach, although I can certainly make the change.

Yes, I'm aware of the legacy code, that's why I said I'm "not sure how GDAL is strong about this" regarding new code. IMHO, it should be fixed.

in reply to:  8 comment:13 by dadler, 8 years ago

Replying to mloskot:

I have done brief review of DB2 driver, a walk through ReviewerChecklist and rfc8_devguide but without any attempt to build or run it.


  1. Whitespaces

Use spaces instead of hard tab characters in source code. There is mixture of both in the submitted files. Also, hard tabs affect indentation.

I'm not sure how hard is the "Use 4 character indentation levels." rule of RFC8 but there are places where indentation could be correct, e.g.:

+    virtual void        ResetReading();
+    virtual GIntBig         GetFeatureCount( int );

Also, there are trailing spaces which shouldn't be there, like after declaration of AppendFieldValue in OGRDB2TableLayer class.

I think I fixed all the tabs to spaces, trailing spaces and indentation.

  1. drv_db2.html
  • Not linked from ogr/ogrsf_frtms/ogr_formats.html
  • AFAIU, currently the driver is Windows-only, it should be mentioned.
  • It may be good to add note on installation/building and third-party software required, if any.

I modified ogr/ogrsf_frtms/ogr_formats.html to include DB2 but none of the links actually work.

  1. I would avoid unnecessary #define's in ogr_db2.h and move to .cpp file, especially that DB2LAYERSTATUS_* seems to be only used in ogrdb2datasource.cpp

Done.

  1. The convention for variable names is to capitalize each word in a variable name.

There are places that that need to be corrected, e.g.

+    void                AppendFieldValue(CPLODBCStatement *poStatement,
+                                       OGRFeature* poFeature, int i, int *bind_num, void **bind_buffer);

A number of the issues you mentioned relate to modifying the MS SQL Spatial driver to support DB2. I've fixed most issues except for the capitalization convention. I'm currently integrating raster image support and would like to incorporate this change with this effort.

  1. Perhaps, int used for boolean variables could be changed to C++ bool where possible (i.e. not part of public interface), then it would save Kurt Schwehr's on refactoring work.

Will consider.

  1. See my previous comment to HUGE_COORDS change and Even's answer.

Done - removed HUGE_COORDS change.

  1. Try to keep lines to 79 characters or less.

There are lines exceeding even 160 characters.

Fixed all of these. It does make it more readable. It is somewhat ironic that there are conventions associated with the size of a computer punched card and 24x80 "green screen" displays.

  1. Avoid leading underscore in preprocessor #define's.

For example here, leading underscore is C/C++ reserved it does not conform ot C/C++ standard http://www.gnu.org/software/libc/manual/html_node/Reserved-Names.html Not sure how GDAL is strong about this, but _OGR_DB2_H_INCLUDED better reads e.g. GDAL_OGR_DB2_H_INCLUDED

I'd rather be consistent with all the other OGR drivers that use the leading underscore.

  1. ogr/ogrsf_frmts/generic/makefile.vc

I understand that the driver does not depend on any thirdpardy software, hence the compilation is enabled by default.

Yes.

  1. Is OGRDB2Driver class needed at all?

Isn't the driver class what is registered? All the other drivers use a corresponding class.

  1. End-of-line characters should be the Unix way (\n)

AFAICT, there are CRLF, so it should be replaced with LF.

These should all be fixed.

  1. db2_test.py

The driver is Windows-only, so perhaps it should be skipped with simple check if os.name != 'nt', though I don't know what is the recommended way to do that in autotest.

Added the test for 'nt'.


I have no more comments. If I have a chance to build/run it, I will let you know. Meanwhile, I see no reason not to accept it.

by dadler, 8 years ago

Attachment: db2_test.patch added

DB2 autotest and database setup script

comment:14 by Mateusz Łoskot, 8 years ago

I've tried to build the driver with Visual Studio 2015 and there are two issues:

  1. The driver seems to include DB2 header, sqlcli1.h, whereas it was mentioned it is based on the ODBC. Has it changed?
cl   /nologo /MD /EHsc /Ox /FC /D_CRT_SECURE_NO_DEPRECATE /D_CRT_NONSTDC_NO_DEPRECATE /DNDEBUG /W4 /wd4127 /wd4251 /wd4275 /wd4786 /wd4100 /wd4245 /wd4206 /wd4611 /DHAVE_SSE_AT_COMPILE_TIME -I..\..\..\port -I..\..\..\ogr -I..\..\..\gcore  -I..\..\..\alg -I..\..\..\ogr\ogrsf_frmts  -I..\..\..\gnm -I..\..\..\gnm\gnm_frmts -I..\..\..\apps /DHAVE_AVX_AT_COMPILE_TIME -I.. -I..\.. -DOGR_ENABLED   -DGDAL_COMPILATION /c ogrdb2driver.cpp ogrdb2datasource.cpp ogrdb2layer.cpp ogrdb2tablelayer.cpp ogrdb2selectlayer.cpp
ogrdb2driver.cpp
d:\dev\gdal\trunk\gdal\ogr\ogrsf_frmts\db2\ogrdb2driver.cpp(29):
  fatal error C1083: Cannot open include file: 'sqlcli1.h': No such file or directory
  1. Compilation is not warnings free. Althought GDAL at large does not compile with VS2015 without warnings yet, this may be worth to address.
ogrdb2layer.cpp(93): warning C4458: declaration of 'poStmt' hides class member
ogr_db2.h(58): note: see declaration of 'OGRDB2Layer::poStmt'
ogrdb2layer.cpp(310): warning C4456: declaration of 'iField' hides previous local declaration
ogrdb2layer.cpp(271): note: see declaration of 'iField'
ogrdb2tablelayer.cpp(273): warning C4458: declaration of 'pszLayerName' hides class member
ogr_db2.h(143): note: see declaration of 'OGRDB2TableLayer::pszLayerName'
ogrdb2tablelayer.cpp(657): warning C4458: declaration of 'pszQuery' hides class member
ogr_db2.h(133): note: see declaration of 'OGRDB2TableLayer::pszQuery'

Other than that, it looks good to me and ready for submission.

comment:15 by dadler, 8 years ago

In regards to point #1. The include of sqlcli1.h is commented out in ogr_db2.h in the patch file that I uploaded. But I see that this is included redundantly and erroneously in ogrdb2driver.cpp. Should I upload another version with this removed?

In regards to #2, there were no warnings when I use nmake with VS 10.0. Is there some additional compiler flag to report these?

It would be helpful to have committer status for the db2-specific code. I wouldn't change anything else without the appropriate project controls. (At some point it would be nice to do some refactoring as I'm copying and duplicating a lot of code that has nothing to do with DB2.)

by dadler, 8 years ago

Attachment: db2.patch added

DB2 driver as of 2015-11-15

comment:16 by dadler, 8 years ago

Uploaded a new db2.patch removing the extraneous include.

I will work on the other warning messages as I apply the m_ convention to class member variables as part of the raster support.

comment:17 by Mateusz Łoskot, 8 years ago

David,

Thanks for removing the sqlcli1.h. Clean patch is always better.

Regarding the warnings, I did not try any different compilation flags.; Simply, later versions of compilers usually become more strict than previous ones. Microsoft has made a mile step in VS2015 in this regard.

I have requested commit access for you (http://lists.osgeo.org/pipermail/gdal-dev/2015-December/043210.html). Please, reply regarding the RFC3 matter. Hopefully, the PSC will vote to pass my motion.

comment:18 by Even Rouault, 8 years ago

For the record: GDAL now builds warning free with VS2015

in reply to:  18 ; comment:19 by dadler, 8 years ago

Replying to rouault:

For the record: GDAL now builds warning free with VS2015

I'm a bit confused.

Does this have to do with the DB2 driver or in general since the DB2 code is not yet integrated.

Should the next step for the DB2 driver be for someone to give me committer access and then I check in my changes associated with this ticket?

in reply to:  19 comment:20 by Even Rouault, 8 years ago

Does this have to do with the DB2 driver or in general since the DB2 code is not yet integrated.

In general (except in the DB2 driver that I've no tested) : https://ci.appveyor.com/project/rouault/gdal-coverage/history has a trunk_vc12 (VS 2013) and trunk_vc13 (VS 2015) branches with a minimal build (no external dependency)

Should the next step for the DB2 driver be for someone to give me committer access and then I check in my changes associated with this ticket?

Yes, I believe Mateusz should declare the motion passed soon and I'll grant you commit rights.

comment:21 by Mateusz Łoskot, 8 years ago

Owner: changed from David Adler to dadler

The motion passed (https://lists.osgeo.org/pipermail/gdal-dev/2015-December/043229.html).

David, once Even grants you with the commit access, you are welcome to submit your driver into the trunk and close the ticket :)

Thanks!

comment:22 by dadler, 8 years ago

OGR DB2 driver committed with revision 32093. DB2 autotest case committed with revision 32095.

comment:23 by dadler, 8 years ago

Resolution: fixed
Status: newclosed
Version: unspecifiedsvn-trunk

comment:24 by Even Rouault, 8 years ago

Resolution: fixed
Status: closedreopened
  • Build failure on error-on-warning VC12 amd64 build : https://ci.appveyor.com/project/rouault/gdal-coverage/build/1.0.1481
     C:\projects\gdal-coverage\gdal\ogr\ogrsf_frmts>cd db2 && nmake /nologo /f makefile.vc && cd .. || exit 1
    cl /Zi /Fd..\..\..\gdal201.pdb /nologo /MD /EHsc /FC /D_CRT_SECURE_NO_DEPRECATE /D_CRT_NONSTDC_NO_DEPRECATE /DDEBUG /W4 /wd4127 /wd4251 /wd4275 /wd4786 /wd4100 /wd4245 /wd4206 /wd4611 /WX /DHAVE_SSE_AT_COMPILE_TIME -I..\..\..\port -I..\..\..\ogr -I..\..\..\gcore -I..\..\..\alg -I..\..\..\ogr\ogrsf_frmts -I..\..\..\gnm -I..\..\..\gnm\gnm_frmts -I..\..\..\apps /DHAVE_AVX_AT_COMPILE_TIME -I.. -I..\.. -DOGR_ENABLED -DGDAL_COMPILATION /c ogrdb2driver.cpp ogrdb2datasource.cpp ogrdb2layer.cpp ogrdb2tablelayer.cpp ogrdb2selectlayer.cpp
    ogrdb2driver.cpp
    ogrdb2datasource.cpp
    c:\projects\gdal-coverage\gdal\ogr\ogrsf_frmts\db2\ogrdb2datasource.cpp(133) : error C2220: warning treated as error - no 'object' file generated 
    c:\projects\gdal-coverage\gdal\ogr\ogrsf_frmts\db2\ogrdb2datasource.cpp(133) : warning C4244: 'initializing' : conversion from '__int64' to 'int', possible loss of data 
    c:\projects\gdal-coverage\gdal\ogr\ogrsf_frmts\db2\ogrdb2datasource.cpp(265) : warning C4244: 'initializing' : conversion from '__int64' to 'int', possible loss of data 
    c:\projects\gdal-coverage\gdal\ogr\ogrsf_frmts\db2\ogrdb2datasource.cpp(495) : warning C4267: 'initializing' : conversion from 'size_t' to 'int', possible loss of data 
    c:\projects\gdal-coverage\gdal\ogr\ogrsf_frmts\db2\ogrdb2datasource.cpp(578) : warning C4267: '=' : conversion from 'size_t' to 'int', possible loss of data 
    c:\projects\gdal-coverage\gdal\ogr\ogrsf_frmts\db2\ogrdb2datasource.cpp(655) : warning C4267: 'initializing' : conversion from 'size_t' to 'int', possible loss of data 
    ogrdb2layer.cpp
    ogrdb2tablelayer.cpp
    ogrdb2selectlayer.cpp
    

comment:25 by Even Rouault, 8 years ago

You might want to remove the CPLDebug() traces in OGRDB2DriverIdentify() and OGRDB2DriverOpen() since they will be called for all datasources identification. We want to limit the verbosity at such generic level.

comment:26 by Even Rouault, 8 years ago

The i386 VC12 build at https://ci.appveyor.com/project/rouault/gdal-coverage/build/1.0.1481/job/ewwgdbxc3f3x0m45 succeeds but the autotest/cpp/gdal_unit_test.exe tests crashes with

cd autotest
cd cpp
nmake /f makefile.vc check MSVC_VER=1800 %WIN64_ARG%
 
Microsoft (R) Program Maintenance Utility Version 12.00.21005.1
Copyright (C) Microsoft Corporation. All rights reserved.
 
gdal_unit_test.exe
ERROR 7: Assertion `FALSE' failed
in file `c:\projects\gdal-coverage\gdal\gcore\gdaldrivermanager.cpp', line 405

This is the first time I see that... Hum, actually I think I know the reason. There's an inconsistency in ogrdb2driver.cpp :

    if (GDALGetDriverByName("DB2") != NULL) return;

    poDriver = new GDALDriver();
    poDriver->SetDescription( "DB2ODBC" );

The string tested in GDALGetDriverByName() and in SetDescription() should be the same so as to detect driver re-registration correctly, which must be what is tested here.

comment:27 by Even Rouault, 8 years ago

Linux builds also fail on tests https://s3.amazonaws.com/archive.travis-ci.org/jobs/95619543/log.txt

Running tests from ogr/ogr_db2.py
  TEST: ogr_db2_check_driver ... fail

--> this should return 'skip' to avoid job failure

comment:28 by Even Rouault, 8 years ago

Not critical, but "poDriver->SetMetadataItem( GDAL_DCAP_RASTER, "YES" );" is not appropriate given the current capabilities of the driver.

comment:29 by Kurt Schwehr, 8 years ago

Please set the test to executable so that people can run just that test. e.g.

Error: Failed to load processor bash
No macro or processor named 'bash' found

in reply to:  24 ; comment:30 by dadler, 8 years ago

Replying to rouault:

Fixed.

  • Build failure on error-on-warning VC12 amd64 build : https://ci.appveyor.com/project/rouault/gdal-coverage/build/1.0.1481
     C:\projects\gdal-coverage\gdal\ogr\ogrsf_frmts>cd db2 && nmake /nologo /f makefile.vc && cd .. || exit 1
    cl /Zi /Fd..\..\..\gdal201.pdb /nologo /MD /EHsc /FC /D_CRT_SECURE_NO_DEPRECATE /D_CRT_NONSTDC_NO_DEPRECATE /DDEBUG /W4 /wd4127 /wd4251 /wd4275 /wd4786 /wd4100 /wd4245 /wd4206 /wd4611 /WX /DHAVE_SSE_AT_COMPILE_TIME -I..\..\..\port -I..\..\..\ogr -I..\..\..\gcore -I..\..\..\alg -I..\..\..\ogr\ogrsf_frmts -I..\..\..\gnm -I..\..\..\gnm\gnm_frmts -I..\..\..\apps /DHAVE_AVX_AT_COMPILE_TIME -I.. -I..\.. -DOGR_ENABLED -DGDAL_COMPILATION /c ogrdb2driver.cpp ogrdb2datasource.cpp ogrdb2layer.cpp ogrdb2tablelayer.cpp ogrdb2selectlayer.cpp
    ogrdb2driver.cpp
    ogrdb2datasource.cpp
    c:\projects\gdal-coverage\gdal\ogr\ogrsf_frmts\db2\ogrdb2datasource.cpp(133) : error C2220: warning treated as error - no 'object' file generated 
    c:\projects\gdal-coverage\gdal\ogr\ogrsf_frmts\db2\ogrdb2datasource.cpp(133) : warning C4244: 'initializing' : conversion from '__int64' to 'int', possible loss of data 
    c:\projects\gdal-coverage\gdal\ogr\ogrsf_frmts\db2\ogrdb2datasource.cpp(265) : warning C4244: 'initializing' : conversion from '__int64' to 'int', possible loss of data 
    c:\projects\gdal-coverage\gdal\ogr\ogrsf_frmts\db2\ogrdb2datasource.cpp(495) : warning C4267: 'initializing' : conversion from 'size_t' to 'int', possible loss of data 
    c:\projects\gdal-coverage\gdal\ogr\ogrsf_frmts\db2\ogrdb2datasource.cpp(578) : warning C4267: '=' : conversion from 'size_t' to 'int', possible loss of data 
    c:\projects\gdal-coverage\gdal\ogr\ogrsf_frmts\db2\ogrdb2datasource.cpp(655) : warning C4267: 'initializing' : conversion from 'size_t' to 'int', possible loss of data 
    

Fixed. This was code that I had copied not long ago from the MSSQLSpatial driver... I can see that it has since been fixed to avoid this problem.

Is there any simple way to test for problems like this if I am using a 32-bit compiler?

ogrdb2layer.cpp ogrdb2tablelayer.cpp ogrdb2selectlayer.cpp }}}

in reply to:  25 comment:31 by dadler, 8 years ago

Replying to rouault:

You might want to remove the CPLDebug() traces in OGRDB2DriverIdentify() and OGRDB2DriverOpen() since they will be called for all datasources identification. We want to limit the verbosity at such generic level.

Fixed.

in reply to:  26 comment:32 by dadler, 8 years ago

Replying to rouault:

The i386 VC12 build at https://ci.appveyor.com/project/rouault/gdal-coverage/build/1.0.1481/job/ewwgdbxc3f3x0m45 succeeds but the autotest/cpp/gdal_unit_test.exe tests crashes with

cd autotest
cd cpp
nmake /f makefile.vc check MSVC_VER=1800 %WIN64_ARG%
 
Microsoft (R) Program Maintenance Utility Version 12.00.21005.1
Copyright (C) Microsoft Corporation. All rights reserved.
 
gdal_unit_test.exe
ERROR 7: Assertion `FALSE' failed
in file `c:\projects\gdal-coverage\gdal\gcore\gdaldrivermanager.cpp', line 405

This is the first time I see that... Hum, actually I think I know the reason. There's an inconsistency in ogrdb2driver.cpp :

    if (GDALGetDriverByName("DB2") != NULL) return;

    poDriver = new GDALDriver();
    poDriver->SetDescription( "DB2ODBC" );

The string tested in GDALGetDriverByName() and in SetDescription() should be the same so as to detect driver re-registration correctly, which must be what is tested here.

Fixed.

in reply to:  28 comment:33 by dadler, 8 years ago

Replying to rouault:

Not critical, but "poDriver->SetMetadataItem( GDAL_DCAP_RASTER, "YES" );" is not appropriate given the current capabilities of the driver.

Fixed.

in reply to:  27 comment:34 by dadler, 8 years ago

Replying to rouault:

Linux builds also fail on tests https://s3.amazonaws.com/archive.travis-ci.org/jobs/95619543/log.txt

Running tests from ogr/ogr_db2.py
  TEST: ogr_db2_check_driver ... fail

--> this should return 'skip' to avoid job failure

There is a test at the end of the testcase driver whether it is on Windows - but I just realized this is only executed when the test is executed standalone.. Fixed.

in reply to:  30 comment:35 by Even Rouault, 8 years ago

Is there any simple way to test for problems like this if I am using a 32-bit compiler?

Thanks for adressing the issues. The build bots are green now. I encourage you checking https://travis-ci.org/rouault/gdal_coverage/builds/ and https://ci.appveyor.com/project/rouault/gdal-coverage/history after your commits ( the failure on trunk_gcc5.2_sanitize is expected )

Regarding your question, I'm not aware of options to easily check warnings thrown in 64bit when you compile in 32bit (although conceptually the compiler could anticipate size_t/int incompatibilities, but I guess they have a hard enough job trying to figure out the right warnings for the current architecture).

comment:36 by dadler, 8 years ago

Resolution: fixed
Status: reopenedclosed

Made all corrections to issues/suggestions raised. Delivered to autotest with r32089 and r32079. Delivered to trunk/gdal with r32096 Closing this item.

comment:37 by dadler, 8 years ago

Made all corrections to issues/suggestions raised. Delivered to autotest with r32089 and r32079. Delivered to trunk/gdal with r32096 Closing this item.

Note: See TracTickets for help on using tickets.