Code

Opened 8 years ago

Closed 7 years ago

#1828 closed defect (fixed)

[patch] sqlreset forgets indexes

Reported by: mdt@… Owned by: russellm
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: UI/UX:

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@… 8 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 8 years ago by mdt@…

  • Summary changed from sqlreset forgets foreign key constraint to outer world to sqlreset 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 Changed 8 years ago by treborhudson@…

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 Changed 8 years ago by russellm

  • Cc freakboy@… added

comment:4 Changed 8 years ago by Simon Greenhill

  • Cc dev@… added
  • Summary changed from sqlreset forgets indexes to [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? )

Changed 8 years ago by dev@…

comment:5 Changed 8 years ago by russellm

  • Owner changed from adrian to russellm

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 Changed 8 years ago by russellm

  • Status changed from new to assigned

comment:7 Changed 8 years ago by russellm

(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 Changed 7 years ago by Simon G. <dev@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.