Precision Problem for Numeric on OGR/OCI driver

[pauloaragao]

Hello, Currently I am using GDAL 1.5.1 In my Oracle Spatial database, the ID column was defined as NUMBER(15). In GDAL code, it is treated as integer (OFTInteger type). As a result, I get wrong results (negative values) because this value will be stored as integer (OFTInteger type).

As I could notice, in OGRField class, the value will be stored in a union type. So, for OFTInteger, the value will be stored as int type. In a 32 bit architecture, the possible values is from –2147483648 to 2147483647. On this way, in general, oracle defined types with precision > 9 can not be represented as integer.

Just as a temporary solution, I have modified the file \gdal-1.5.1\ogr\ogrsf_frmts\oci\ogrocisession.cpp, method OGROCISession::GetParmInfo, as you can see below.

Note 1: In Oracle definition, the format is NUMBER(precision, scale). On reflection, I believe OGR and Oracle use the same approach though the names are a bit different. What OGR calls "width and precision" Oracle calls "precision and scale".

Note 2: In Oracle, Precision is the total number of digits and scale is the number of digits to the right of the decimal point. So, for precision should be considered extra spaces for “-“ sign and “.” decimal point.

switch( nOCIType )

{

case SQLT_CHR: case SQLT_AFC: /* CHAR(), NCHAR() */

poOGRDefn->SetType( OFTString ); if( nOCILen < 2048 )

poOGRDefn->SetWidth( nOCILen );

break;

case SQLT_NUM: {

NOTE: OCI docs say this should be ub1 type, but we have determined that oracle is actually returning a short so we use that type and try to compensate for possible problems by initializing, and dividing by 256 if it is large. unsigned short byPrecision = 0; sb1 nScale;

if( Failed(

OCIAttrGet( hParmDesc, OCI_DTYPE_PARAM, (dvoid )&byPrecision,

0, OCI_ATTR_PRECISION, hError ),

"OCIAttrGet(Precision)" ) ) return CE_Failure;

if( Failed(

OCIAttrGet( hParmDesc, OCI_DTYPE_PARAM, (dvoid )&nScale,

0, OCI_ATTR_SCALE, hError ),

"OCIAttrGet(Scale)") ) return CE_Failure;

#ifdef notdef

CPLDebug( "OCI", "%s: Scale=%d, Precision=%d",

szTermColName, nScale, byPrecision );

#endif

if( byPrecision > 255 )

byPrecision = byPrecision / 256;

if( nScale < 0 )

poOGRDefn->SetType( OFTReal );

else if( nScale > 0 ) {

poOGRDefn->SetType( OFTReal ); poOGRDefn->SetWidth( byPrecision ); poOGRDefn->SetPrecision( nScale );

} Code commented from the original code by Paulo Aragao (temp solution) else if( byPrecision < 38 ) { poOGRDefn->SetType( OFTInteger ); poOGRDefn->SetWidth( byPrecision ); }

Code Inserted by Paulo Aragao else if( byPrecision > 9 ) {

poOGRDefn->SetType( OFTReal ); poOGRDefn->SetWidth( byPrecision );

poOGRDefn->SetPrecision( 0 );

}

/ else {

poOGRDefn->SetType( OFTInteger );

}

} break;

[pauloaragao]

Also should be considered the extra space for "-" signal

[pauloaragao]

As wrong value can be saved to database, I set up this problem as major

[warmerdam]

Pushing off to 1.6.4.

[ilucena]

I have tested the suggestions made by Paulo and that seems to make sense overall except that it looks kind of funny to see a primary key showing up as Real. But in the other hand, the comments of ogr_core.h does mention the possibility of extension. e.g.:

/**
 * List of feature field types.  This list is likely to be extended in the
 * future ... avoid coding applications based on the assumption that all
 * field types can be known.
 */

typedef enum 
{
  /** Simple 32bit integer */                   OFTInteger = 0,
  /** List of 32bit integers */                 OFTIntegerList = 1,
  /** Double Precision floating point */        OFTReal = 2,
  /** List of doubles */                        OFTRealList = 3,
  /** String of ASCII chars */                  OFTString = 4,
  /** Array of strings */                       OFTStringList = 5,
  /** deprecated */                             OFTWideString = 6,
  /** deprecated */                             OFTWideStringList = 7,
  /** Raw Binary data */                        OFTBinary = 8,
  /** Date */                                   OFTDate = 9,
  /** Time */                                   OFTTime = 10,
  /** Date and Time */                          OFTDateTime = 11,
                                                OFTMaxType = 11
} OGRFieldType;

I believe that the natural choice would be long integer, right?

But I don't know what would be the consequence for adding something like that to ogr_core.h:

/ Simple 64bit integer */ OFTLongInteger = 12,

What do you think Frank?

[warmerdam]

I am queasy about the idea of using a real as a key due to precision issues.

There has been a long standing need for a long integer field type in OGR. Perhaps it is time to add it. But to do so will require a small RFC.

[ilucena]

The time has come: http://trac.osgeo.org/gdal/wiki/rfc31_ogr_64

[Even Rouault]

Fixed per http://trac.osgeo.org/gdal/wiki/rfc31_ogr_64

[Even Rouault]

Milestone renamed