[GRASS-dev] [GRASS-SVN] r73995 - grass/trunk/scripts/v.db.addtable

Hi,

On Tue, Jan 22, 2019 at 3:51 AM <svn_grass@osgeo.org> wrote:

Author: hcho
Date: 2019-01-21 18:51:33 -0800 (Mon, 21 Jan 2019)
New Revision: 73995

Modified:
   grass/trunk/scripts/v.db.addtable/v.db.addtable.py
Log:
v.db.addtable: Do not create unique index from this script; v.db.connect will try to create it again causing some drivers to fail (PostgreSQL)

Modified: grass/trunk/scripts/v.db.addtable/v.db.addtable.py

--- grass/trunk/scripts/v.db.addtable/v.db.addtable.py 2019-01-21 22:37:59 UTC (rev 73994)
+++ grass/trunk/scripts/v.db.addtable/v.db.addtable.py 2019-01-22 02:51:33 UTC (rev 73995)
@@ -139,16 +139,6 @@
         except CalledModuleError:
             grass.fatal(_("Unable to create table <%s>") % table)

- # create index, see db/driver/*/index.c
- if driver != "dbf":
- sql = "CREATE UNIQUE INDEX %s_%s ON %s (%s)" % (table, key, table, key)
- try:
- grass.run_command('db.execute',
- database=database, driver=driver, sql=sql)
- except:
- grass.warning(_("Unable to create index on table <%s>") % table)
- pass
-
     # connect the map to the DB:
     if schema:
         table = '{schema}.{table}'.format(schema=schema, table=table)

just a conceptual question:
... are we sure that this index creation removal never leads to a
table without index?

Markus

Markus,

If there is a linked table, v.db.addtable stops in line 106. If not, this script doesn’t create a unique index and calls v.db.connect. v.db.connect adds a db link in line 317 and creates a unique index (db_create_index2) in line 334 if linking was successful. SQLite didn’t complain when both modules created the same unique index, but PostgreSQL failed in v.db.connect (2nd time creating the same unique index). Not sure which code was added first/later. I think it’s more of how we design both modules. v.db.connect will fail if we try to link a table with a unique index. Isn’t v.db.connect supposed to “just” connect a table to a layer (without creating any database objects like index)? Which module should be responsible for creating unique indices?

Before this commit:

  1. v.db.addtable creates unique index
  2. v.db.connect tries to create unique index again ==> fatal error

After this commit:

  1. v.db.addtable doesn’t create unique index
  2. v.db.connect creates unique index

Probably, it should be:

  1. v.db.addtable should create unique index
  2. v.db.connect shouldn’t try to create unique index? Just “connect”…

Regards,
Huidae

···

Huidae Cho, Ph.D., GISP, PE (MD), CFM, M.ASCE
Open Source GIS Developer, GRASS GIS Development Team

Or maybe, to be more flexible…

  1. v.db.addtable doesn’t create unique index
  2. v.db.connect creates unique index “only if” there is no unique index

Just my 2 cents

···

Huidae Cho, Ph.D., GISP, PE (MD), CFM, M.ASCE
Open Source GIS Developer, GRASS GIS Development Team

On Sat, Jan 26, 2019 at 2:50 PM Huidae Cho <grass4u@gmail.com> wrote:

Markus,

If there is a linked table, v.db.addtable stops in line 106. If not, this script doesn’t create a unique index and calls v.db.connect. v.db.connect adds a db link in line 317 and creates a unique index (db_create_index2) in line 334 if linking was successful. SQLite didn’t complain when both modules created the same unique index, but PostgreSQL failed in v.db.connect (2nd time creating the same unique index). Not sure which code was added first/later. I think it’s more of how we design both modules. v.db.connect will fail if we try to link a table with a unique index. Isn’t v.db.connect supposed to “just” connect a table to a layer (without creating any database objects like index)? Which module should be responsible for creating unique indices?

Before this commit:

  1. v.db.addtable creates unique index
  2. v.db.connect tries to create unique index again ==> fatal error

After this commit:

  1. v.db.addtable doesn’t create unique index
  2. v.db.connect creates unique index

Probably, it should be:

  1. v.db.addtable should create unique index
  1. v.db.connect shouldn’t try to create unique index? Just “connect”…

Yes, this is the preferred solution. Consider v.db.connect -d followed by v.db.connect, i.e. disconnecting a table and then reconnecting the same table: in this case v.db.connect should also fail with PostgreSQL. IMHO, a unique index should only be created when the table is created.

Markus M

Regards,
Huidae

On Sat, Jan 26, 2019 at 7:39 AM Markus Neteler <neteler@osgeo.org> wrote:

Hi,

On Tue, Jan 22, 2019 at 3:51 AM <svn_grass@osgeo.org> wrote:

Author: hcho
Date: 2019-01-21 18:51:33 -0800 (Mon, 21 Jan 2019)
New Revision: 73995

Modified:
grass/trunk/scripts/v.db.addtable/v.db.addtable.py
Log:
v.db.addtable: Do not create unique index from this script; v.db.connect will try to create it again causing some drivers to fail (PostgreSQL)

