[GRASS-dev] v.db.update: any reason to keep both value and qcolumn parameter ?

In v.db.update we currently have:

#%option
#% key: value
#% type: string
#% description: Value to update the column with, can be (combination of) other column(s)
#% required : no
#%end
#%option
#% key: qcolumn
#% type: string
#% description: Column to query
#% required : no
#%end

'qcolumn' was added in a later stage, AFAIK mostly because of a misunderstanding of 'value'. As said in the help text, value can be either a value, a name of another column or a combination of other columns, so it already comprises qcolumn.

However, in its current version (since rev 23578), value is now adapted according to column type, which means that you cannot use column operations on varchar columns (e.g. concatenation).

So, we should either:

- change the doc to reflect that value should only be a nominal value, or
- go back to the original, i.e. no qcolumn and no type checking for value, so that it's up to the user to use correct quoting

Any preferences ?

Moritz

On Tue, Sep 30, 2008 at 10:58 AM, Moritz Lennert
<mlennert@club.worldonline.be> wrote:

In v.db.update we currently have:

#%option
#% key: value
#% type: string
#% description: Value to update the column with, can be (combination of)
other column(s)
#% required : no
#%end
#%option
#% key: qcolumn
#% type: string
#% description: Column to query
#% required : no
#%end

'qcolumn' was added in a later stage, AFAIK mostly because of a
misunderstanding of 'value'. As said in the help text, value can be either a
value, a name of another column or a combination of other columns, so it
already comprises qcolumn.

Only today I learned that (I added qcolumn if I remember correctly)
:slight_smile:

However, in its current version (since rev 23578), value is now adapted
according to column type, which means that you cannot use column operations
on varchar columns (e.g. concatenation).

So, we should either:

- change the doc to reflect that value should only be a nominal value, or

yes

- go back to the original, i.e. no qcolumn and no type checking for value,
so that it's up to the user to use correct quoting

No - I don't find it obvious at all that "value" could be a column name.
qcolumn also exists in other commands AFAIK, to that's consistent.

Markus

Markus Neteler wrote:

> However, in its current version (since rev 23578), value is now adapted
> according to column type, which means that you cannot use column operations
> on varchar columns (e.g. concatenation).
>
> So, we should either:
>
> - change the doc to reflect that value should only be a nominal value, or

yes

> - go back to the original, i.e. no qcolumn and no type checking for value,
> so that it's up to the user to use correct quoting

No - I don't find it obvious at all that "value" could be a column name.
qcolumn also exists in other commands AFAIK, to that's consistent.

Another option: add expression=, which is just used verbatim as the
RHS of the assignment (you can already use qcolumn= this way, but its
name and description makes that counter-intuitive). Optionally remove
value= and/or qcolumn=.

--
Glynn Clements <glynn@gclements.plus.com>

However, in its current version (since rev 23578), value is now
adapted according to column type, which means that you cannot
use column operations on varchar columns (e.g. concatenation).

this puts it at odds with the value= option description, both WRT quotes
and combinations:
"Value to update the column with (varchar values have to be in single
  quotes, e.g. 'grass'), can be (combination of) other column(s)"

So, we should either:

- change the doc to reflect that value should only be a nominal
value, or

Markus:

> yes
>

- go back to the original, i.e. no qcolumn and no type checking for
value, so that it's up to the user to use correct quoting

>
> No - I don't find it obvious at all that "value" could be a column
> name.
> qcolumn also exists in other commands AFAIK, to that's consistent.

Glynn:

Another option: add expression=, which is just used verbatim as the
RHS of the assignment (you can already use qcolumn= this way, but its
name and description makes that counter-intuitive).
Optionally remove value= and/or qcolumn=.

Replacing qcolumn= with expression= sounds good to me. It's more to the
point.

At which point value= becomes a little redundant, but it helps to avoid
a some minor 'quote' proofing when used from the shell.

Hamish

On 30/09/08 16:44, Glynn Clements wrote:

Markus Neteler wrote:

However, in its current version (since rev 23578), value is now adapted
according to column type, which means that you cannot use column operations
on varchar columns (e.g. concatenation).

So, we should either:

- change the doc to reflect that value should only be a nominal value, or

yes

- go back to the original, i.e. no qcolumn and no type checking for value,
so that it's up to the user to use correct quoting

No - I don't find it obvious at all that "value" could be a column name.
qcolumn also exists in other commands AFAIK, to that's consistent.

Another option: add expression=, which is just used verbatim as the
RHS of the assignment (you can already use qcolumn= this way, but its
name and description makes that counter-intuitive).

That was what the original value= was for, admittedly with a bad choice of parameter name.

Moritz