After studying the OGR code in more detail, I think that
the attached patch is a valid solution to the SQLite export
problem. Potentially, it could also fix similar problems
with other output drivers that have not yet been reported.
Essentially, it takes care of creating new OGR feature
structures when they are needed and discards them
right after use, so that each feature exported by
v.out.ogr will start with a "clean" structure.
Unpatched v.out.ogr will "recycle" the same OGR feature
struct for each feature to export. Whether or not this
results in any problems depends on how a particular
OGR driver handles features without a preassigned
feature ID -- it is essentially unpredictable.
The overhead for memory management incurred by this
is minor. I tested exporting ca. 32,000 point records
and could not detect a measurable time difference.
It seems to have no negative effect on other drivers'
operations (would be hard to imagine how). I did some
tests with PostgreSQL and Shapefile data sources to
confirm.
Attached patch is for the current 6.4 SVN version of
v.out.ogr.
Best,
Ben
Benjamin Ducke wrote:
OK, I _may_ have fixed it.
The problem is/seems that v.out.ogr creates only one instance
of "OGRFeatureH" struct and then recycles that for every
vector object that needs to be exported, only clearing
the Geometry once for every run through the main export loop.
It also clears all user-defined attributes.This is obviously not a problem for file-based geometries such
as Shapefiles, because they consist of nothing more than
geometry + user attributes. But it seems that the SQLite driver
creates an "OGC_FID" as part of the feature and this "sticks around".
I have not tried very many other OGR drivers, but PostGIS handles
this differently and is OK.So what needs to be done is to destroy the entire feature
struct after every pass through the main export loop.Thus, the code for exporting points and lines would look as
follows (see "//NEW" tags):---
/* Lines (run always to count features of different type) */
if ((otype & GV_POINTS) || (otype & GV_LINES)) {
G_message(_("Exporting %i points/lines..."), Vect_get_num_lines(&In));
for (i = 1; i <= Vect_get_num_lines(&In); i++) {G_percent(i, Vect_get_num_lines(&In), 1);
//NEW:
Ogr_feature = OGR_F_Create(Ogr_featuredefn);type = Vect_read_line(&In, Points, Cats, i);
G_debug(2, "line = %d type = %d", i, type);
if (!(otype & type)) {
G_debug(2, "type %d not specified -> skipping", type);
fskip++;
continue;
}Vect_cat_get(Cats, field, &cat);
if (cat < 0 && !donocat) { /* Do not export not labeled */
nocatskip++;
continue;
}/* Geometry */
if (type == GV_LINE && poly_flag->answer) {
OGRGeometryH ring;ring = OGR_G_CreateGeometry(wkbLinearRing);
Ogr_geometry = OGR_G_CreateGeometry(wkbPolygon);/* Area */
for (j = 0; j < Points->n_points; j++) {
OGR_G_AddPoint(ring, Points->x[j], Points->y[j],
Points->z[j]);
}
if (Points->x[Points->n_points - 1] != Points->x[0] ||
Points->y[Points->n_points - 1] != Points->y[0] ||
Points->z[Points->n_points - 1] != Points->z[0]) {
OGR_G_AddPoint(ring, Points->x[0], Points->y[0],
Points->z[0]);
}OGR_G_AddGeometryDirectly(Ogr_geometry, ring);
}
else if (type == GV_POINT) {
Ogr_geometry = OGR_G_CreateGeometry(wkbPoint);
OGR_G_AddPoint(Ogr_geometry, Points->x[0], Points->y[0],
Points->z[0]);
}
else { /* GV_LINE or GV_BOUNDARY */Ogr_geometry = OGR_G_CreateGeometry(wkbLineString);
for (j = 0; j < Points->n_points; j++) {
OGR_G_AddPoint(Ogr_geometry, Points->x[j], Points->y[j],
Points->z[j]);
}
}
OGR_F_SetGeometry(Ogr_feature, Ogr_geometry);/* Output one feature for each category */
for (j = -1; j < Cats->n_cats; j++) {
if (j == -1) {
if (cat >= 0)
continue; /* cat(s) exists */
}
else {
if (Cats->field[j] == field)
cat = Cats->cat[j];
else
continue;
}mk_att(cat, Fi, Driver, ncol, doatt, Ogr_feature);
OGR_L_CreateFeature(Ogr_layer, Ogr_feature);
}
OGR_G_DestroyGeometry(Ogr_geometry);
//NEW:
OGR_F_Destroy(Ogr_feature);
}
}---
Clearly, this adds additional memory management overhead and will
slow down everything.So my questions are:
1. Do you think this fix is plausible?
2. Should it be enabled ONLY for SQLite type datasources?I would still feel better about this if someone more knowledgeable
about GDAL/OGR could comment on this...Thanks,
Ben
Benjamin Ducke wrote:
Dear all,
I have made some progress on this and think that I have actually
found the reason for this problem.The error message is issued by this block of C++ code
in gdal-1.6.1/ogr/ogrsf_frmts/sqlite/ogrsqlitetablelayer.cpp
(ca. line 826):/* -------------------------------------------------------------------- */
/* Execute the insert. */
/* -------------------------------------------------------------------- */
rc = sqlite3_step( hInsertStmt );if( rc != SQLITE_OK && rc != SQLITE_DONE )
{
CPLError( CE_Failure, CPLE_AppDefined,
"sqlite3_step() failed:\n %s",
sqlite3_errmsg(hDB) );return OGRERR_FAILURE;
}I have added some debugging code to the OGR SQLite driver to see
what the SQL statement looks like that actually gets processed
by sqlite3_step().The output of "v.out.ogr" then was:
Exporting 2898 points/lines...
SQL: 'INSERT INTO 'dem' (WKT_GEOMETRY,'cat','cat_','flt1') VALUES (?,'1','1','75.5')'SQL: 'INSERT INTO 'dem' (OGC_FID,WKT_GEOMETRY,'cat','cat_','flt1') VALUES (1,?,'2','2','73.57')'
ERROR 1: sqlite3_step() failed:
SQL logic error or missing databaseSQL: 'INSERT INTO 'dem' (OGC_FID,WKT_GEOMETRY,'cat','cat_','flt1') VALUES (1,?,'3','3','75.41')'
ERROR 1: sqlite3_step() failed:
SQL logic error or missing database
[...]Interestingly, the SQL code for the first feature (which gets stored
OK in the DB) differs from that of all the following in that it does
not insert a value into the "OGC_FID" column!After that, the statements look OK but they are not: "OGC_FID" is
a primary key field, but the same value "1" is generated for all
rows.Actually, the first statement is the right one! Since it does not
touch the "OGC_FID" field, but that field is properly declared
as an auto-incrementing primary key, the SQLite3 driver takes care
of inserting a proper value by itself.So it seems to me the fix would be to stop v.out.ogr from creating
an "OGC_FID" value as part of the attribute record for the OGR
feature (after the first record, which is for some reason OK!).Unfortunately, I know very little about the OGR C API and don't quite
understand how to change the code in v.out.ogr. Could someone more
familiar with that module help out, please?Thanks,
Ben
P.S.: Why this all seems to be no problem for a PostGIS DBMS completely
eludes me -- perhaps it just ignores wrong values for a PK field and
silently replaces them with correct values...Benjamin Ducke wrote:
Dear all,
I have been trying in vain to store some very simple GRASS vector
map in an SQLite3 DBMS using v.out.ogr. The data consists of
only 93 3D points with attached integer and double attributes.
No complex shapes, timestamps, text or blobs.Using v.out.ogr, I get:
ERROR 1: sqlite3_step() failed:
SQL logic error or missing databasefor all except the first record, which gets stored correctly
in the (new) database. It is not possible to test this with
an existing SQLite database file, because v.out.ogr in RC5
does not support the OGR "update" action.I have searched the Trac system for clues and found one ticket
that may be related: http://trac.osgeo.org/grass/ticket/548I have also tried this with different versions of SQLite and
on different Linux systems, but always with the same result.My setup:
32Bit Gentoo Linux
GRASS 6.4RC5
SQLite 3.6.16
GDAL 1.6.1 (SQLite driver using native code, not linked
to spatialite)Is there anybody here experiencing the same problems?
Any idea where to start looking? I am very interested in
making external SQLite support via OGR work, but have no
clue where to start looking to resolve the problem, so
any ideas are more than welcome!Thanks,
Ben
------
Files attached to this email may be in ISO 26300 format (OASIS Open Document Format). If you have difficulty opening them, please visit http://iso26300.info for more information._______________________________________________
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev------
Files attached to this email may be in ISO 26300 format (OASIS Open Document Format). If you have difficulty opening them, please visit http://iso26300.info for more information._______________________________________________
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev------
Files attached to this email may be in ISO 26300 format (OASIS Open Document Format). If you have difficulty opening them, please visit http://iso26300.info for more information._______________________________________________
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev
--
Benjamin Ducke
Senior Applications Support and Development Officer
Oxford Archaeological Unit Limited
Janus House
Osney Mead
OX2 0ES
Oxford, U.K.
Tel: +44 (0)1865 263 800 (switchboard)
Tel: +44 (0)1865 980 758 (direct)
Fax :+44 (0)1865 793 496
benjamin.ducke@oxfordarch.co.uk
------
Files attached to this email may be in ISO 26300 format (OASIS Open Document Format). If you have difficulty opening them, please visit http://iso26300.info for more information.
(attachments)
v.out.ogr.diff (1.48 KB)