Modified: grass/trunk/scripts/v.db.addtable/v.db.addtable.py

— grass/trunk/scripts/v.db.addtable/v.db.addtable.py 2019-01-21 22:37:59 UTC (rev 73994)
+++ grass/trunk/scripts/v.db.addtable/v.db.addtable.py 2019-01-22 02:51:33 UTC (rev 73995)
@@ -139,16 +139,6 @@
except CalledModuleError:
grass.fatal(_(“Unable to create table <%s>”) % table)

  • create index, see db/driver/*/index.c

  • if driver != “dbf”:
  • sql = “CREATE UNIQUE INDEX %s_%s ON %s (%s)” % (table, key, table, key)
  • try:
  • grass.run_command(‘db.execute’,
  • database=database, driver=driver, sql=sql)
  • except:
  • grass.warning(_(“Unable to create index on table <%s>”) % table)
  • pass

connect the map to the DB:

if schema:
table = ‘{schema}.{table}’.format(schema=schema, table=table)

just a conceptual question:
… are we sure that this index creation removal never leads to a
table without index?

Markus


Huidae Cho, Ph.D., GISP, PE (MD), CFM, M.ASCE
Open Source GIS Developer, GRASS GIS Development Team


grass-dev mailing list
grass-dev@lists.osgeo.org
https://lists.osgeo.org/mailman/listinfo/grass-dev

On Sun, Jan 27, 2019 at 11:21 AM Markus Metz <markus.metz.giswork@gmail.com> wrote:

On Sat, Jan 26, 2019 at 2:50 PM Huidae Cho <grass4u@gmail.com> wrote:

Markus,

If there is a linked table, v.db.addtable stops in line 106. If not, this script doesn’t create a unique index and calls v.db.connect. v.db.connect adds a db link in line 317 and creates a unique index (db_create_index2) in line 334 if linking was successful. SQLite didn’t complain when both modules created the same unique index, but PostgreSQL failed in v.db.connect (2nd time creating the same unique index).

What do you mean with “failed”? v.db.connect will issue a warning if it can not create an index but finish successfully.

Markus M

Not sure which code was added first/later. I think it’s more of how we design both modules. v.db.connect will fail if we try to link a table with a unique index. Isn’t v.db.connect supposed to “just” connect a table to a layer (without creating any database objects like index)? Which module should be responsible for creating unique indices?

Before this commit:

  1. v.db.addtable creates unique index
  2. v.db.connect tries to create unique index again ==> fatal error

After this commit:

  1. v.db.addtable doesn’t create unique index
  2. v.db.connect creates unique index

Probably, it should be:

  1. v.db.addtable should create unique index
  2. v.db.connect shouldn’t try to create unique index? Just “connect”…

Yes, this is the preferred solution. Consider v.db.connect -d followed by v.db.connect, i.e. disconnecting a table and then reconnecting the same table: in this case v.db.connect should also fail with PostgreSQL. IMHO, a unique index should only be created when the table is created.

Markus M

Regards,
Huidae

On Sat, Jan 26, 2019 at 7:39 AM Markus Neteler <neteler@osgeo.org> wrote:

Hi,

On Tue, Jan 22, 2019 at 3:51 AM <svn_grass@osgeo.org> wrote:

Author: hcho
Date: 2019-01-21 18:51:33 -0800 (Mon, 21 Jan 2019)
New Revision: 73995

Modified:
grass/trunk/scripts/v.db.addtable/v.db.addtable.py
Log:
v.db.addtable: Do not create unique index from this script; v.db.connect will try to create it again causing some drivers to fail (PostgreSQL)

Modified: grass/trunk/scripts/v.db.addtable/v.db.addtable.py

— grass/trunk/scripts/v.db.addtable/v.db.addtable.py 2019-01-21 22:37:59 UTC (rev 73994)
+++ grass/trunk/scripts/v.db.addtable/v.db.addtable.py 2019-01-22 02:51:33 UTC (rev 73995)
@@ -139,16 +139,6 @@
except CalledModuleError:
grass.fatal(_(“Unable to create table <%s>”) % table)

  • create index, see db/driver/*/index.c

  • if driver != “dbf”:
  • sql = “CREATE UNIQUE INDEX %s_%s ON %s (%s)” % (table, key, table, key)
  • try:
  • grass.run_command(‘db.execute’,
  • database=database, driver=driver, sql=sql)
  • except:
  • grass.warning(_(“Unable to create index on table <%s>”) % table)
  • pass

connect the map to the DB:

if schema:
table = ‘{schema}.{table}’.format(schema=schema, table=table)

just a conceptual question:
… are we sure that this index creation removal never leads to a
table without index?

Markus


Huidae Cho, Ph.D., GISP, PE (MD), CFM, M.ASCE
Open Source GIS Developer, GRASS GIS Development Team


grass-dev mailing list
grass-dev@lists.osgeo.org
https://lists.osgeo.org/mailman/listinfo/grass-dev

You’re right. It was a warning with PostgresSQL error messages (attached below), not a fatal error. But I don’t think there should be any warning (or PostgreSQL errors) because the user didn’t do anything wrong here (v.edit map tool=create; v.db.addtable).

