Opened 2 years ago

Closed 12 months ago

#3466 closed enhancement (fixed)

v.to.db: check if column exists before running

Reported by: sbl Owned by: grass-dev@…
Priority: normal Milestone: 7.6.0
Component: Vector Version: svn-releasebranch74
Keywords: v.to.db, column Cc:
CPU: All Platform: All

Description

When updating a vector attribute column with v.to.db, the module does not check if the column to be updated exists.

It starts anyway, and the user gets lots of error messages like the following:

DBMI-SQLite driver error:
Error in sqlite3_prepare():
no such column: length

Would be convenient if v.to.db would check if columns exist and fails with respective error message if not.

Change History (14)

comment:1 Changed 2 years ago by sbl

Version: unspecifiedsvn-releasebranch74

comment:2 Changed 22 months ago by martinl

Milestone: 7.5.07.6.0

Milestone renamed

comment:3 Changed 15 months ago by mmetz

Type: defectenhancement

Changing to enhancement because the result is not incorrect if the column(s) don't exist. The requested check if the columns to be updated exist will not change the result.

comment:4 Changed 15 months ago by neteler

Source code to be reused is here (loop over column names):

https://trac.osgeo.org/grass/browser/grass/trunk/vector/v.distance/main.c#L524

comment:5 Changed 15 months ago by mlennert

While we are discussing this: couldn't the module just create the columns if they don't exist, yet ?

I think this should be the default behavior of all modules writing output into DB columns. Possibly with a flag to determine if overwriting an existing column is ok.

comment:6 Changed 15 months ago by pvanbosgeo

+1 for creating columns if they don't exist (and the flag to determine if overwriting is OK) as default behavior

comment:7 in reply to:  5 Changed 15 months ago by mlennert

Replying to mlennert:

While we are discussing this: couldn't the module just create the columns if they don't exist, yet ?

I think this should be the default behavior of all modules writing output into DB columns. Possibly with a flag to determine if overwriting an existing column is ok.

See the -c flag in v.rast.stats.

comment:8 in reply to:  6 ; Changed 15 months ago by veroandreo

Replying to pvanbosgeo:

+1 for creating columns if they don't exist (and the flag to determine if overwriting is OK) as default behavior

+1

comment:9 in reply to:  8 ; Changed 15 months ago by mmetz

Replying to veroandreo:

Replying to pvanbosgeo:

+1 for creating columns if they don't exist (and the flag to determine if overwriting is OK) as default behavior

+1

+1

Just for clarification:

  • if the column(s) exist, require --overwrite
  • if the column(s) do not exist, create them with the appropriate type

Additionally,

  • if the column(s) exist, check if the column type is correct

comment:10 in reply to:  9 ; Changed 15 months ago by mlennert

Replying to mmetz:

Replying to veroandreo:

Replying to pvanbosgeo:

+1 for creating columns if they don't exist (and the flag to determine if overwriting is OK) as default behavior

+1

+1

Just for clarification:

  • if the column(s) exist, require --overwrite
  • if the column(s) do not exist, create them with the appropriate type

Additionally,

  • if the column(s) exist, check if the column type is correct

So, you are suggesting no -c flag as in v.rast.stats ? Should that flag in v.rast.stats be deprecated in order to maintain coherence ?

comment:11 in reply to:  10 Changed 15 months ago by mmetz

Replying to mlennert:

Replying to mmetz:

Replying to veroandreo:

Replying to pvanbosgeo:

+1 for creating columns if they don't exist (and the flag to determine if overwriting is OK) as default behavior

+1

+1

Just for clarification:

  • if the column(s) exist, require --overwrite
  • if the column(s) do not exist, create them with the appropriate type

Additionally,

  • if the column(s) exist, check if the column type is correct

So, you are suggesting no -c flag as in v.rast.stats ? Should that flag in v.rast.stats be deprecated in order to maintain coherence ?

Yes. The reasoning is, if a column exists and a module writes values to that column, it will overwrite existing values in that column. Thus requiring the overwrite flag to overwrite existing values seems appropriate. The overwrite flag is not required if the module both creates the column and populates the column.

Makes sense?

comment:12 Changed 15 months ago by mmetz

In 73236:

v.to.db: check if column(s) exist before updating, includes check for correct column type, create column(s) of not existing, see #3466

comment:13 Changed 15 months ago by mmetz

In 73237:

v.to.db: check if column(s) exist before updating, includes check for correct column type, create column(s) of not existing, see #3466 (backport trunk r73236)

comment:14 Changed 12 months ago by mlennert

Resolution: fixed
Status: newclosed

Closing this ticket as this enhancement has been applied to trunk and 7.6 release.

Note: See TracTickets for help on using tickets.