[Django] #34727: Error in CharField with TextChoices and missing max_length parameter
Django
-------------------------------------+-------------------------------------
Reporter: | Owner: nobody
osamahasanone |
Type: Bug | Status: new
Component: | Version: 4.2
Uncategorized | Keywords: test, sqlite,
Severity: Normal | CharField, max_length,choices
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Dear Django Support Team,
I hope this message finds you well. I am writing to report an issue that I
encountered while using Django, specifically with the CharField and
TextChoices combination when specifying choices but not providing the
max_length parameter.
**Issue Description:**
When attempting to add a field of type CharField with TextChoices defined
but no max_length parameter in the model, running ./manage.py test USING
SQLITE results in the following error:
{{{
django.db.utils.OperationalError: near "None": syntax error
}}}
**Reproduction Steps:**
1. Define a Django model with a CharField, TextChoices, and choices, but
omit the max_length parameter:
{{{
from django.db import models
from django.utils.text import gettext_lazy as _
class DummyModel(models.Model):
class DummyFieldChoices(models.TextChoices):
OPTION1 = 'option1', _('Option 1')
OPTION2 = 'option2', _('Option 2')
OPTION3 = 'option3', _('Option 3')
dummy_field = models.CharField(
choices=DummyFieldChoices.choices,
default=DummyFieldChoices.OPTION1,
)
}}}
2. Run the Django test suite using WITH SQLITE (no issue with Postgres):
{{{
./manage.py test
}}}
**Expected Result:**
The Django test suite should run without any errors, and the model should
be properly created in the database.
Actual Result:
The Django test suite encounters an OperationalError, with the error
message being:
{{{
django.db.utils.OperationalError: near "None": syntax error
}}}
**Example of Correct Usage:**
{{{
from django.db import models
from django.utils.text import gettext_lazy as _
class DummyModel(models.Model):
class DummyFieldChoices(models.TextChoices):
OPTION1 = 'option1', _('Option 1')
OPTION2 = 'option2', _('Option 2')
OPTION3 = 'option3', _('Option 3')
dummy_field = models.CharField(
max_length=10, # Specify the appropriate maximum length for your
choices
choices=DummyFieldChoices.choices,
default=DummyFieldChoices.OPTION1,
)
}}}
**Additional Information:**
Django Version: [4.2.3]
Python Version: [3.11.4]
Database: [Sqlite]
I hope this information helps you understand the issue I encountered. If
you require any further details or assistance, please do not hesitate to
reach out.
Thank you for your attention to this matter, and I look forward to your
response.
--
Ticket URL: <https://code.djangoproject.com/ticket/34727>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
Component: Testing framework | Version: 4.2
Severity: Normal | Resolution:
Keywords: test, sqlite, | Triage Stage:
CharField, max_length,choices | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* component: Uncategorized => Testing framework
Comment:
`max_length` is required on databases other than PostgreSQL. I believe the
issue is that system checks (which would highlight this issue) are run
after the test database is created (which fails due to missing
`max_length`). I guess it's common to catch this sort of mistake using
`runserver` before running `test`.
--
Ticket URL: <https://code.djangoproject.com/ticket/34727#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: osamahasanone | Owner: nobody
Component: Testing framework | Version: 4.2
Keywords: test, sqlite, | Triage Stage:
CharField, max_length,choices | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => invalid
Comment:
I cannot reproduce your issue, when running `test`, `makemigrations`, or
`migrate`, an error is raised in all cases:
{{{
$ ./manage.py test
Found 1 test(s).
Creating test database for alias 'default'...
Destroying test database for alias 'default'...
SystemCheckError: System check identified some issues:
ERRORS:
test_one.DummyModel.dummy_field: (fields.E120) CharFields must define a
'max_length' attribute.
System check identified 1 issue (0 silenced).
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34727#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: osamahasanone | Owner: nobody
Type: Bug | Status: closed
Component: Testing framework | Version: 4.2
Severity: Normal | Resolution: invalid
Keywords: test, sqlite, | Triage Stage:
CharField, max_length,choices | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham):
Mariusz, did you perhaps remove `max_length` from a model without also
updating the migration? I don't see how valid SQL could be producd with
`None` interpolated as a
[https://github.com/django/django/blob/addbc90049083f1d5f7ac138ed00111b71a75233/django/db/backends/sqlite3/base.py#L64
max_length].
--
Ticket URL: <https://code.djangoproject.com/ticket/34727#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: osamahasanone | Owner: nobody
Type: Bug | Status: closed
Component: Testing framework | Version: 4.2
Severity: Normal | Resolution: invalid
Keywords: test, sqlite, | Triage Stage:
CharField, max_length,choices | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak):
Replying to [comment:3 Tim Graham]:
> Mariusz, did you perhaps remove `max_length` from a model without also
updating the migration? I don't see how valid SQL could be producd with
`None` interpolated as a
[https://github.com/django/django/blob/addbc90049083f1d5f7ac138ed00111b71a75233/django/db/backends/sqlite3/base.py#L64
max_length].
I missed that `makemigrations` didn't generate any migrations for me:
{{{
$ python manage.py makemigrations
SystemCheckError: System check identified some issues:
ERRORS:
test_one.DummyModel.dummy_field: (fields.E120) CharFields must define a
'max_length' attribute.
}}}
It seems that they used PostgreSQL to generate a migration and SQLite for
tests.
--
Ticket URL: <https://code.djangoproject.com/ticket/34727#comment:4>
Django
-------------------------------------+-------------------------------------
Reporter: osamahasanone | Owner: nobody
Type: Bug | Status: closed
Component: Testing framework | Version: 4.2
Severity: Normal | Resolution: invalid
Keywords: test, sqlite, | Triage Stage:
CharField, max_length,choices | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by osamahasanone):
** It seems that they used PostgreSQL to generate a migration and SQLite
for tests.**
That's correct.
We usually run tests against SQLite as it's faster.
--
Ticket URL: <https://code.djangoproject.com/ticket/34727#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: osamahasanone | Owner: nobody
Type: Bug | Status: closed
Component: Testing framework | Version: 4.2
Severity: Normal | Resolution: invalid
Keywords: test, sqlite, | Triage Stage:
CharField, max_length,choices | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham):
Offhand, I can't think of a reason why the test runner couldn't/shouldn't
run checks before creating the test database. That would catch this and
improve other situations where time is wasted on database setup only to
fail at the check stage. Thoughts?
--
Ticket URL: <https://code.djangoproject.com/ticket/34727#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: osamahasanone | Owner: nobody
Type: Bug | Status: closed
Component: Testing framework | Version: 4.2
Severity: Normal | Resolution: invalid
Keywords: test, sqlite, | Triage Stage:
CharField, max_length,choices | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak):
Replying to [comment:6 Tim Graham]:
> Offhand, I can't think of a reason why the test runner
couldn't/shouldn't run checks before creating the test database. That
would catch this and improve other situations where time is wasted on
database setup only to fail at the check stage. Thoughts?
Unfortunately, a database is required for some checks because database
features can only be confirmed by introspecting the database, e.g.
[https://github.com/django/django/blob/addbc90049083f1d5f7ac138ed00111b71a75233/django/db/backends/sqlite3/features.py#L149-L159
supports_json_field] on SQLite.
--
Ticket URL: <https://code.djangoproject.com/ticket/34727#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: osamahasanone | Owner: nobody
Type: Bug | Status: closed
Component: Testing framework | Version: 4.2
Severity: Normal | Resolution: invalid
Keywords: test, sqlite, | Triage Stage:
CharField, max_length,choices | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham):
I suppose database setup could be split between `CREATE DATABASE` and
table creation with checks run in between.
--
Ticket URL: <https://code.djangoproject.com/ticket/34727#comment:8>