DBMI-PostgreSQL driver error:
Unable to create index: create unique index tmp2_cat on tmp2 ( cat )
ERROR: relation “tmp2_cat” already exists

DBMI-PostgreSQL driver error:
Unable to create index: create unique index tmp2_cat on tmp2 ( cat )
ERROR: relation “tmp2_cat” already exists

WARNING: Cannot create index
here
WARNING: Values in column will be overwritten
Reading features…
100%
Updating database…
100%

Huidae

On Tue, Jan 29, 2019 at 9:37 AM Markus Metz <markus.metz.giswork@gmail.com> wrote:

On Sun, Jan 27, 2019 at 11:21 AM Markus Metz <markus.metz.giswork@gmail.com> wrote:

On Sat, Jan 26, 2019 at 2:50 PM Huidae Cho <grass4u@gmail.com> wrote:

Markus,

If there is a linked table, v.db.addtable stops in line 106. If not, this script doesn’t create a unique index and calls v.db.connect. v.db.connect adds a db link in line 317 and creates a unique index (db_create_index2) in line 334 if linking was successful. SQLite didn’t complain when both modules created the same unique index, but PostgreSQL failed in v.db.connect (2nd time creating the same unique index).

What do you mean with “failed”? v.db.connect will issue a warning if it can not create an index but finish successfully.

Markus M

Not sure which code was added first/later. I think it’s more of how we design both modules. v.db.connect will fail if we try to link a table with a unique index. Isn’t v.db.connect supposed to “just” connect a table to a layer (without creating any database objects like index)? Which module should be responsible for creating unique indices?

Before this commit:

  1. v.db.addtable creates unique index
  2. v.db.connect tries to create unique index again ==> fatal error

After this commit:

  1. v.db.addtable doesn’t create unique index
  2. v.db.connect creates unique index

Probably, it should be:

  1. v.db.addtable should create unique index
  2. v.db.connect shouldn’t try to create unique index? Just “connect”…

Yes, this is the preferred solution. Consider v.db.connect -d followed by v.db.connect, i.e. disconnecting a table and then reconnecting the same table: in this case v.db.connect should also fail with PostgreSQL. IMHO, a unique index should only be created when the table is created.

Markus M

Regards,
Huidae

On Sat, Jan 26, 2019 at 7:39 AM Markus Neteler <neteler@osgeo.org> wrote:

Hi,

On Tue, Jan 22, 2019 at 3:51 AM <svn_grass@osgeo.org> wrote:

Author: hcho
Date: 2019-01-21 18:51:33 -0800 (Mon, 21 Jan 2019)
New Revision: 73995

Modified:
grass/trunk/scripts/v.db.addtable/v.db.addtable.py
Log:
v.db.addtable: Do not create unique index from this script; v.db.connect will try to create it again causing some drivers to fail (PostgreSQL)

Modified: grass/trunk/scripts/v.db.addtable/v.db.addtable.py

