[GRASS-dev] [GRASS GIS] #461: v.db.update crashes with updating shapefile connected with v.external

#461: v.db.update crashes with updating shapefile connected with v.external
----------------------+-----------------------------------------------------
Reporter: msieczka | Owner: grass-dev@lists.osgeo.org
     Type: defect | Status: new
Priority: critical | Milestone: 6.4.0
Component: Vector | Version: svn-develbranch6
Keywords: | Platform: Linux
      Cpu: x86-64 |
----------------------+-----------------------------------------------------
Backtrace:

{{{
GRASS 6.5.svn (javier):~ > gdb v.to.db
GNU gdb 6.8-debian
Copyright (C) 2008 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later
<http://gnu.org/licenses/gpl.html&gt;
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law. Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu"...
(gdb) run map=gshhs_shp opt=area units=k col=AREA
Starting program: /usr/local/grass-6.5.svn/bin/v.to.db map=gshhs_shp
opt=area units=k col=AREA
[Thread debugging using libthread_db enabled]
warning: Lowest section in /usr/lib/libicudata.so.38 is .hash at
0000000000000120
[New Thread 0x7fe8946c9710 (LWP 9814)]
Reading areas...
  100%

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fe8946c9710 (LWP 9814)]
0x00007fe89418f539 in db_get_column_sqltype (column=0x0) at column.c:107
107 return column->sqlDataType;
(gdb) bt
#0 0x00007fe89418f539 in db_get_column_sqltype (column=0x0) at
column.c:107
#1 0x00007fe8938f480d in db_select_int (driver=0x9237ba0, tab=0x98bb7f0
"gshhs", col=0x98b8ba0 "", where=0x0, pval=0x7fff9c7ff858) at select.c:138
#2 0x00000000004069f2 in update (Map=0x7fff9c7ff8c0) at update.c:53
#3 0x00000000004043e4 in main (argc=5, argv=0x7fff9c7fffd8) at main.c:84
}}}

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/461&gt;
GRASS GIS <http://grass.osgeo.org>

#461: v.db.update crashes with updating shapefile connected with v.external
-----------------------+----------------------------------------------------
  Reporter: msieczka | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: critical | Milestone: 6.4.0
Component: Vector | Version: svn-develbranch6
Resolution: | Keywords:
  Platform: Linux | Cpu: x86-64
-----------------------+----------------------------------------------------
Comment (by mlennert):

The title of this bug concerns v.db.update, but the backtrace is on
v.to.db.

Maciej, could you detail this a bit more ?

Moritz

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/461#comment:1&gt;
GRASS GIS <http://grass.osgeo.org>

#461: v.to.db crashes on a shapefile connected with v.external
-----------------------+----------------------------------------------------
  Reporter: msieczka | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: critical | Milestone: 6.4.0
Component: Vector | Version: svn-develbranch6
Resolution: | Keywords:
  Platform: Linux | Cpu: x86-64
-----------------------+----------------------------------------------------
Changes (by msieczka):

  * summary: v.db.update crashes with updating shapefile connected with
              v.external => v.to.db crashes on a shapefile
              connected with v.external

Comment:

Replying to [comment:1 mlennert]:
> The title of this bug concerns v.db.update, but the backtrace is on
v.to.db.

My bad. Correcting the title.

> Maciej, could you detail this a bit more ?

1. run v.external on some shapefile

2. try to populate the shapefile's datatable with v.to.db - crash

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/461#comment:2&gt;
GRASS GIS <http://grass.osgeo.org>

#461: v.to.db crashes on a shapefile connected with v.external
-----------------------+----------------------------------------------------
  Reporter: msieczka | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: critical | Milestone: 6.4.0
Component: Vector | Version: svn-develbranch6
Resolution: | Keywords:
  Platform: Linux | Cpu: x86-64
-----------------------+----------------------------------------------------
Comment (by neteler):

The manual
http://grass.osgeo.org/grass64/manuals/html64_user/v.external.html says:
Creates a new vector as a '''read-only''' link to OGR layer. Apparently
some error trapping is missing which tells the user that v.external is a
read-only link.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/461#comment:3&gt;
GRASS GIS <http://grass.osgeo.org>

#461: v.to.db crashes on a shapefile connected with v.external
-----------------------+----------------------------------------------------
  Reporter: msieczka | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: critical | Milestone: 6.4.0
Component: Vector | Version: svn-develbranch6
Resolution: | Keywords:
  Platform: Linux | Cpu: x86-64
-----------------------+----------------------------------------------------
Comment (by neteler):

Do we have a function in order to test if an attribute table is open for
writing?

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/461#comment:4&gt;
GRASS GIS <http://grass.osgeo.org>

#461: v.to.db crashes on a shapefile connected with v.external
-----------------------+----------------------------------------------------
  Reporter: msieczka | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: critical | Milestone: 6.4.0
Component: Vector | Version: svn-develbranch6
Resolution: | Keywords:
  Platform: Linux | Cpu: x86-64
-----------------------+----------------------------------------------------
Comment (by hamish):

[copied from the ML]

