[GRASS-dev] bug in v.db.renamecol [was: Re: [GRASS-user] Rename multiple sqlite columns at once]

[Moritz Lennert]

Nikos,
(moving this to grass-dev)

On 13/11/08 20:15, Nikos Alexandris wrote:
> On Thu, 2008-11-13 at 10:28 +0100, Moritz Lennert wrote:
>> On 07/11/08 01:47, Nikos Alexandris wrote:
>>> On Fri, 2008-11-07 at 01:38 +0100, Moritz Lennert wrote:
> [...]
> 
>> Sorry, for not coming back on this earlier, but I think I found it now. 
>> The error (which also appears in other drivers, at least dbf), is due to 
>> the following line in v.db.renamecol:
>>
>> oldcoltype="`db.describe -c table=$table database=$database\
>> 		driver=$driver | grep $oldcol | cut -d':' -f3`"
>>
>> If you have two columns with the name of the first a subset of the name 
>> of the second (e.g. DATA and DATA2, or in the nc_spm_06 dataset's 
>> comm_colleges map, CC_ and CC_NAME and CCL_ and CCL_ID), the grep in the 
>> above line will find both lines and result in an invalid oldcoltype:
>>
>> GRASS 6.4.svn (nc_spm_06):~ > db.describe -c table=test | grep CC_ 
>> Column 7: CC_:CHARACTER:8
>> Column 8: CC_NAME:CHARACTER:30
>>
>> GRASS 6.4.svn (nc_spm_06):~ > db.describe -c table=test | grep CC_ | 
>> cut -d':' -f3
>> CHARACTER
>> CHARACTER
>>
>> GRASS 6.4.svn (nc_spm_06):~ > oldcoltype="`db.describe -c table=test | 
>> grep CC_ |  cut -d':' -f3`"
>> GRASS 6.4.svn (nc_spm_06):~ > echo $oldcoltype
>> CHARACTER CHARACTER
>>
>> Which then obviously leads to an error as 'CHARACTER CHARACTER' is not a 
>> valid column type.
>>
>> Can you confirm that this is the case for you as well ?
>> And actually this is not only a warning issue, but the column is lost 
>> because of this !
> 
> Well, not exactly. Columns are renamed in the end (even if some names
> are "subsets" of other column names) but it looses a column which was
> already in lower case!? See example below (using sqlite):
> 
> # some table
> db.describe befliegung_copy -c
> 
> ncols: 12
> nrows: 361
> Column 1: block_id:INTEGER:20
> Column 2: cat:INTEGER:20
> Column 3: AREA:DOUBLE PRECISION:20
> Column 4: PERIMETER:DOUBLE PRECISION:20
> Column 5: RAS2X2_:DOUBLE PRECISION:20
> Column 6: RAS2X2_ID:DOUBLE PRECISION:20
> Column 7: RAS2X2_NR:CHARACTER:13
> Column 8: block:CHARACTER:40
> Column 9: STAND:CHARACTER:16
> Column 10: BEFL_DATUM:CHARACTER:16
> Column 11: BEFL_JAHR:INTEGER:20
> Column 12: QUALITAET:CHARACTER:16

Testing with this table schema led me to notice that the error only 
occurs with RAS2X2 if it is of type INTEGER and CHARACTER, not with type 
DOUBLE PRECISION, although $oldtype also is DOUBLE PRECISION DOUBLE 
PRECISION CHARACTER in the latter case. The problem is a bit hidden by 
the fact that sqlite let's you create a column with any type you wish, 
but it comes back out in the v.db.dropcol part of v.db.renamecol as 
there the table goes through the filter of the GRASS sqlite driver. In 
the case of the dbf driver it already fails in the stage of the table 
creation, even with DOUBLE PRECISION.

The fact that it works with DOUBLE PRECISION with the sqlite driver is 
linked to the way the sqlite driver parses the column types (line 392 of 
db/drivers/sqlite/describe.c):


     if (sscanf(buf, "%s %s", word[0], word[1]) == 2) {
         if (streq(word[0], "double") && streq(word[1], "precision"))
             return DB_SQL_TYPE_DOUBLE_PRECISION;
         if (streq(word[0], "character") && streq(word[1], "varying"))
             return DB_SQL_TYPE_CHARACTER;
     }


An easy way around this problem is to modify v.db.renamecol:

--- SRC/GRASS/grass6_devel/scripts/v.db.renamecol/v.db.renamecol 
2008-11-06 14:00:04.000000000 +0100
+++ SRC/GRASS/grass6_devel/dist.i486-pc-linux-gnu/scripts/v.db.renamecol 
       2008-11-18 14:56:53.000000000 +0100
@@ -143,8 +143,8 @@
  fi

  # describe old col
-oldcoltype="`db.describe -c table=$table database=$database 
driver=$driver | grep $oldcol | cut -d':' -f3`"
-oldcollength=`db.describe -c table=$table database=$database 
driver=$driver | grep $oldcol | cut -d':' -f4`
+oldcoltype="`db.describe -c table=$table database=$database 
driver=$driver | grep $oldcol":" | cut -d':' -f3`"
+oldcollength=`db.describe -c table=$table database=$database 
driver=$driver | grep $oldcol":" | cut -d':' -f4`

This should only grep for the actual column and not any supersets of the 
column name.

Anybody object to me committing this ?

Moritz