— grass/trunk/scripts/v.db.addtable/v.db.addtable.py 2019-01-21 22:37:59 UTC (rev 73994)
+++ grass/trunk/scripts/v.db.addtable/v.db.addtable.py 2019-01-22 02:51:33 UTC (rev 73995)
@@ -139,16 +139,6 @@
except CalledModuleError:
grass.fatal(_(“Unable to create table <%s>”) % table)

  • create index, see db/driver/*/index.c

  • if driver != “dbf”:
  • sql = “CREATE UNIQUE INDEX %s_%s ON %s (%s)” % (table, key, table, key)
  • try:
  • grass.run_command(‘db.execute’,
  • database=database, driver=driver, sql=sql)
  • except:
  • grass.warning(_(“Unable to create index on table <%s>”) % table)
  • pass

connect the map to the DB:

if schema:
table = ‘{schema}.{table}’.format(schema=schema, table=table)

just a conceptual question:
… are we sure that this index creation removal never leads to a
table without index?

Markus


Huidae Cho, Ph.D., GISP, PE (MD), CFM, M.ASCE
Open Source GIS Developer, GRASS GIS Development Team


grass-dev mailing list
grass-dev@lists.osgeo.org
https://lists.osgeo.org/mailman/listinfo/grass-dev

Huidae Cho, Ph.D., GISP, PE (MD), CFM, M.ASCE
Open Source GIS Developer, GRASS GIS Development Team

On Tue, Jan 29, 2019 at 4:12 PM Huidae Cho <grass4u@gmail.com> wrote:

You’re right. It was a warning with PostgresSQL error messages (attached below), not a fatal error. But I don’t think there should be any warning (or PostgreSQL errors) because the user didn’t do anything wrong here (v.edit map tool=create; v.db.addtable).

Hmm yes I agree. At least for v.db.addtable the index can be created by v.db.connect when the new table is actually linked to the vector map.

You might still get these PG errors when you try to connect an already existing table, but this is probably not happening very often, and v.db.connect finishes anyway.

Markus M

DBMI-PostgreSQL driver error:
Unable to create index: create unique index tmp2_cat on tmp2 ( cat )
ERROR: relation “tmp2_cat” already exists

DBMI-PostgreSQL driver error:
Unable to create index: create unique index tmp2_cat on tmp2 ( cat )
ERROR: relation “tmp2_cat” already exists

WARNING: Cannot create index
here
WARNING: Values in column will be overwritten
Reading features…
100%
Updating database…
100%

Huidae

On Tue, Jan 29, 2019 at 9:37 AM Markus Metz <markus.metz.giswork@gmail.com> wrote:

On Sun, Jan 27, 2019 at 11:21 AM Markus Metz <markus.metz.giswork@gmail.com> wrote:

On Sat, Jan 26, 2019 at 2:50 PM Huidae Cho <grass4u@gmail.com> wrote:

Markus,

If there is a linked table, v.db.addtable stops in line 106. If not, this script doesn’t create a unique index and calls v.db.connect. v.db.connect adds a db link in line 317 and creates a unique index (db_create_index2) in line 334 if linking was successful. SQLite didn’t complain when both modules created the same unique index, but PostgreSQL failed in v.db.connect (2nd time creating the same unique index).

What do you mean with “failed”? v.db.connect will issue a warning if it can not create an index but finish successfully.

Markus M

Not sure which code was added first/later. I think it’s more of how we design both modules. v.db.connect will fail if we try to link a table with a unique index. Isn’t v.db.connect supposed to “just” connect a table to a layer (without creating any database objects like index)? Which module should be responsible for creating unique indices?

Before this commit:

  1. v.db.addtable creates unique index
  2. v.db.connect tries to create unique index again ==> fatal error

After this commit:

  1. v.db.addtable doesn’t create unique index
  2. v.db.connect creates unique index

Probably, it should be:

  1. v.db.addtable should create unique index
  2. v.db.connect shouldn’t try to create unique index? Just “connect”…

Yes, this is the preferred solution. Consider v.db.connect -d followed by v.db.connect, i.e. disconnecting a table and then reconnecting the same table: in this case v.db.connect should also fail with PostgreSQL. IMHO, a unique index should only be created when the table is created.

Markus M

Regards,
Huidae

On Sat, Jan 26, 2019 at 7:39 AM Markus Neteler <neteler@osgeo.org> wrote:

Hi,

On Tue, Jan 22, 2019 at 3:51 AM <svn_grass@osgeo.org> wrote:

Author: hcho
Date: 2019-01-21 18:51:33 -0800 (Mon, 21 Jan 2019)
New Revision: 73995

Modified:
grass/trunk/scripts/v.db.addtable/v.db.addtable.py
Log:
v.db.addtable: Do not create unique index from this script; v.db.connect will try to create it again causing some drivers to fail (PostgreSQL)

Modified: grass/trunk/scripts/v.db.addtable/v.db.addtable.py

— grass/trunk/scripts/v.db.addtable/v.db.addtable.py 2019-01-21 22:37:59 UTC (rev 73994)
+++ grass/trunk/scripts/v.db.addtable/v.db.addtable.py 2019-01-22 02:51:33 UTC (rev 73995)
@@ -139,16 +139,6 @@
except CalledModuleError:
grass.fatal(_(“Unable to create table <%s>”) % table)

  • create index, see db/driver/*/index.c

  • if driver != “dbf”:
  • sql = “CREATE UNIQUE INDEX %s_%s ON %s (%s)” % (table, key, table, key)
  • try:
  • grass.run_command(‘db.execute’,
  • database=database, driver=driver, sql=sql)
  • except:
  • grass.warning(_(“Unable to create index on table <%s>”) % table)
  • pass

connect the map to the DB:

if schema:
table = ‘{schema}.{table}’.format(schema=schema, table=table)

just a conceptual question:
… are we sure that this index creation removal never leads to a
table without index?

Markus


Huidae Cho, Ph.D., GISP, PE (MD), CFM, M.ASCE
Open Source GIS Developer, GRASS GIS Development Team


grass-dev mailing list
grass-dev@lists.osgeo.org
https://lists.osgeo.org/mailman/listinfo/grass-dev


Huidae Cho, Ph.D., GISP, PE (MD), CFM, M.ASCE
Open Source GIS Developer, GRASS GIS Development Team

Yes. Like you said, v.db.connect followed by v.db.connect -d gives me the same pg errors and grass warning. I agree with you that the preferred solution would be to create the index in v.db.addtable, not in v.db.connect “if” no modules rely on v.db.connect for creating the unique index.

Huidae

···

Huidae Cho, Ph.D., GISP, PE (MD), CFM, M.ASCE
Open Source GIS Developer, GRASS GIS Development Team

I just did a quick search and found that these modules (i.segment.stats, r.mwprecip, v.in.geopaparazzi, v.link.precip, v.out.gps, v.ply.rectify, v.unpack) create/copy new tables themselves without the index and invoke v.db.connect. For now, we cannot move index creation to v.db.addtable.

Huidae

···

Huidae Cho, Ph.D., GISP, PE (MD), CFM, M.ASCE
Open Source GIS Developer, GRASS GIS Development Team

Just for our records, actually, PostgreSQL 9.5+ supports CREATE INDEX IF NOT EXISTS, which the grass sqlite driver already does (this is why we didn’t have this warning with sqlite before). Not sure if we want to enforce the minimum version for a specific database though.