>> -> still valid?
Markus Metz wrote:
> I think so because attribute queries are not possible on OGR
> vectors connected with v.external (most OGR vector layers don't
> have a grass-like key column). I think Martin L fixed that in
> grass7.
...
> Attribute table operations (query, upload values) are still not
> possible in grass7 for OGR vector layers that don't have a key
> column, i.e. for most OGR-supported formats. Still valid for both
> v.external and direct OGR link.

We may not be able to add the requested functionality for the next
release, but can we do anything about replacing the segfault with a
G_fatal_error()?

Hamish

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/461#comment:5&gt;
GRASS GIS <http://grass.osgeo.org>

#461: v.to.db crashes on a shapefile connected with v.external
-----------------------+----------------------------------------------------
  Reporter: msieczka | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: critical | Milestone: 6.4.0
Component: Vector | Version: svn-develbranch6
Resolution: | Keywords:
  Platform: Linux | Cpu: x86-64
-----------------------+----------------------------------------------------
Comment (by glynn):

Replying to [comment:5 hamish]:
> We may not be able to add the requested functionality for the next
release, but can we do anything about replacing the segfault with a
G_fatal_error()?

The following is based upon the backtrace, and is completely untested:

{{{
--- lib/db/dbmi_client/select.c (revision 40258)
+++ lib/db/dbmi_client/select.c (working copy)
@@ -134,6 +134,8 @@

      table = db_get_cursor_table(&cursor);
      column = db_get_table_column(table, 0); /* first column */
+ if (!column)
+ G_fatal_error(_("Table has no columns"));
      value = db_get_column_value(column);
      type = db_get_column_sqltype(column);
      type = db_sqltype_to_Ctype(type);
}}}

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/461#comment:6&gt;
GRASS GIS <http://grass.osgeo.org>

#461: v.to.db crashes on a shapefile connected with v.external
-----------------------+----------------------------------------------------
  Reporter: msieczka | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: critical | Milestone: 6.4.0
Component: Vector | Version: svn-develbranch6
Resolution: | Keywords:
  Platform: Linux | Cpu: x86-64
-----------------------+----------------------------------------------------
Comment (by neteler):

The suggested patch looks good:

{{{
# NC dataset
v.external medfacs.shp out=medfacs layer=medfacs

# unpatched:
GRASS 6.4.0svn (nc_spm_08):~/books/kluwerbook/data3rd/wake > v.to.db
medfacs option=area columns=AREA
Reading areas...
Segmentation fault

# patched:
GRASS 6.4.0svn (nc_spm_08):~/books/kluwerbook/data3rd/wake > v.to.db
medfacs option=area columns=AREA
Reading areas...
ERROR: Table has no columns
}}}

It may not be immediate to derive the problem in this particular case but
a healthy test.
Please submit and I'll backport.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/461#comment:7&gt;
GRASS GIS <http://grass.osgeo.org>

#461: v.to.db crashes on a shapefile connected with v.external
-----------------------+----------------------------------------------------
  Reporter: msieczka | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: critical | Milestone: 6.4.0
Component: Vector | Version: svn-develbranch6
Resolution: | Keywords:
  Platform: Linux | Cpu: x86-64
-----------------------+----------------------------------------------------
Comment (by mmetz):

Replying to [comment:6 glynn]:
> Replying to [comment:5 hamish]:
> > We may not be able to add the requested functionality for the next
release, but can we do anything about replacing the segfault with a
G_fatal_error()?
>
> The following is based upon the backtrace, and is completely untested:
>
{{{
  --- lib/db/dbmi_client/select.c (revision 40258)
  +++ lib/db/dbmi_client/select.c (working copy)
  @@ -134,6 +134,8 @@

       table = db_get_cursor_table(&cursor);
       column = db_get_table_column(table, 0); /* first column */
  + if (!column)
  + G_fatal_error(_("Table has no columns"));
       value = db_get_column_value(column);
       type = db_get_column_sqltype(column);
      type = db_sqltype_to_Ctype(type);
}}}

Going a bit further to the root of the problem: in this case const char
*col passed to db_select_int() in lib/db/dbmi_client/select.c is a zero
length string, you could do for 'col' the same test that's done for
'where' and exit with an error.

Even further down, the question is, what to do if OGR_L_GetFIDColumn()
doesn't return FID name, to cite a comment from Vect_read_dblinks().

Markus M

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/461#comment:8&gt;
GRASS GIS <http://grass.osgeo.org>

#461: v.to.db crashes on a shapefile connected with v.external
-----------------------+----------------------------------------------------
  Reporter: msieczka | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: critical | Milestone: 6.4.0
Component: Vector | Version: svn-develbranch6
Resolution: | Keywords:
  Platform: Linux | Cpu: x86-64
-----------------------+----------------------------------------------------
Comment (by mmetz):

Replying to [comment:8 mmetz]:
>
> Going a bit further to the root of the problem: in this case const char
*col passed to db_select_int() in lib/db/dbmi_client/select.c is a zero
length string, you could do for 'col' the same test that's done for
'where' and exit with an error.

The following patch works for me

