Opened 11 years ago

Closed 10 years ago

Last modified 10 years ago

#20968 closed Cleanup/optimization (fixed)

Add a migrate hook for automatically running database setup SQL

Reported by: burton449geo@… Owned by: nobody
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords: geodjango, spatialite, indexes
Cc: keniallee@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The database is Spatialite 4.1.0

On Syncdb these errors messages are shown:

Superuser created successfully.
Installing custom SQL ...
Installing indexes ...
AddGeometryColumn() error: "no such table: geometry_columns"
CreateSpatialIndex() error: "no such table: geometry_columns"

Change History (21)

comment:1 by Mathieu Leplatre, 11 years ago

Initializing the database with spatialite binary is the answer :

spatialite database.db "SELECT InitSpatialMetaData();"

comment:2 by Claude Paroz, 11 years ago

This should be done automatically:
https://github.com/django/django/blob/master/django/contrib/gis/db/backends/spatialite/creation.py#L102-L107

Maybe something changed in Spatialite 4.1? I cannot test right now.

comment:3 by Aymeric Augustin, 11 years ago

Since no one managed to reproduce the error, should I close this ticket as needsinfo?

comment:4 by Ramiro Morales, 11 years ago

Resolution: needsinfo
Status: newclosed

Tried to reproduce this to not avail (tested with the GeoDjango tutorial project).

Environment:

Creating spatial metadata tables (per https://docs.djangoproject.com/en/dev/ref/contrib/gis/install/spatialite/#creating-a-spatial-database-for-spatialite) -- Not strictly needed but useful to get the versions of the C-level components printed to the console:

(spatialite41)ramiro@mang:~/venv/spatialite41/src/geodjango$ spatialite db.sqlite3 "SELECT InitSpatialMetaData();"
SpatiaLite version ..: 4.1.0	Supported Extensions:
	- 'VirtualShape'	[direct Shapefile access]
	- 'VirtualDbf'		[direct DBF access]
	- 'VirtualText'		[direct CSV/TXT access]
	- 'VirtualNetwork'	[Dijkstra shortest path]
	- 'RTree'		[Spatial Index - R*Tree]
	- 'MbrCache'		[Spatial Index - MBR cache]
	- 'VirtualSpatialIndex'	[R*Tree metahandler]
	- 'VirtualFDO'		[FDO-OGR interoperability]
	- 'SpatiaLite'		[Spatial SQL - OGC]
PROJ.4 version ......: Rel. 4.7.1, 23 September 2009
GEOS version ........: 3.2.2-CAPI-1.6.2
the SPATIAL_REF_SYS table already contains some row(s)
 InitSpatiaMetaData() error:"table spatial_ref_sys already exists"
0

DB schema manage.py commands:

(spatialite41)ramiro@mang:~/venv/spatialite41/src/geodjango$ PYTHONPATH=~/django/upstream python manage.py sqlall world
BEGIN;
CREATE TABLE "world_worldborder" (
    "id" integer NOT NULL PRIMARY KEY AUTOINCREMENT,
    "name" varchar(50) NOT NULL,
    "area" integer NOT NULL,
    "pop2005" integer NOT NULL,
    "fips" varchar(2) NOT NULL,
    "iso2" varchar(2) NOT NULL,
    "iso3" varchar(3) NOT NULL,
    "un" integer NOT NULL,
    "region" integer NOT NULL,
    "subregion" integer NOT NULL,
    "lon" real NOT NULL,
    "lat" real NOT NULL
)
;
SELECT AddGeometryColumn('world_worldborder', 'mpoly', 4326, 'MULTIPOLYGON', 2, 1);
SELECT CreateSpatialIndex('world_worldborder', 'mpoly');

COMMIT;
(spatialite41)ramiro@mang:~/venv/spatialite41/src/geodjango$ PYTHONPATH=~/django/upstream python manage.py migrate
Operations to perform:
  Synchronize unmigrated apps: gis, sessions, admin, messages, auth, staticfiles, contenttypes, world
  Apply all migrations: (none)
Synchronizing apps without migrations:
  Creating tables...
    Creating table django_admin_log
    Creating table auth_permission
    Creating table auth_group_permissions
    Creating table auth_group
    Creating table auth_user_groups
    Creating table auth_user_user_permissions
    Creating table auth_user
    Creating table django_content_type
    Creating table django_session
    Creating table world_worldborder
  Installing custom SQL...
  Installing indexes...
Installed 0 object(s) from 0 fixture(s)
Running migrations:
  No migrations needed.

You just installed Django's auth system, which means you don't have any superusers defined.
Would you like to create one now? (yes/no): no

Also ran the GeoDjango tests without problems:

(spatialite41)ramiro@mang:~/django/upstream/tests$ PYTHONPATH=.. python runtests.py --settings=test_gis django.contrib.gis
Testing against Django installed in '/home/ramiro/django/upstream/django'
Creating test database for alias 'default'...
Creating test database for alias 'other'...
.s...s....s..s............s.......sssssssssss..........ssssss...s............s....s.......sss...s.s.s.s.......ssssss..........................................................................................................................................................................................
----------------------------------------------------------------------
Ran 302 tests in 5.177s

OK (skipped=38)
Destroying test database for alias 'default'...
Destroying test database for alias 'other'...

If you have a system-wide spatialite installation of a different version (e.g. in the case of Ubuntu 12.04 it could be the 3.0.0~beta20110817-3 .deb package) then make sure you direct GeoDjango to use the 4.1.x one using the SPATIALITE_LIBRARY_PATH setting in setting.py:

SPATIALITE_LIBRARY_PATH='/path/to/your/spatialite/4.1.x/libspatialite.so'

Closing 'needsinfo' for now. Please reopen with a similar description of your environment if you can reproduce and wish to push this further.

comment:5 by Ramiro Morales, 11 years ago

Resolution: needsinfoinvalid

Actually I can reproduce the issue if I don't create the spatial tables with spatialite db.sqlite3 "SELECT InitSpatialMetaData();" then the index creation fails.

So comment:1 is right. I'm wrong in the last comment because I assumed these tables were created automatically if not present. That's only true then creating the test DB to run the GeoDjango tests.

So what one needs to do is actually follow the documentation (https://docs.djangoproject.com/en/dev/ref/contrib/gis/install/spatialite/#creating-a-spatial-database-for-spatialite) because at no point it describes that step as optional.

comment:6 by Javier Domingo Cansino, 10 years ago

It does specify that step as optional in 1.7 docs:

https://docs.djangoproject.com/en/1.7/ref/contrib/gis/install/spatialite/#creating-a-spatial-database-for-spatialite

Maybe an update would be recommended?

EDIT: Sorry, misread, maybe refactor a little the text to be more hard to misread? =)

Last edited 10 years ago by Javier Domingo Cansino (previous) (diff)

comment:7 by Kenial Sookyum Lee, 10 years ago

Using spatialite db.sqlite3 "SELECT InitSpatialMetaData();" is the answer, though, however, can we prevent Django users from meeting this error by adding workaround codes, which would be in syncdb command or database file creation code of django.contrib.gis.db.backends.spatialite?

p.s: txomon, I'm one of them recently updated it and feel sorry. As you know, using spatialite is something... feels like rolling under the hood :(

comment:8 by Claude Paroz, 10 years ago

Has patch: set
Resolution: invalid
Status: closednew
Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization
Version: 1.5master

in reply to:  8 comment:9 by Kenial Sookyum Lee, 10 years ago

Replying to claudep:

What about this patch?
https://github.com/django/django/pull/3697

if InitSpatialMetaData query is executed in init_connection_state() method, then it will check it every time when opening a connection. As you know, there is no connection pooling on sqlite3 (and SpatiaLite) backend by default. It means that every query execution will lead to execute PRAGMA table_info(geometry_columns);, I think it's a sort of redundancy. My patch updating django/contrib/gis/apps.py intended to execute InitSpatialMetaData query when doing migrate: https://github.com/django/django/pull/3695

What do you think about?

comment:10 by Kenial Sookyum Lee, 10 years ago

Cc: keniallee@… added

comment:11 by Claude Paroz, 10 years ago

I recognize my patch has the drawback of adding a query for each database connection creation.
What I don't like in your solution, it's the backend-specific "pollution" in the main gis code. If we miss a backend-specific hook during app initialization, then let's create that hook. But importing anything from django.contrib.gis.db.backends.<backend> inside apps.py is not an option.

comment:12 by Claude Paroz, 10 years ago

Another option would be to use the pre-migrate signal.

in reply to:  12 comment:13 by Kenial Sookyum Lee, 10 years ago

If we're going with the hook, it still does "migrate" stuff in context of another application (in this case, gis). So hook isn't looking good. (is there a way to completely split into migrate and gis context in app initialization?)

pre-migrate signal seems to be nice, but I find out ... it's model based. We're doing this on database-level. Maybe it's safe to be executed repeatedly, because when migrating only.

My original approach was to add a method for this purpose (name was before_migration) to BaseDatabaseWrapper with empty implementation, and call it when running migrate command. For sure, django.contrib.gis.db.backends.spatialite.base.DatabaseWrapper implements this before_migration method to init metadata table.

in reply to:  12 comment:14 by Kenial Sookyum Lee, 10 years ago

Here's my original approach: https://github.com/django/django/pull/3698

How about this?

comment:15 by Claude Paroz, 10 years ago

I've rewritten the patch with the migrate hook idea. I also added a patch which moves a PostGIS operation to add some weight to this solution.

comment:16 by Tim Graham, 10 years ago

Component: GISDatabase layer (models, ORM)
Summary: Error creating Indexes on SyncdbAdd a migrate hook for automatically running database setup SQL
Triage Stage: AcceptedReady for checkin

comment:17 by Claude Paroz <claude@…>, 10 years ago

Resolution: fixed
Status: newclosed

In 8f97413faed5431713c034897cda486507bf0cc3:

Fixed #20968 -- Checked Spatialite metadata before migrations

Thanks Kenial S. Lee for the initial patch and Tim Graham for
the review.

in reply to:  17 comment:18 by Kenial Sookyum Lee, 10 years ago

Great work! Thanks :)

comment:19 by Tim Graham, 10 years ago

Has patch: unset
Triage Stage: Ready for checkinAccepted

Claude, PostGIS on the django-master-trusty build is failing since this change. Could you take a look?

comment:21 by Claude Paroz <claude@…>, 10 years ago

In df30ae07fc7d7500bbbe51ed3361982f645169f2:

Fixed postgis test database initialization

Refs #20968. Allow querying template_postgis presence without
existing test database.
Thanks Tim Graham for the review.

Note: See TracTickets for help on using tickets.
Back to Top