Best,

Huidae

···

Huidae Cho, Ph.D., GISP, PE (MD), CFM, M.ASCE
Open Source GIS Developer, GRASS GIS Development Team

On Tue, Jan 29, 2019 at 11:17 PM Huidae Cho <grass4u@gmail.com> wrote:

I just did a quick search and found that these modules (i.segment.stats, r.mwprecip, v.in.geopaparazzi, v.link.precip, v.out.gps, v.ply.rectify, v.unpack) create/copy new tables themselves without the index and invoke v.db.connect. For now, we cannot move index creation to v.db.addtable.

Yes, removing index creation from v.db.connect would require that all modules relying on v,db.connect must create an index themselves, this is not very elegant. So let’s leave it as it is now.

Markus M

Huidae

On Tue, Jan 29, 2019 at 4:12 PM Huidae Cho <grass4u@gmail.com> wrote:

Yes. Like you said, v.db.connect followed by v.db.connect -d gives me the same pg errors and grass warning. I agree with you that the preferred solution would be to create the index in v.db.addtable, not in v.db.connect “if” no modules rely on v.db.connect for creating the unique index.

Huidae

On Tue, Jan 29, 2019 at 3:51 PM Markus Metz <markus.metz.giswork@gmail.com> wrote:

On Tue, Jan 29, 2019 at 4:12 PM Huidae Cho <grass4u@gmail.com> wrote:

You’re right. It was a warning with PostgresSQL error messages (attached below), not a fatal error. But I don’t think there should be any warning (or PostgreSQL errors) because the user didn’t do anything wrong here (v.edit map tool=create; v.db.addtable).

Hmm yes I agree. At least for v.db.addtable the index can be created by v.db.connect when the new table is actually linked to the vector map.

You might still get these PG errors when you try to connect an already existing table, but this is probably not happening very often, and v.db.connect finishes anyway.

Markus M

DBMI-PostgreSQL driver error:
Unable to create index: create unique index tmp2_cat on tmp2 ( cat )
ERROR: relation “tmp2_cat” already exists

DBMI-PostgreSQL driver error:
Unable to create index: create unique index tmp2_cat on tmp2 ( cat )
ERROR: relation “tmp2_cat” already exists

WARNING: Cannot create index
here
WARNING: Values in column will be overwritten
Reading features…
100%
Updating database…
100%

Huidae

On Tue, Jan 29, 2019 at 9:37 AM Markus Metz <markus.metz.giswork@gmail.com> wrote:

On Sun, Jan 27, 2019 at 11:21 AM Markus Metz <markus.metz.giswork@gmail.com> wrote:

On Sat, Jan 26, 2019 at 2:50 PM Huidae Cho <grass4u@gmail.com> wrote:

Markus,

If there is a linked table, v.db.addtable stops in line 106. If not, this script doesn’t create a unique index and calls v.db.connect. v.db.connect adds a db link in line 317 and creates a unique index (db_create_index2) in line 334 if linking was successful. SQLite didn’t complain when both modules created the same unique index, but PostgreSQL failed in v.db.connect (2nd time creating the same unique index).

What do you mean with “failed”? v.db.connect will issue a warning if it can not create an index but finish successfully.

Markus M

Not sure which code was added first/later. I think it’s more of how we design both modules. v.db.connect will fail if we try to link a table with a unique index. Isn’t v.db.connect supposed to “just” connect a table to a layer (without creating any database objects like index)? Which module should be responsible for creating unique indices?

Before this commit:

  1. v.db.addtable creates unique index
  2. v.db.connect tries to create unique index again ==> fatal error

After this commit:

  1. v.db.addtable doesn’t create unique index
  2. v.db.connect creates unique index

Probably, it should be:

  1. v.db.addtable should create unique index
  2. v.db.connect shouldn’t try to create unique index? Just “connect”…

Yes, this is the preferred solution. Consider v.db.connect -d followed by v.db.connect, i.e. disconnecting a table and then reconnecting the same table: in this case v.db.connect should also fail with PostgreSQL. IMHO, a unique index should only be created when the table is created.

Markus M

Regards,
Huidae

On Sat, Jan 26, 2019 at 7:39 AM Markus Neteler <neteler@osgeo.org> wrote:

Hi,

On Tue, Jan 22, 2019 at 3:51 AM <svn_grass@osgeo.org> wrote:

Author: hcho
Date: 2019-01-21 18:51:33 -0800 (Mon, 21 Jan 2019)
New Revision: 73995

Modified:
grass/trunk/scripts/v.db.addtable/v.db.addtable.py
Log:
v.db.addtable: Do not create unique index from this script; v.db.connect will try to create it again causing some drivers to fail (PostgreSQL)

Modified: grass/trunk/scripts/v.db.addtable/v.db.addtable.py