{{{
Index: select.c

--- select.c (revision 40416)
+++ select.c (working copy)
@@ -115,6 +115,9 @@

      G_debug(3, "db_select_int()");

+ if (col == NULL || strlen(col) == 0)
+ G_fatal_error(_("Missing column name"));
+
      /* allocate */
      alloc = 1000;
      val = (int *)G_malloc(alloc * sizeof(int));
@@ -203,6 +206,12 @@
      dbValue *value;
      dbTable *table;

+ if (key == NULL || strlen(key) == 0)
+ G_fatal_error(_("Missing key column name"));
+
+ if (col == NULL || strlen(col) == 0)
+ G_fatal_error(_("Missing column name"));
+
      G_zero(val, sizeof(dbValue));
      sprintf(buf, "SELECT %s FROM %s WHERE %s = %d\n", col, tab, key, id);
      db_init_string(&stmt);
@@ -259,6 +268,12 @@

      G_debug(3, "db_select_db_select_CatValArray ()");

+ if (key == NULL || strlen(key) == 0)
+ G_fatal_error(_("Missing key column name"));
+
+ if (col == NULL || strlen(col) == 0)
+ G_fatal_error(_("Missing column name"));
+
      db_init_string(&stmt);

      sprintf(buf, "SELECT %s, %s FROM %s", key, col, tab);
}}}

Problem is when a module gives a broken sql string to
db_execute_immediate(), hopefully that aborts with an error and not a
segfault.

Glynn's patch could be useful in other circumstances too.

Markus M

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/461#comment:9&gt;
GRASS GIS <http://grass.osgeo.org>

#461: v.to.db crashes on a shapefile connected with v.external
-----------------------+----------------------------------------------------
  Reporter: msieczka | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: critical | Milestone: 6.4.0
Component: Vector | Version: svn-develbranch6
Resolution: | Keywords:
  Platform: Linux | Cpu: x86-64
-----------------------+----------------------------------------------------
Comment (by mmetz):

Regarding the recent discussion about GRASS libraries and G_fatal_error()
as well as the description of these functions, maybe rather issue a
warning instead of an error and return -1? That would work because (at
least) v.to.db checks the return value of db_select_int() and exits with
an error if db_select_int() returns -1.

Markus M

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/461#comment:10&gt;
GRASS GIS <http://grass.osgeo.org>

#461: v.to.db crashes on a shapefile connected with v.external
-----------------------+----------------------------------------------------
  Reporter: msieczka | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: critical | Milestone: 6.4.0
Component: Vector | Version: svn-develbranch6
Resolution: | Keywords:
  Platform: Linux | Cpu: x86-64
-----------------------+----------------------------------------------------
Comment (by mmetz):

segfault fixed in r40568 (relbr6), r40569 (devbr6), r40570 (trunk).

v.to.db as all modules working with attribute tables still doesn't work
with OGR-linked vectors, I guess that will only get fixed in grass7 if
ever. I suggest to change milestone to 7.0 and priority to major.

Markus M

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/461#comment:11&gt;
GRASS GIS <http://grass.osgeo.org>

#461: v.to.db crashes on a shapefile connected with v.external
-----------------------+----------------------------------------------------
  Reporter: msieczka | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 7.0.0
Component: Vector | Version: svn-trunk
Resolution: | Keywords: v.external, v.to.db
  Platform: Linux | Cpu: x86-64
-----------------------+----------------------------------------------------
Changes (by hamish):

  * keywords: => v.external, v.to.db
  * priority: critical => major
  * version: svn-develbranch6 => svn-trunk
  * milestone: 6.4.0 => 7.0.0

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/461#comment:12&gt;
GRASS GIS <http://grass.osgeo.org>

#461: v.to.db crashes on a shapefile connected with v.external
---------------------------------+------------------------------------------
Reporter: msieczka | Owner: grass-dev@…
     Type: defect | Status: new
Priority: major | Milestone: 7.0.0
Component: Vector | Version: svn-trunk
Keywords: v.external, v.to.db | Platform: Linux
      Cpu: x86-64 |
---------------------------------+------------------------------------------

Comment(by mlennert):

What is the status of this bug ? Any chance v.external will ever offer
write access to attribute tables of OGR-linked vectors ? With a recent
grass7 I still get the "Missing column name" message. I actually do not
find that message very clear. My first reaction was to check whether I had
forgotten to indicate the column I wanted to upload to. Maybe some hint
about linked maps would be helpful.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/461#comment:13&gt;
GRASS GIS <http://grass.osgeo.org>

#461: v.to.db crashes on a shapefile connected with v.external
---------------------------------+------------------------------------------
Reporter: msieczka | Owner: grass-dev@…
     Type: defect | Status: new
Priority: major | Milestone: 7.0.0
Component: Vector | Version: svn-trunk
Keywords: v.external, v.to.db | Platform: Linux
      Cpu: x86-64 |
---------------------------------+------------------------------------------
Changes (by martinl):

* cc: martinl (added)

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/461#comment:14&gt;
GRASS GIS <http://grass.osgeo.org>