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 Tim Graham)

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 Tim Graham, 9 years ago

Component: UncategorizedDatabase layer (models, ORM)
Description: modified (diff)
Summary: Query's str() method fails when default Database has no parametersQuery's str() method fails when 'default' database is empty
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

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."

comment:2 by liboz, 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 Egor R, 3 years ago

Cc: Egor R added

comment:4 by Teddy Ni, 3 years ago

Owner: changed from nobody to Teddy Ni
Status: newassigned

comment:5 by Teddy Ni, 3 years ago

Has patch: set

comment:6 by Mariusz Felisiak, 3 years ago

Patch needs improvement: set

comment:7 by jayvynl, 8 months ago

Owner: changed from Teddy Ni to jayvynl

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.

Version 0, edited 8 months ago by jayvynl (next)

comment:8 by jayvynl, 8 months ago

Version: 1.9dev

comment:9 by jayvynl, 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 jayvynl, 8 months ago

Patch needs improvement: unset

comment:12 by Natalia Bidart, 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:

  1. ensure that you have latest django installed and venv enabled if that applies
  2. create an empty test project django-admin startproject projectmultidb
  3. change into the project directory cd projectmultidb
  4. 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
    
  5. tweak the settings to the following:
    DATABASE_ROUTERS = ["dbrouter.testrouter.TestRouter"]
    DATABASES = {
        "default": {},
        'test': {
            'ENGINE': 'django.db.backends.sqlite3',
            'NAME': BASE_DIR / 'test.sqlite3',
        },
    }
    
  6. Double check no migrations are pending python -Wall manage.py makemigrations should return No changes detected
  7. 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
    
  8. Start a new app testapp, add a Question 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
    
  9. 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 Sarah Boyce, 5 months ago

Patch needs improvement: set
Note: See TracTickets for help on using tickets.
Back to Top