— grass/trunk/scripts/v.db.addtable/v.db.addtable.py 2019-01-21 22:37:59 UTC (rev 73994)
+++ grass/trunk/scripts/v.db.addtable/v.db.addtable.py 2019-01-22 02:51:33 UTC (rev 73995)
@@ -139,16 +139,6 @@
except CalledModuleError:
grass.fatal(_(“Unable to create table <%s>”) % table)

  • create index, see db/driver/*/index.c

  • if driver != “dbf”:
  • sql = “CREATE UNIQUE INDEX %s_%s ON %s (%s)” % (table, key, table, key)
  • try:
  • grass.run_command(‘db.execute’,
  • database=database, driver=driver, sql=sql)
  • except:
  • grass.warning(_(“Unable to create index on table <%s>”) % table)
  • pass

connect the map to the DB:

if schema:
table = ‘{schema}.{table}’.format(schema=schema, table=table)

just a conceptual question:
… are we sure that this index creation removal never leads to a
table without index?

Markus


Huidae Cho, Ph.D., GISP, PE (MD), CFM, M.ASCE
Open Source GIS Developer, GRASS GIS Development Team


grass-dev mailing list
grass-dev@lists.osgeo.org
https://lists.osgeo.org/mailman/listinfo/grass-dev


Huidae Cho, Ph.D., GISP, PE (MD), CFM, M.ASCE
Open Source GIS Developer, GRASS GIS Development Team


Huidae Cho, Ph.D., GISP, PE (MD), CFM, M.ASCE
Open Source GIS Developer, GRASS GIS Development Team


Huidae Cho, Ph.D., GISP, PE (MD), CFM, M.ASCE
Open Source GIS Developer, GRASS GIS Development Team

On Tue, Jan 29, 2019 at 11:37 PM Huidae Cho <grass4u@gmail.com> wrote:

Just for our records, actually, PostgreSQL 9.5+ supports CREATE INDEX IF NOT EXISTS, which the grass sqlite driver already does (this is why we didn’t have this warning with sqlite before). Not sure if we want to enforce the minimum version for a specific database though.

We can use “if not exists” for PostgreSQL 9.5+, otherwise not, please try trunk r74042.

Markus M

Best,
Huidae

On Tue, Jan 29, 2019 at 5:16 PM Huidae Cho <grass4u@gmail.com> wrote:

I just did a quick search and found that these modules (i.segment.stats, r.mwprecip, v.in.geopaparazzi, v.link.precip, v.out.gps, v.ply.rectify, v.unpack) create/copy new tables themselves without the index and invoke v.db.connect. For now, we cannot move index creation to v.db.addtable.

Huidae

On Tue, Jan 29, 2019 at 4:12 PM Huidae Cho <grass4u@gmail.com> wrote:

Yes. Like you said, v.db.connect followed by v.db.connect -d gives me the same pg errors and grass warning. I agree with you that the preferred solution would be to create the index in v.db.addtable, not in v.db.connect “if” no modules rely on v.db.connect for creating the unique index.

Huidae

On Tue, Jan 29, 2019 at 3:51 PM Markus Metz <markus.metz.giswork@gmail.com> wrote:

On Tue, Jan 29, 2019 at 4:12 PM Huidae Cho <grass4u@gmail.com> wrote:

You’re right. It was a warning with PostgresSQL error messages (attached below), not a fatal error. But I don’t think there should be any warning (or PostgreSQL errors) because the user didn’t do anything wrong here (v.edit map tool=create; v.db.addtable).

Hmm yes I agree. At least for v.db.addtable the index can be created by v.db.connect when the new table is actually linked to the vector map.

You might still get these PG errors when you try to connect an already existing table, but this is probably not happening very often, and v.db.connect finishes anyway.

Markus M

DBMI-PostgreSQL driver error:
Unable to create index: create unique index tmp2_cat on tmp2 ( cat )
ERROR: relation “tmp2_cat” already exists

DBMI-PostgreSQL driver error:
Unable to create index: create unique index tmp2_cat on tmp2 ( cat )
ERROR: relation “tmp2_cat” already exists

WARNING: Cannot create index
here
WARNING: Values in column will be overwritten
Reading features…
100%
Updating database…
100%

Huidae

On Tue, Jan 29, 2019 at 9:37 AM Markus Metz <markus.metz.giswork@gmail.com> wrote:

On Sun, Jan 27, 2019 at 11:21 AM Markus Metz <markus.metz.giswork@gmail.com> wrote:

On Sat, Jan 26, 2019 at 2:50 PM Huidae Cho <grass4u@gmail.com> wrote:

Markus,

If there is a linked table, v.db.addtable stops in line 106. If not, this script doesn’t create a unique index and calls v.db.connect. v.db.connect adds a db link in line 317 and creates a unique index (db_create_index2) in line 334 if linking was successful. SQLite didn’t complain when both modules created the same unique index, but PostgreSQL failed in v.db.connect (2nd time creating the same unique index).

What do you mean with “failed”? v.db.connect will issue a warning if it can not create an index but finish successfully.

Markus M

Not sure which code was added first/later. I think it’s more of how we design both modules. v.db.connect will fail if we try to link a table with a unique index. Isn’t v.db.connect supposed to “just” connect a table to a layer (without creating any database objects like index)? Which module should be responsible for creating unique indices?

Before this commit:

  1. v.db.addtable creates unique index
  2. v.db.connect tries to create unique index again ==> fatal error

After this commit:

  1. v.db.addtable doesn’t create unique index
  2. v.db.connect creates unique index

Probably, it should be:

  1. v.db.addtable should create unique index
  2. v.db.connect shouldn’t try to create unique index? Just “connect”…

Yes, this is the preferred solution. Consider v.db.connect -d followed by v.db.connect, i.e. disconnecting a table and then reconnecting the same table: in this case v.db.connect should also fail with PostgreSQL. IMHO, a unique index should only be created when the table is created.

Markus M

Regards,
Huidae

On Sat, Jan 26, 2019 at 7:39 AM Markus Neteler <neteler@osgeo.org> wrote:

Hi,

On Tue, Jan 22, 2019 at 3:51 AM <svn_grass@osgeo.org> wrote:

Author: hcho
Date: 2019-01-21 18:51:33 -0800 (Mon, 21 Jan 2019)
New Revision: 73995

Modified:
grass/trunk/scripts/v.db.addtable/v.db.addtable.py
Log:
v.db.addtable: Do not create unique index from this script; v.db.connect will try to create it again causing some drivers to fail (PostgreSQL)

Modified: grass/trunk/scripts/v.db.addtable/v.db.addtable.py

— grass/trunk/scripts/v.db.addtable/v.db.addtable.py 2019-01-21 22:37:59 UTC (rev 73994)
+++ grass/trunk/scripts/v.db.addtable/v.db.addtable.py 2019-01-22 02:51:33 UTC (rev 73995)
@@ -139,16 +139,6 @@
except CalledModuleError:
grass.fatal(_(“Unable to create table <%s>”) % table)

  • create index, see db/driver/*/index.c

  • if driver != “dbf”:
  • sql = “CREATE UNIQUE INDEX %s_%s ON %s (%s)” % (table, key, table, key)
  • try:
  • grass.run_command(‘db.execute’,
  • database=database, driver=driver, sql=sql)
  • except:
  • grass.warning(_(“Unable to create index on table <%s>”) % table)
  • pass

