I just tried to add a column to an existing attribute table in
PostgreSQL and found a misleading help message. The help message of
v.db.addcol/v.db.addtable says "types depend on database backend, but
all support VARCHAR(), INT, DOUBLE and DATE". I'm not so sure if the
postgres driver in GRASS supports the DOUBLE type internally. However,
since v.db.add* scripts directly run db.execute which calls
db_driver_execute_immediate(), and PostgreSQL only supports DOUBLE
PRECISION and REAL for floating-point numbers, the help message is not
valid at least for PostgreSQL.
If those three data types are supposed to be common data types for all
database backends, v.db.add* scripts need to convert DOUBLE to a
corresponding valid data type based on the database backend. Otherwise,
IMO, the help message needs to be revised to avoid confusion.
I just tried to add a column to an existing attribute table in
PostgreSQL and found a misleading help message. The help message of
v.db.addcol/v.db.addtable says "types depend on database backend, but
all support VARCHAR(), INT, DOUBLE and DATE". I'm not so sure if the
postgres driver in GRASS supports the DOUBLE type internally. However,
since v.db.add* scripts directly run db.execute which calls
db_driver_execute_immediate(), and PostgreSQL only supports DOUBLE
PRECISION and REAL for floating-point numbers, the help message is not
valid at least for PostgreSQL.
If those three data types are supposed to be common data types for all
database backends, v.db.add* scripts need to convert DOUBLE to a
corresponding valid data type based on the database backend. Otherwise,
IMO, the help message needs to be revised to avoid confusion.
I am the culprit of this formulation. I agree that it should be adapted, but someone would have to check which common types are available across all backends...
I did some research about SQL2003-compliant data types and found the
SQL2003 specification (http://savage.net.au/SQL/sql-2003-2.bnf.html).
According to the specification, VARCHAR, INT, DOUBLE PRECISION, and DATE
are standard data types, and "MySQL treats DOUBLE as a synonym for
DOUBLE PRECISION (a non-standard extension)"
(http://dev.mysql.com/doc/refman/5.0/en/numeric-types.html). Since the
built-in SQL parser supports "DOUBLE PRECISION", the dbf backend has no
problem with this data type. SQLite is also fine because it's typeless.
However, PostgreSQL does not recognize "DOUBLE", so I suggest to adapt
the help message.
What do you think?
Huidae
On Thu, May 29, 2008 at 11:11:10PM +0200, Moritz Lennert wrote:
On 29/05/08 09:25, Huidae Cho wrote:
>Hi,
>I just tried to add a column to an existing attribute table in
>PostgreSQL and found a misleading help message. The help message of
>v.db.addcol/v.db.addtable says "types depend on database backend, but
>all support VARCHAR(), INT, DOUBLE and DATE". I'm not so sure if the
>postgres driver in GRASS supports the DOUBLE type internally. However,
>since v.db.add* scripts directly run db.execute which calls
>db_driver_execute_immediate(), and PostgreSQL only supports DOUBLE
>PRECISION and REAL for floating-point numbers, the help message is not
>valid at least for PostgreSQL.
>If those three data types are supposed to be common data types for all
>database backends, v.db.add* scripts need to convert DOUBLE to a
>corresponding valid data type based on the database backend. Otherwise,
>IMO, the help message needs to be revised to avoid confusion.
I am the culprit of this formulation. I agree that it should be adapted, but
someone would have to check which common types are available across all
backends...
On Sat, Jun 28, 2008 at 08:31:32AM -0700, Hamish wrote:
Hi,
re. "sep= changed to separator= while keeping backward compatibility"
two points-
) I think it should be changed to fs= to match the rest of GRASS.
) backwards compatibility with grass7 (trunk) is nice to do, but it is not at all guaranteed. ie we can break things as we like. it's the dev-branch6 where backwards compatibility must be strictly followed.
Hi,
Yes, fs= looks better than separator=. If there are no objections to
breaking this backward compatibility, I'll change it again to fs=.
cheers,
Hamish
ps- glad to see the speedups! Now we only have to rewrite that module in C.. parsing the g.list output will always be a little fragile/slow, and a goal of grass7 is to do without helping unix tools so there are fewer problems on the MS Windows port.
I agree with you that something needs to be done for grass7. Ideally,
g.mlist/g.mremove should be removed by extending g.list/g.remove.
Probably, grass7 will require one more library by default that handles
regular expressions (e.g., POSIX regex, pcre). More generally, we could
implement regex expansion into standard options (too much?).
On Sat, Jun 28, 2008 at 01:15:47PM -0500, Huidae Cho wrote:
On Sat, Jun 28, 2008 at 08:31:32AM -0700, Hamish wrote:
> Hi,
>
> re. "sep= changed to separator= while keeping backward compatibility"
>
> two points-
> ) I think it should be changed to fs= to match the rest of GRASS.
>
> ) backwards compatibility with grass7 (trunk) is nice to do, but it is not at all guaranteed. ie we can break things as we like. it's the dev-branch6 where backwards compatibility must be strictly followed.
>
Hi,
Yes, fs= looks better than separator=. If there are no objections to
breaking this backward compatibility, I'll change it again to fs=.
>
> cheers,
> Hamish
>
> ps- glad to see the speedups! Now we only have to rewrite that module in C.. parsing the g.list output will always be a little fragile/slow, and a goal of grass7 is to do without helping unix tools so there are fewer problems on the MS Windows port.
>
BTW, does this mean we cannot take advantage of backticks (e.g.,
input=`g.mlist pat=time*`) any more? Then is the only option regex
support for standard options? Or is it just g.mlist/g.mremove?
I agree with you that something needs to be done for grass7. Ideally,
g.mlist/g.mremove should be removed by extending g.list/g.remove.
Probably, grass7 will require one more library by default that handles
regular expressions (e.g., POSIX regex, pcre). More generally, we could
implement regex expansion into standard options (too much?).
> re. "sep= changed to separator= while keeping backward compatibility"
>
> two points-
> ) I think it should be changed to fs= to match the rest of GRASS.
>
> ) backwards compatibility with grass7 (trunk) is nice to do, but
> ) it is not at all guaranteed. ie we can break things as we like. it's
> ) the dev-branch6 where backwards compatibility must be strictly
> ) followed.
I would suggest separator=, changing the rest of GRASS to match if
compatibility is desired.
fs= isn't immediately clear, while separator= is, and can be
abbreviated to sep=.
More generally, we should try to avoid using abbreviations in option
names. The user can abbreviate them if they desire, but should have
the option of using a non-abbreviated name for clarity.
I know that creates problems with multi-word names. I will look into
the possibility of supporting multi-word abbreviations at some point.
If that works out, it would be possible to call it field_separator and
abbreviate it to fs=, fsep= etc (not sure about sep=).
I did some research about SQL2003-compliant data types and found the
SQL2003 specification (http://savage.net.au/SQL/sql-2003-2.bnf.html).
According to the specification, VARCHAR, INT, DOUBLE PRECISION, and DATE
are standard data types, and "MySQL treats DOUBLE as a synonym for
DOUBLE PRECISION (a non-standard extension)"
(http://dev.mysql.com/doc/refman/5.0/en/numeric-types.html). Since the
built-in SQL parser supports "DOUBLE PRECISION", the dbf backend has no
problem with this data type. SQLite is also fine because it's typeless.
However, PostgreSQL does not recognize "DOUBLE", so I suggest to adapt
the help message.
What do you think?
Thanks for your research !
Please go ahead to change if you want to, otherwise I'll do it. I also think the rest of the text needs some improvement to make it clear what exactly is expected.