Opened 16 years ago
Closed 13 years ago
#1960 closed defect (fixed)
OCI: ogr2ogr with append option is not considering layer creation option(lco) values
Reported by: | Spatial01 | Owned by: | ilucena |
---|---|---|---|
Priority: | normal | Milestone: | |
Component: | OGR_SF | Version: | 1.4.2 |
Severity: | normal | Keywords: | append lco dim oracle oci |
Cc: | warmerdam |
Description
When using the 'append' option of ogr2ogr with -lco DIM=2 with OCI driver for Oracle it's creating geometries in 3 dimensionality even though I have specified 2 dimension using DIM=2 option.
Here is the syntax I have used:
ogr2ogr -append -update -f OCI "OCI:username/password@tnsalias" -lco DIM=2 -lco SRID=8307 -nln table1 Shapefile.shp
My geometry looks like this after executing the above command:
sdo_geometry(3003,8307,Null,SDO_ELEM_INFO_ARRAY(1,1003,3),SDO_ORDINATE_ARRAY(1,1, 5,1, 5,7, 1,7, 1,1))
But the correct entry should be 2003 instead of 3003
sdo_geometry(2003,8307,Null,SDO_ELEM_INFO_ARRAY(1,1003,3),SDO_ORDINATE_ARRAY(1,1, 5,1, 5,7, 1,7, 1,1))
Attachments (2)
Change History (30)
comment:1 by , 16 years ago
Cc: | added |
---|---|
Milestone: | → 1.5.0 |
Owner: | changed from | to
comment:2 by , 16 years ago
Version: | 1.3.2 → 1.4.2 |
---|
I actually tested with 1.4.2 version. Sorry about that typing wrong version number. In Oracle there is no way of knowing dimensionality if the table is empty and there is no spatial index on it. After testing more I found couple of things that are happening in append mode.
-It commits after each record since MULTI_LOAD option is not considered in append mode.
- It is not using bind variables with INSERT statement and is causing high CPU usage by forcing Oracle to parse every INSERT statement in append mode. But if ogr2ogr is creating a table and inserts the data it binds all columns with variables. Can we make MULTI_LOAD as default option in append mode?
I think we have to incorporate the following checks into OCI layer to check for dimensionality.
If data exists in table, check for existing geometry dimensionality.
select a.geometry_column.get_Dims() from table_name a where rownum=1
If no data but spatial index exists then check for index dimensionality.
select m.sdo_index_dims from all_sdo_index_info i, all_sdo_index_metadata m where i.sdo_index_owner=m.sdo_index_owner and i.index_name=m.sdo_index_name and i.table_name='TABLE_NAME'
comment:3 by , 15 years ago
Priority: | high → normal |
---|---|
Severity: | major → normal |
comment:4 by , 15 years ago
Milestone: | 1.5.0 → 1.5.1 |
---|---|
Summary: | ogr2ogr with append option is not considering layer creation option(lco) values → OCI: ogr2ogr with append option is not considering layer creation option(lco) values |
comment:5 by , 15 years ago
Keywords: | oracle added |
---|---|
Status: | new → assigned |
by , 15 years ago
Attachment: | ogrocitablelayer.cpp added |
---|
comment:6 by , 15 years ago
Hello,
I've adapted the OCI driver to handle the dimension according to the dimension of the data that already exists. This overwrite the default dimension setting.
To do that, I've included this code in the function OGROCITableLayer::ReadTableDefinition (file ogrocitablelayer.cpp)
I've tested it succesfully and it's easy to understand.
/* ------------------------------------------------------------ */ /* Identify Geometry dimension */ /* ------------------------------------------------------------ */
OGROCIStringBuf oDimCmd; OGROCIStatement oDimStatement( poSession ); char papszResult; int iDim = -1;
oDimCmd.Append( "SELECT a." ); oDimCmd.Append( pszGeomName ); oDimCmd.Append( ".GET_DIMS() DIM FROM " ); oDimCmd.Append( pszTable ); oDimCmd.Append( " a WHERE ROWNUM = 1" );
oDimStatement.Execute( oDimCmd.GetString() );
papszResult = oDimStatement.SimpleFetchRow();
if( CSLCount(papszResult) < 1 )
CPLDebug( "OCI", "get dim based on data failed." );
else {
iDim = atoi(papszResult[0]); SetDimension( iDim );
}
I join the full source
May this code help
Nicolas
comment:7 by , 14 years ago
Hi Nicolas, Thanks for providing the code. When I tested the it error out since I use layer name <owner>.<tablename>. So I copied the split function from another function.
OGROCITableLayer::ReadTableDefinition? (file ogrocitablelayer.cpp)
158 /* -------------------------------------------------------------------- */ 159 /* Split out the owner if available. */ 160 /* -------------------------------------------------------------------- */ 161 const char *pszTableName = pszTable; 162 char *pszOwner = NULL; 163 164 if( strstr(pszTableName,".") != NULL ) 165 { 166 pszOwner = CPLStrdup(pszTableName); 167 pszTableName = strstr(pszTableName,".") + 1; 168 169 *(strstr(pszOwner,".")) = '\0'; 170 }
Also I have modified the dimension query so that it does not depend on the table having data. Here is the modified code.
OGROCITableLayer::ReadTableDefinition? (file ogrocitablelayer.cpp)
256 /* -------------------------------------------------------------------- */ 257 /* Identify Geometry dimension */ 258 /* -------------------------------------------------------------------- */ 259 260 OGROCIStringBuf oDimCmd; 261 OGROCIStatement oDimStatement( poSession ); 262 char papszResult; 263 int iDim = -1; 264 265 oDimCmd.Append( "SELECT COUNT(*) FROM ALL_SDO_GEOM_METADATA u,TABLE(u.diminfo) t" ); 266 oDimCmd.Append( " WHERE u.TABLE_NAME='" ); 267 oDimCmd.Append( pszTableName ); 268 oDimCmd.Append( "' AND u.COLUMN_NAME='" ); 269 oDimCmd.Append( pszGeomName ); 270 oDimCmd.Append( "'" ); 271 272 oDimStatement.Execute( oDimCmd.GetString() ); 273 274 papszResult = oDimStatement.SimpleFetchRow(); 275 276 if( CSLCount(papszResult) < 1 ) 277 CPLDebug( "OCI", "get dim based on all_sdo_geom_metadata failed." ); 278 else 279 { 280 iDim = atoi(papszResult[0]); 281 SetDimension( iDim ); 282 } 283 284 /* -------------------------------------------------------------------- */ 285 /* End modif */ 286 /* -------------------------------------------------------------------- */
Thanks again. It will be good if someone can modify UnboundCreateFeature to use bind variables and commit after every 100 rows instead of after each row.
-Bala
comment:8 by , 13 years ago
Keywords: | og2ogr removed |
---|---|
Milestone: | 1.5.4 → 1.7.0 |
Owner: | changed from | to
Status: | assigned → new |
Pushed to 1.7.0 for review, taking it back myself.
comment:10 by , 13 years ago
Status: | new → assigned |
---|
Committed revision r19477
I loaded two polygon on each one of those commands:
% ogr2ogr -f OCI oci:scott/tiger@orcl test.shp -lco DIM=2 % ogr2ogr -append -update -f OCI oci:scott/tiger@orcl -nln test -lco DIM=2 test2.shp WARNING: Layer creation options ignored since an existing layer is being appended to.
And this is the resulting geometries:
SQL> select ora_geometry from test; ORA_GEOMETRY(SDO_GTYPE, SDO_SRID, SDO_POINT(X, Y, Z), SDO_ELEM_INFO, SDO_ORDINAT -------------------------------------------------------------------------------- SDO_GEOMETRY(2003, NULL, NULL, SDO_ELEM_INFO_ARRAY(1, 1003, 1), SDO_ORDINATE_ARR AY(-74, 40, -73.999995, 40, -73.999995, 40.000009, -74, 40.000009, -74, 40)) SDO_GEOMETRY(2003, NULL, NULL, SDO_ELEM_INFO_ARRAY(1, 1003, 1), SDO_ORDINATE_ARR AY(-73.999995, 40, -73.999991, 40, -73.999991, 40.000009, -73.999995, 40.000009, -73.999995, 40)) SDO_GEOMETRY(2003, NULL, NULL, SDO_ELEM_INFO_ARRAY(1, 1003, 1), SDO_ORDINATE_ARR AY(-74.000005, 40, -74, 40, -74, 40.000009, -74.000005, 40.000009, -74.000005, 4 0)) ORA_GEOMETRY(SDO_GTYPE, SDO_SRID, SDO_POINT(X, Y, Z), SDO_ELEM_INFO, SDO_ORDINAT -------------------------------------------------------------------------------- SDO_GEOMETRY(2003, NULL, NULL, SDO_ELEM_INFO_ARRAY(1, 1003, 1), SDO_ORDINATE_ARR AY(-74.000009, 40, -74.000005, 40, -74.000005, 40.000009, -74.000009, 40.000009, -74.000009, 40))
I haven't touched the warning message problem or optimization issues yet.
Ivan.
comment:11 by , 13 years ago
Ivan,
Could you also add support for setting the SRID? See ticket #3622 for example
comment:12 by , 13 years ago
The option -lco SRID works when the layer is "new":
% ogr2ogr -f OCI oci:scott/tiger@orcl test.shp -nln test4 -lco DIM=*2* -lco SRID=*8307* SQL> select ora_geometry from test4; ORA_GEOMETRY(SDO_GTYPE, SDO_SRID, SDO_POINT(X, Y, Z), SDO_ELEM_INFO, SDO_ORDINAT -------------------------------------------------------------------------------- SDO_GEOMETRY(*2003*, *8307*, NULL, SDO_ELEM_INFO_ARRAY(1, 1003, 1), SDO_ORDINATE_ARR AY(-74, 40, -73.999995, 40, -73.999995, 40.000009, -74, 40.000009, -74, 40))
The options -append and -update works well and it is not necessary to inform -lco DIM or -lco SRID. The OCI driver will use the ones entered previously:
% ogr2ogr -append -update -f OCI oci:scott/tiger@orcl -nln test4 test2.shp SQL> select ora_geometry from test4; ... SDO_GEOMETRY(2003, 8307, NULL, SDO_ELEM_INFO_ARRAY(1, 1003, 1), SDO_ORDINATE_ARR AY(-73.999995, 40, -73.999991, 40, -73.999991, 40.000009, -73.999995, 40.000009, -73.999995, 40))
That looks consistent. Now, if I try to inform -lco DIM or -lco SRID that is what I got:
% ogr2ogr -append -update -f OCI oci:scott/tiger@orcl -nln test4 test2.shp -lco SRID=8307 WARNING: Layer creation options ignored since an existing layer is being appended to.
What also seems consistent.
Should we call that bug fixed?
comment:13 by , 13 years ago
The dimensionality determination does seem to work ok but the SRID code only works for certain types of data. I was trying from KMZ using the libkml driver and the SRID does not get populated properly. It would be better if the append code read the srid from the user_sdo_geom_metadata table to determine the SRID.
u4rt9mds@maps:~/programming/MapServer/gdal_trunk$ ~/programming/MapServer/gdal_trunk/apps/ogr2ogr -append -update -f OCI oci:user/pass@tns:marine_test2 /htdocs/dhs/marine/MarineTraffic.kmz -nln marine_test2 -lco dim=2 -lco srid=8307 WARNING: Layer creation options ignored since an existing layer is
being appended to.
ERROR 1: ORA-29875: failed in the execution of the ODCIINDEXINSERT routine ORA-13365: layer SRID does not match geometry SRID ORA-06512: at "MDSYS.SDO_INDEX_METHOD_10I", line 709 ORA-06512: at "MDSYS.SDO_INDEX_METHOD_10I", line 225
in INSERT INTO marine_test2 (ORA_GEOMETRY, OGR_FID, "Name", "description", "tessellate", "extrude", "visibility") VALUES (MDSYS.SDO_GEOMETRY(2001,NULL,MDSYS.SDO_POINT_TYPE(117.7996,38.97557,0),NULL,NULL), 1, 'JIN YUAN 8', '<b>Speed/Course:</b> 9.1 knots / 131°<br/><b>Flag:</b> KH <img src="http://www.marinetraffic.com/ais/flags/KH.gif"/><br/><br/><a href="http://www.marinetraffic.com/ais/shipdetails.aspx?mmsi=514465000"><img src="http://photos.marinetraffic.com/ais/showphoto.aspx?size=thumb&mmsi=514465000"/><br/>Vessels details on MarineTraffic.com<br/></a><a href="http://www.marinetraffic.com/ais/gettrackkml.aspx?mmsi=514465000">Show Vessels Track</a>', -1, -1, -1)
comment:14 by , 13 years ago
Mike?
It is still working fine for me. I can see the warning and then the features are loaded with the same SRID of the pre loaded ones.
How did your initial ogr2ogr went, what was -lco options, What was the SRID?
Can I have access to your input file?
Thanks a lot.
Ivan
comment:15 by , 13 years ago
Original load:
/ogr2ogr -f OCI oci:user/pass@tns:tmp /htdocs/dhs/marine/MarineTraffic.kmz -nln marine_test2 -lco dim=2 -lco srid=8307
Source File:
curl -o MarineTraffic.kmz http://marinetraffic2.aegean.gr/ais/getkml.aspx
comment:16 by , 13 years ago
I haven't taken the time to understand the oracle specific issues, but I would like to stress that layer creation options are only for layer creation, and we should not expect them to have any effect with ogr2ogr when just appending to an existing layer.
If the OCI driver cannot determine important information like the dimension for an existing layer, then the alternatives are:
- Having some similar config options to the layer creation options that could be specified on the ogr2ogr commandline using --config
- or it might just not be possible to append to an existing table with good control. I can't really think of other good options.
But I do hope all possibilities for detecting reasonable settings would be used. This may not have been well done in the past.
comment:17 by , 13 years ago
The code changes suggested previously on that ticket, that have been committed to the trunk, are working fine on my system so I am trying to add KMZ support to try to replicate the same error. No luck so far.
comment:18 by , 13 years ago
I am using trunk and using the libkml driver to read KMZ. Oracle version is 10.2.0.4 with instant client 11.1.0.7 if that helps. The dim checks work fine. Its the srid checks that don't work for me.
comment:19 by , 13 years ago
I tried to run the same test on a Windows system with Oracle 10g but I wasn't able to use libkmz. How did you compile it? Can you send the makefile.vc?
But I went back to Linux and then I was able to replicated the same problem with the same data but in a Oracle 11g server.
% ogr2ogr -append -update -f oci oci:scott/tiger@orcl:marine_test1 MarineTraffic.kmz -nln marine_test1 -lco dim=2 -lco srid=8307 WARNING: Layer creation options ignored since an existing layer is being appended to. ERROR 1: ORA-29875: failed in the execution of the ODCIINDEXINSERT routine ORA-13365: layer SRID does not match geometry SRID ORA-06512: at "MDSYS.SDO_INDEX_METHOD_10I", line 720 ORA-06512: at "MDSYS.SDO_INDEX_METHOD_10I", line 225 in INSERT INTO marine_test1 (ORA_GEOMETRY, OGR_FID, "Name", "description", "tessellate", "extrude", "visibility") VALUES (MDSYS.SDO_GEOMETRY(2001,NULL,MDSYS.SDO_POINT_TYPE(4.292449,51.34238,0),NULL,NULL), 1, 'TUG 21', '<b>Speed/Course:</b> 0 knots / 126°<br/><b>Flag:</b> BE <img src="http://www.marinetraffic.com/ais/flags/BE.gif"/><br/><br/><a href="http://www.marinetraffic.com/ais/shipdetails.aspx?mmsi=205360190"><img src="http://photos.marinetraffic.com/ais/showphoto.aspx?size=thumb&mmsi=205360190"/><br/>Vessel''s details on MarineTraffic.com<br/></a><a href="http://www.marinetraffic.com/ais/gettrackkml.aspx?mmsi=205360190">Show Vessel''s Track</a>', -1, -1, -1) ERROR 1: Terminating translation prematurely after failed translation of layer 0 (use -skipfailures to skip errors)
But I noticed that if I don't inform the layer name on the identification string the problem doesn't show up and the data is included perfectly.
% ogr2ogr -append -update -f oci oci:scott/tiger@orcl MarineTraffic.kmz -nln marine_test1 -lco dim=2 -lco srid=8307 WARNING: Layer creation options ignored since an existing layer is being appended to.
We still have a problem to fix on the code but could that be a work around for you?
comment:20 by , 13 years ago
I haven't been able to compile libkml on windows yet. I'm not sure if the options have been added to the makefile.vc yet.
Hmm. I'll give it a try without the layer name. However, in the past when doing that, it has to run through all the spatial layers we have in the database and it really increases the time to go through 1000 layers.
comment:21 by , 13 years ago
Due to the number of layers that we have in our database, it fails. I get a series of
OCI: get dim based of existing data failed. OCI: Prepare(SELECT SRID FROM ALL_SDO_GEOM_METADATA WHERE TABLE_NAME = 'AQUIFRP025' AND COLUMN_NAME = 'SHAPE' AND OWNER = 'NAT_ATLAS') OCI: Prepare(SELECT a.SHAPE.GET_DIMS() DIM FROM SIP_USN_PARKING_A a WHERE ROWNUM = 1) ERROR 1: ORA-00942: table or view does not exist
in SELECT a.SHAPE.GET_DIMS() DIM FROM SIP_USN_PARKING_A a WHERE ROWNUM = 1
ERROR 1: ORA-24374: define not done before fetch or execute and fetch
in OCIStmtFetch
OCI: get dim based of existing data failed. OCI: Prepare(SELECT SRID FROM ALL_SDO_GEOM_METADATA WHERE TABLE_NAME = 'SIP_USN_PARKING_A' AND COLUMN_NAME = 'SHAPE' AND OWNER = 'DISDI') OCI: Prepare(SELECT a.SHAPE.GET_DIMS() DIM FROM MC a WHERE ROWNUM = 1) OCI: Prepare(SELECT SRID FROM ALL_SDO_GEOM_METADATA WHERE TABLE_NAME = 'MC' AND COLUMN_NAME = 'SHAPE') OCI: Prepare(SELECT a.SHAPE.GET_DIMS() DIM FROM RH a WHERE ROWNUM = 1) OCI: Prepare(SELECT SRID FROM ALL_SDO_GEOM_METADATA WHERE TABLE_NAME = 'RH' AND COLUMN_NAME = 'SHAPE') Aborted
And then it aborts and no data gets loaded. So looks like this isn't a workaround I can use.
comment:22 by , 13 years ago
Yes,
It takes forever to run if you don't inform table name, so I have another work around, actually it might be the solution. Try to use the table name in uppercase:
% ogr2ogr -append -update -f oci oci:scott/tiger@orcl:MARINE_TEST1 MarineTraffic.kmz -nln marine_test1 -lco dim=2 -lco srid=8307
If that solves your problem then you can grab and build the code from GDAL trunk with the changes I just made: http://trac.osgeo.org/gdal/changeset/19871/trunk
Then, please test again the same command with table name in lowercase, just to make sure that it is fixed and we are not going to have that problem again.
comment:23 by , 13 years ago
Ivan, That worked well for the SRID, it properly picks it up. But the DIM is incorrect. The code examines the first row to check layer dim. Instead, it should the ALL_SDO_INDEX_METADATA and ALL_SDO_INDEX_INFO like this
select m.sdo_index_dims from all_sdo_index_metadata m, all_sdo_index_info i where i.index_name = m.sdo_index_name
and i.sdo_index_owner = m.sdo_index_owner and i.sdo_index_owner = :OWNER_NAME and i.table_name = :TABLE_NAME
Then I think it would all work fine.
comment:24 by , 13 years ago
You are right, the current code get the dimension from the function GET_DIMS applied to the geometry column and as long as the data was recorded consistently, the first row should be equal to all the rest. Right?
SELECT a.<geometry column>.GET_DIMS() FROM <table> a WHERE ROWNUM = 1;
Is ti possible that after all those attempts, your table has inconsistent geometry objects with different dimensions?
comment:25 by , 13 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
I have confirmed that once that changes are applied to OGR/OCI the process of loading the first layer or appending new layers should keep the same dimension as it was initially defined. In that case, the layers type should be consistent with the index and metadata.
comment:26 by , 13 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
Dimension check only works if there is at least one row. If table is empty, dimension check doesn't work. It would be better to check dimensionality with the SQL I posted.
I want to use this to reload data on a periodic basis so the table will be empty before i reload.
comment:27 by , 13 years ago
msmitherdc, Thanks for your clarification. Give it a try to the current revision r19894 and post your comments so we can close not only based on the initial post but also based on the subsequent additions.
Note that at that point the driver doesn't know who is the owner of the selected table so I needed to simplify your statement. That could be a potential problem if different schemes use same table name, but at least I kept the ROWNUM based query to be safe.
If the MULTI_LOAD questions (3 years ago) is still a concern, I suggest you to file another ticket so that those major changes could be handled properly.
comment:28 by , 13 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
Revision works well. Thanks
Spatial01,
Layer creation options are not used in append mode since the layer is not being created.
Assuming the layer was originally created with DIM=2, I think the driver needs to detect that when the layer is accessed, and thereafter work in 2D mode.
Is it practical for you to confirm that this problem remains the same with GDAL/OGR 1.4.2 or newer? In fact, I suspect there is already an outstanding bug report on this particular issue.
Turning over to Mateusz to address along with a batch of other oracle issues for the 1.5 release. I will also modify ogr2ogr to issue a warning if layer creation options are used along with -append to reduce confusion.