Opened 8 years ago

Closed 7 years ago

#5376 closed defect (fixed)

OGR OCI: UpdateLayerExtents() perfomed also for NON-geometric tables

Reported by: giorgiomugnaini Owned by: warmerdam
Priority: normal Milestone: 1.11.2
Component: default Version: 1.11.0
Severity: major Keywords: ogrocidatasource, oci
Cc: Even Rouault, ilucena

Description

In SyncToDisk?(), UpdateLayerExtents?() is performed, regardless the table has geometry field or not.

Therefore during UpdateLayerExtents?(), spurious insertions in sdo_geometry_metadata_table are performed, because of the following fragment:

 sDimUpdate.Append( "INSERT INTO USER_SDO_GEOM_METADATA VALUES " );
        sDimUpdate.Appendf( strlen(poFeatureDefn->GetName()) + 100,
                            "('%s', '%s', ",
                            poFeatureDefn->GetName(),
                            pszGeomName );

        sDimUpdate.Append( "MDSYS.SDO_DIM_ARRAY(" );
        sDimUpdate.Appendf(200,
                           "MDSYS.SDO_DIM_ELEMENT('X',%.16g,%.16g,%.12g)",
                           dfXMin, dfXMax, dfXRes );
        sDimUpdate.Appendf(200,
                           ",MDSYS.SDO_DIM_ELEMENT('Y',%.16g,%.16g,%.12g)",
                           dfYMin, dfYMax, dfYRes );

        if( nDimension == 3 )
        {
            sDimUpdate.Appendf(200,
                               ",MDSYS.SDO_DIM_ELEMENT('Z',%.16g,%.16g,%.12g)",
                               dfZMin, dfZMax, dfZRes );
        }

        if( nSRID == -1 )
            sDimUpdate.Append( "), NULL)" );
        else
            sDimUpdate.Appendf( 100, "), %d)", nSRID );

Change History (8)

comment:1 Changed 8 years ago by giorgiomugnaini

After the first time, the spurious insertions generate SQL exception, because the violation of uniqueness constraint in sdo_geometry_metadata_table as in the following example:

ERROR 1: ORA-13223: duplicate entry for MY_TABLE_DATA.(null) in SDO_GEOM_METAD
ATA
ORA-06512: at "MDSYS.MD", line 1723
ORA-06512: at "MDSYS.MDERR", line 17
ORA-06512: at "MDSYS.SDO_GEOM_TRIG_INS1", line 48
ORA-04088: error during execution of trigger 'MDSYS.SDO_GEOM_TRIG_INS1'
 in INSERT INTO USER_SDO_GEOM_METADATA VALUES ('MY_TABLE_DATA', '(null)', MDSY
S.SDO_DIM_ARRAY(MDSYS.SDO_DIM_ELEMENT('X',-3e-007,3e-007,1e-007),MDSYS.SDO_DIM_E
LEMENT('Y',-3e-007,3e-007,1e-007),MDSYS.SDO_DIM_ELEMENT('Z',-100000,100000,0.002
)), NULL)

comment:2 Changed 7 years ago by giorgiomugnaini

Version: unspecified1.11.0

comment:3 Changed 7 years ago by giorgiomugnaini

The OciDataSource? has been created specifying two tables (one only having a geometry field) as in the following case

 ... = OGRSFDriverRegistrar::Open( "OCI:user:passwd@db:TABLE_WITH_GEOMETRY,MY_TABLE_DATA" ,TRUE )

where:

  • TABLE_WITH_GEOMETRY : table having a geometry field;
  • MY_TABLE_DATA : table without geometry field.

comment:4 Changed 7 years ago by giorgiomugnaini

It is interesting to observe that OGR seems to believe that table MY_TABLE_DATA has a fictitious geometry field:

pszGeomName='(null)'

insted of no geometry, i.e.

pszGeomName = NULL

comment:5 Changed 7 years ago by giorgiomugnaini

Cc: Even Rouault ilucena added
Keywords: ogrocidatasource oci added
Severity: normalmajor

comment:6 Changed 7 years ago by Even Rouault

Resolution: fixed
Status: newclosed

Should be fixed with trunk r27826, branches/1.11 r27827 "OCI: initialize member variable to avoid UpdateLayerExtents?() to be called randomly on non spatial tables (#5376)"

comment:7 Changed 7 years ago by giorgiomugnaini

Resolution: fixed
Status: closedreopened

Thank you Rouault, in fact this attribute was not initialized. But a problem is still present in my point of view.

If we examine UpdateLayerExtents?() ( row 1360 in ogrocitablelayer.cpp) we read:

void OGROCITableLayer::UpdateLayerExtents()

{
    if( !bExtentUpdated )
        return;
...

I think that bExtentUpdated should be false for non-spatial tables, otherwise the spurious writing on USER_SDO_GEOM_METADATA will be performed.

In the constructor OGROCITableLayer::OGROCITableLayer() (at row 70):

   poFeatureDefn = ReadTableDefinition( pszTableName );

I suggest to add the following correction after row 70:

  if( !pszGeomName ) ///<pszGeomName  is null for non-spatial tables
     bExtentUpdated = false;

What do you think about such proprosal?

comment:8 Changed 7 years ago by Even Rouault

Milestone: 1.11.2
Resolution: fixed
Status: reopenedclosed

Doh, I initialized the value to the opposite value I intended to ! trunk r27830, branches/1.11 r27831 "OCI: fix previous commit. bExtentUpdated should be initialized to false (#5376)"

Note: See TracTickets for help on using tickets.