connect the map to the DB:

if schema:
table = ‘{schema}.{table}’.format(schema=schema, table=table)

just a conceptual question:
… are we sure that this index creation removal never leads to a
table without index?

Markus


Huidae Cho, Ph.D., GISP, PE (MD), CFM, M.ASCE
Open Source GIS Developer, GRASS GIS Development Team


grass-dev mailing list
grass-dev@lists.osgeo.org
https://lists.osgeo.org/mailman/listinfo/grass-dev


Huidae Cho, Ph.D., GISP, PE (MD), CFM, M.ASCE
Open Source GIS Developer, GRASS GIS Development Team


Huidae Cho, Ph.D., GISP, PE (MD), CFM, M.ASCE
Open Source GIS Developer, GRASS GIS Development Team


Huidae Cho, Ph.D., GISP, PE (MD), CFM, M.ASCE
Open Source GIS Developer, GRASS GIS Development Team


Huidae Cho, Ph.D., GISP, PE (MD), CFM, M.ASCE
Open Source GIS Developer, GRASS GIS Development Team

On Wed, Jan 30, 2019 at 9:42 AM Markus Metz <markus.metz.giswork@gmail.com> wrote:

On Tue, Jan 29, 2019 at 11:37 PM Huidae Cho <grass4u@gmail.com> wrote:

Just for our records, actually, PostgreSQL 9.5+ supports CREATE INDEX IF NOT EXISTS, which the grass sqlite driver already does (this is why we didn’t have this warning with sqlite before). Not sure if we want to enforce the minimum version for a specific database though.

We can use “if not exists” for PostgreSQL 9.5+, otherwise not, please try trunk r74042.

trunk r74043

Markus M

Best,
Huidae

On Tue, Jan 29, 2019 at 5:16 PM Huidae Cho <grass4u@gmail.com> wrote:

I just did a quick search and found that these modules (i.segment.stats, r.mwprecip, v.in.geopaparazzi, v.link.precip, v.out.gps, v.ply.rectify, v.unpack) create/copy new tables themselves without the index and invoke v.db.connect. For now, we cannot move index creation to v.db.addtable.

Huidae

On Tue, Jan 29, 2019 at 4:12 PM Huidae Cho <grass4u@gmail.com> wrote:

Yes. Like you said, v.db.connect followed by v.db.connect -d gives me the same pg errors and grass warning. I agree with you that the preferred solution would be to create the index in v.db.addtable, not in v.db.connect “if” no modules rely on v.db.connect for creating the unique index.

Huidae

On Tue, Jan 29, 2019 at 3:51 PM Markus Metz <markus.metz.giswork@gmail.com> wrote:

On Tue, Jan 29, 2019 at 4:12 PM Huidae Cho <grass4u@gmail.com> wrote:

You’re right. It was a warning with PostgresSQL error messages (attached below), not a fatal error. But I don’t think there should be any warning (or PostgreSQL errors) because the user didn’t do anything wrong here (v.edit map tool=create; v.db.addtable).

Hmm yes I agree. At least for v.db.addtable the index can be created by v.db.connect when the new table is actually linked to the vector map.

