[GRASS-dev] SQLite 3 "SQL logic error" -- fixed
Benjamin Ducke
benjamin.ducke at oxfordarch.co.uk
Mon Aug 31 08:16:48 EDT 2009
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 database
>>
>> SQL: '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 database
>>>
>>> for 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/548
>>>
>>> I 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 at 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 at 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 at 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 at 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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: v.out.ogr.diff
Type: text/x-patch
Size: 1514 bytes
Desc: not available
Url : http://lists.osgeo.org/pipermail/grass-dev/attachments/20090831/039f9a11/v.out.ogr.bin
More information about the grass-dev
mailing list