Opened 12 years ago

Closed 7 years ago

Last modified 6 years ago

#3150 closed defect (fixed)

Precision Problem for Numeric on OGR/OCI driver

Reported by: pauloaragao Owned by: ilucena
Priority: normal Milestone: 2.0.0
Component: OGR_SF Version: unspecified
Severity: major Keywords: OCI
Cc: warmerdam

Description

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;

Attachments (1)

ogrocisession.cpp (16.5 KB) - added by pauloaragao 12 years ago.

Download all attachments as: .zip

Change History (11)

Changed 12 years ago by pauloaragao

Attachment: ogrocisession.cpp added

comment:1 Changed 12 years ago by pauloaragao

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

comment:2 Changed 12 years ago by pauloaragao

Severity: normalmajor

comment:3 Changed 12 years ago by pauloaragao

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

comment:4 Changed 12 years ago by warmerdam

Milestone: 1.5.51.6.4

Pushing off to 1.6.4.

comment:5 Changed 12 years ago by warmerdam

Cc: warmerdam added
Keywords: OCI added
Milestone: 1.6.4
Owner: changed from warmerdam to ilucena

comment:6 Changed 12 years ago by ilucena

Status: newassigned

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?

comment:7 Changed 12 years ago by 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.

comment:9 Changed 7 years ago by Even Rouault

Milestone: 2.0
Resolution: fixed
Status: assignedclosed

comment:10 Changed 6 years ago by Even Rouault

Milestone: 2.02.0.0

Milestone renamed

Note: See TracTickets for help on using tickets.