Opened 18 years ago

Closed 17 years ago

#1828 closed defect (fixed)

[patch] sqlreset forgets indexes

Reported by: mdt@… Owned by: Russell Keith-Magee
Component: Database layer (models, ORM) Version:
Severity: normal Keywords:
Cc: freakboy@…, dev@… Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

when isseuing syncdb a different database schema is created than with sqlreset. one difference is that foreign key constraints are not created when the model comes from another application. another difference are the additional indexes (i.e. on slug fields) which are not created. this is my test:

	(cd $(PROJ); PYTHONPATH=`pwd`/.. ./manage.py sqlreset myapp) | sqlite3 $(PROJ).db
	sqlite3 $(PROJ).db .schema > $(PROJ).sql

against:

	(cd $(PROJ); PYTHONPATH=`pwd`/.. ./manage.py syncdb)
	sqlite3 $(PROJ).db .schema > $(PROJ).sql

done each with no existing database

Attachments (1)

1828.diff (1.4 KB ) - added by dev@… 17 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 by mdt@…, 18 years ago

Summary: sqlreset forgets foreign key constraint to outer worldsqlreset forgets indexes

okay found out that the foreign key problem is my fault. shure i have to add sqlresets for all apps when starting from scratch.

still the indexes are missing...

comment:2 by treborhudson@…, 18 years ago

Yes, syncdb does not create indexes. (In my case, I'm using MySQL). I wasn't sure at first if it was supposed to, but the docs in tutorial one say that it should explicitly:

The syncdb command runs the sql from 'sqlall' on your database for all apps in INSTALLED_APPS that don't already exist in your database. This creates all the tables, initial data and indexes for any apps you have added to your project since the last time you ran syncdb. syncdb can be called as often as you like, and it will only ever create the tables that don't exist.

If you run ./manage.py install [model] it works but syncdb should be fixed to run the indexes.

comment:3 by Russell Keith-Magee, 18 years ago

Cc: freakboy@… added

comment:4 by Simon Greenhill, 17 years ago

Cc: dev@… added
Summary: sqlreset forgets indexes[patch] sqlreset forgets indexes

This should fix it, I've added a call to get_sql_indexes for each app that got created. I've added this AFTER the initial SQL import, as this will speed up the initial sql import (MySQL for example imports data much faster if there's fewer indexes).

No idea how to write a regression test for it though, without breaking into custom SQL (show indexes...etc), which would probably raise portability issues. The model._meta_db_index comes directly from the model and not the database so that can't be used. ( Aside: should AutoFields be marked with db_index=True? )

by dev@…, 17 years ago

Attachment: 1828.diff added

comment:5 by Russell Keith-Magee, 17 years ago

Owner: changed from Adrian Holovaty to Russell Keith-Magee

While this patch will get the job done, there is a bigger issue -- the code duplication between install and syncdb. The better solution here isn't to keep patching syncdb until it matches install - it's to refactor the two methods so that there is a single implementation. For example, this patch doesn't address the issue that install doesn't issue a post-syncdb signal, which an increasing number of applications are relying upon for initialization.

At present, install (and the other get_sql_ methods) only work on a per-app basis; the reason for syncdb is the existence of forward dependencies during installation. A better fix for this ticket would be to modify install to take a list of apps, and make syncdb a call to install that determines the list of applications that haven't yet been installed.

I'm planning to work on this refactoring RSN as part of the #2333 test fixture work - but I've been saying that for a couple of weeks.

comment:6 by Russell Keith-Magee, 17 years ago

Status: newassigned

comment:7 by Russell Keith-Magee, 17 years ago

(In [3893]) Refs #1828 -- Added creation of indexes as a step in syncdb. This is an interim solution; the long term solution requires a non-trivial refactoring of syncdb, install and the get_* calls in management.py. Thanks to mdt@… for the original report, and to Simon Greenhill for prodding me to an interim fix.

comment:8 by Simon G. <dev@…>, 17 years ago

Resolution: fixed
Status: assignedclosed
Note: See TracTickets for help on using tickets.
Back to Top