You might still get these PG errors when you try to connect an already existing table, but this is probably not happening very often, and v.db.connect finishes anyway.

Markus M

DBMI-PostgreSQL driver error:
Unable to create index: create unique index tmp2_cat on tmp2 ( cat )
ERROR: relation “tmp2_cat” already exists

DBMI-PostgreSQL driver error:
Unable to create index: create unique index tmp2_cat on tmp2 ( cat )
ERROR: relation “tmp2_cat” already exists

WARNING: Cannot create index
here
WARNING: Values in column will be overwritten
Reading features…
100%
Updating database…
100%

Huidae

On Tue, Jan 29, 2019 at 9:37 AM Markus Metz <markus.metz.giswork@gmail.com> wrote:

On Sun, Jan 27, 2019 at 11:21 AM Markus Metz <markus.metz.giswork@gmail.com> wrote:

On Sat, Jan 26, 2019 at 2:50 PM Huidae Cho <grass4u@gmail.com> wrote:

Markus,

If there is a linked table, v.db.addtable stops in line 106. If not, this script doesn’t create a unique index and calls v.db.connect. v.db.connect adds a db link in line 317 and creates a unique index (db_create_index2) in line 334 if linking was successful. SQLite didn’t complain when both modules created the same unique index, but PostgreSQL failed in v.db.connect (2nd time creating the same unique index).

What do you mean with “failed”? v.db.connect will issue a warning if it can not create an index but finish successfully.

Markus M

Not sure which code was added first/later. I think it’s more of how we design both modules. v.db.connect will fail if we try to link a table with a unique index. Isn’t v.db.connect supposed to “just” connect a table to a layer (without creating any database objects like index)? Which module should be responsible for creating unique indices?

Before this commit:

  1. v.db.addtable creates unique index
  2. v.db.connect tries to create unique index again ==> fatal error

After this commit:

  1. v.db.addtable doesn’t create unique index
  2. v.db.connect creates unique index

Probably, it should be:

  1. v.db.addtable should create unique index
  2. v.db.connect shouldn’t try to create unique index? Just “connect”…

Yes, this is the preferred solution. Consider v.db.connect -d followed by v.db.connect, i.e. disconnecting a table and then reconnecting the same table: in this case v.db.connect should also fail with PostgreSQL. IMHO, a unique index should only be created when the table is created.

Markus M

Regards,
Huidae

On Sat, Jan 26, 2019 at 7:39 AM Markus Neteler <neteler@osgeo.org> wrote:

Hi,

On Tue, Jan 22, 2019 at 3:51 AM <svn_grass@osgeo.org> wrote:

Author: hcho
Date: 2019-01-21 18:51:33 -0800 (Mon, 21 Jan 2019)
New Revision: 73995

Modified:
grass/trunk/scripts/v.db.addtable/v.db.addtable.py
Log:
v.db.addtable: Do not create unique index from this script; v.db.connect will try to create it again causing some drivers to fail (PostgreSQL)

Modified: grass/trunk/scripts/v.db.addtable/v.db.addtable.py

— grass/trunk/scripts/v.db.addtable/v.db.addtable.py 2019-01-21 22:37:59 UTC (rev 73994)
+++ grass/trunk/scripts/v.db.addtable/v.db.addtable.py 2019-01-22 02:51:33 UTC (rev 73995)
@@ -139,16 +139,6 @@
except CalledModuleError:
grass.fatal(_(“Unable to create table <%s>”) % table)

  • create index, see db/driver/*/index.c

  • if driver != “dbf”:
  • sql = “CREATE UNIQUE INDEX %s_%s ON %s (%s)” % (table, key, table, key)
  • try:
  • grass.run_command(‘db.execute’,
  • database=database, driver=driver, sql=sql)
  • except:
  • grass.warning(_(“Unable to create index on table <%s>”) % table)
  • pass

connect the map to the DB:

if schema:
table = ‘{schema}.{table}’.format(schema=schema, table=table)

just a conceptual question:
… are we sure that this index creation removal never leads to a
table without index?

Markus


Huidae Cho, Ph.D., GISP, PE (MD), CFM, M.ASCE
Open Source GIS Developer, GRASS GIS Development Team


grass-dev mailing list
grass-dev@lists.osgeo.org
https://lists.osgeo.org/mailman/listinfo/grass-dev


Huidae Cho, Ph.D., GISP, PE (MD), CFM, M.ASCE
Open Source GIS Developer, GRASS GIS Development Team


Huidae Cho, Ph.D., GISP, PE (MD), CFM, M.ASCE
Open Source GIS Developer, GRASS GIS Development Team


Huidae Cho, Ph.D., GISP, PE (MD), CFM, M.ASCE
Open Source GIS Developer, GRASS GIS Development Team


Huidae Cho, Ph.D., GISP, PE (MD), CFM, M.ASCE
Open Source GIS Developer, GRASS GIS Development Team

r74043 works.

Thanks,
Huidae

···

Huidae Cho, Ph.D., GISP, PE (MD), CFM, M.ASCE
Open Source GIS Developer, GRASS GIS Development Team