Opened 9 years ago
Last modified 5 months ago
#25947 assigned Bug
Query's str() method fails when 'default' database is empty
Reported by: | liboz | Owned by: | jayvynl |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | Egor R | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
According to the docs, we can have
default database...[with]...parameters dictionary...blank if it will not be used.
However, when trying to print a query with something like:
print Question.objects.all().query
you get the error that
settings.DATABASES is improperly configured. Please supply the ENGINE value.
even though the query itself can return results.
You can replicate this by creating a new project, creating a router that routes everything to a test database like so:
class Router(object): def db_for_read(self, model, **hints): """ Reads go to a randomly-chosen replica. """ return 'test' def db_for_write(self, model, **hints): """ Writes always go to primary. """ return 'test' def allow_relation(self, obj1, obj2, **hints): """ Relations between objects are allowed if both objects are in the primary/replica pool. """ return True def allow_migrate(self, db, app_label, model=None, **hints): """ All non-auth models end up in this pool. """ return True # Database # https://docs.djangoproject.com/en/1.8/ref/settings/#databases DATABASE_ROUTERS = ['test123.settings.Router'] DATABASES = { 'default': {}, 'test': { 'ENGINE': 'django.db.backends.sqlite3', 'NAME': os.path.join(BASE_DIR, 'db.sqlite3'), } }
Create a simple model like this one:
from django.db import models # Create your models here. class Question(models.Model): question_text = models.CharField(max_length=200) pub_date = models.DateTimeField('date published')
and run the appropriate migrations on the test database.
Then attempting to print the query will fail, but the query itself will work. I believe the error is because the sql_with_params method in django.db.models.sql.query forces the uses of the DEFAULT_DB_ALIAS:
def sql_with_params(self): """ Returns the query as an SQL string and the parameters that will be substituted into the query. """ return self.get_compiler(DEFAULT_DB_ALIAS).as_sql()
Change History (13)
comment:1 by , 9 years ago
Component: | Uncategorized → Database layer (models, ORM) |
---|---|
Description: | modified (diff) |
Summary: | Query's str() method fails when default Database has no parameters → Query's str() method fails when 'default' database is empty |
Triage Stage: | Unreviewed → Accepted |
Type: | Uncategorized → Bug |
comment:2 by , 9 years ago
I looked into this a bit and Django seems to crash when running quote_name_unless_alias
in django.db.models.sql.compiler
because it uses DatabaseOperations' quote_name
function. Could changing the DatabaseOperations for django.db.backends.dummy
to have a quote_name
function like that of sqlite?
I'm thinking of something like:
def quote_name(self, name): if name.startswith('"') and name.endswith('"'): return name # Quoting once is enough. return '"%s"' % name
comment:3 by , 3 years ago
Cc: | added |
---|
comment:4 by , 3 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:5 by , 3 years ago
Has patch: | set |
---|
Patch is provided in https://github.com/django/django/pull/15142.
comment:6 by , 3 years ago
Patch needs improvement: | set |
---|
comment:7 by , 8 months ago
Owner: | changed from | to
---|
I think it's reasonable to use QuerySet.db()
to replace DEFAULT_DB_ALIAS
in sql_with_db
method. To achieve this, QuerySet
should pass itself to Query
, this only need just a few lines change.
comment:8 by , 8 months ago
Version: | 1.9 → dev |
---|
comment:9 by , 8 months ago
Passing QuseySet
to Query
will make QuerySet
object be evaluated when pickling Query
object and make pickle results big.
Related ticket #27159
It should have some reason why Query
will be pickled. I will come up with another approch to solve this issue.
comment:10 by , 8 months ago
Patch needs improvement: | unset |
---|
comment:12 by , 5 months ago
Following a chat with Sarah about this ticket and PR, I can confirm this issue is still valid. For the sake of explicitness, these are the steps to reproduce the original report:
- ensure that you have latest django installed and venv enabled if that applies
- create an empty test project
django-admin startproject projectmultidb
- change into the project directory
cd projectmultidb
- create a router definition, for example in
dbrouter.testrouter.TestRouter
, use the router as provided above (no(object)
inheritance though)class TestRouter: db_alias = "test" def db_for_read(self, model, **hints): return self.db_alias def db_for_write(self, model, **hints): return self.db_alias def allow_relation(self, obj1, obj2, **hints): return True def allow_migrate(self, db, app_label, model_name=None, **hints): return True
- tweak the settings to the following:
DATABASE_ROUTERS = ["dbrouter.testrouter.TestRouter"] DATABASES = { "default": {}, 'test': { 'ENGINE': 'django.db.backends.sqlite3', 'NAME': BASE_DIR / 'test.sqlite3', }, }
- Double check no migrations are pending
python -Wall manage.py makemigrations
should returnNo changes detected
- Apply migrations from apps
python -Wall manage.py migrate --database=test
with output:Operations to perform: Apply all migrations: admin, auth, contenttypes, sessions Running migrations: Applying contenttypes.0001_initial... OK ... Applying auth.0012_alter_user_first_name_max_length... OK Applying sessions.0001_initial... OK
- Start a new app
testapp
, add aQuestion
model as described above, generate migration, and apply it:(django-3.12) [projectmultidb]$ python -Wall manage.py startapp testapp (django-3.12) [projectmultidb]$ python -Wall manage.py makemigrations Migrations for 'testapp': testapp/migrations/0001_initial.py + Create model Question (django-3.12) [projectmultidb]$ python -Wall manage.py migrate --database=test Operations to perform: Apply all migrations: admin, auth, contenttypes, sessions, testapp Running migrations: Applying testapp.0001_initial... OK
- Open the shell
python -Wall manage.py shell
and run:In [1]: from django.db import connections In [2]: from testapp.models import Question In [3]: Question.objects.all() Out[3]: <QuerySet []> In [4]: connections["test"].queries Out[4]: [{'sql': 'SELECT "testapp_question"."id", "testapp_question"."question_text", "testapp_question"."pub_date" FROM "testapp_question" LIMIT 21', 'time': '0.001'}] In [5]: print(Question.objects.all().query) --------------------------------------------------------------------------- ImproperlyConfigured Traceback (most recent call last) ... ImproperlyConfigured: settings.DATABASES is improperly configured. Please supply the ENGINE value. Check settings documentation for more details.
comment:13 by , 5 months ago
Patch needs improvement: | set |
---|
I guess this might be tricky to fix for reasons similar to this comment in the file: "We need to use DEFAULT_DB_ALIAS here, as QuerySet does not have (nor should it have) knowledge of which connection is going to be used."