Re: [Django] #14094: Cannot define CharField with unlimited length
Django
---------------------------------------+------------------------------
Reporter: millerdev | Owner: nobody
Type: New feature | Status: reopened
Milestone: | Component: Core (Other)
Version: 1.2 | Severity: Normal
Resolution: | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
---------------------------------------+------------------------------
Changes (by joe@…):
* status: closed => reopened
* severity: => Normal
* cc: joe@… (added)
* resolution: invalid =>
* easy: => 0
* ui_ux: => 0
* type: => New feature
Comment:
PostgreSQL allows char fields (which are distinct from Text fields) which
are "character varying" with no preset length.
It only makes sense that these should map to a Django CharField, with
max_length=None, rather than be forced to use a Django TextField, only
because of this limitation of Django.
In common PostgreSQL usage, these are small text fields of 20 or 50
characters or less, NOT large text fields.
Fixing this issue would also make the introspection of existing postgres
databases *much* smoother.
--
Ticket URL: <https://code.djangoproject.com/ticket/14094#comment:3>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: millerdev | Owner: nobody
Type: New | Status: reopened
feature | Component: Core (Other)
Milestone: | Severity: Normal
Version: 1.2 | Keywords:
Resolution: | Has patch: 0
Triage Stage: Design | Needs tests: 0
decision needed | Easy pickings: 0
Needs documentation: 0 |
Patch needs improvement: 0 |
UI/UX: 0 |
-------------------------------------+-------------------------------------
Changes (by aaugustin):
* stage: Unreviewed => Design decision needed
--
Ticket URL: <https://code.djangoproject.com/ticket/14094#comment:4>
Django
Reporter: millerdev | Owner: nobody
Type: New feature | Status: reopened
Component: Core (Other) | Version: 1.2
Severity: Normal | Resolution:
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by jacob):
* stage: Design decision needed => Accepted
Comment:
We should do this. Postgres, at least, has no performance or storage space
penalty for using `varchar` over `varchar(n)` -- the length requirement is
basically a constraint, that's all. I imagine other modern DBs are
similar, but even if not it's still a good idea to let users make the
call, not do it unilaterally.
--
Ticket URL: <https://code.djangoproject.com/ticket/14094#comment:5>
Django
------------------------------+------------------------------------
Reporter: millerdev | Owner: nobody
Type: New feature | Status: reopened
Component: Core (Other) | Version: 1.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------
Comment (by aaugustin):
I can't comment on performance, but I've always found choosing the length
of `CharField`s incredibly annoying.
"How long can this be? 100 chars? Fields are always too short, et's make
it 200. Oh why not 255, everyone does that..."
I'm in favor of removing the requirement to define `max_length` on
`CharField`s, and making them unlimited by default — assuming this isn't a
''Gun Aimed At Feet''.
--
Ticket URL: <https://code.djangoproject.com/ticket/14094#comment:6>
Django
------------------------------+------------------------------------
Reporter: millerdev | Owner: nobody
Type: New feature | Status: reopened
Component: Core (Other) | Version: 1.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------
Comment (by akaariai):
Oracle for example uses nvarchar2 for CharField, NCLOB for TextField,
MySQL has varchar and longtext... And these "unlimited" size text fields
really are different than limited size text field without the limit.
How about just having a "large" default value, like 1024 chars?
--
Ticket URL: <https://code.djangoproject.com/ticket/14094#comment:7>
Django
------------------------------+------------------------------------
Reporter: millerdev | Owner: nobody
Component: Core (Other) | Version: 1.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------
* cc: mightyiam (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/14094#comment:9>
Django
------------------------------+------------------------------------
Reporter: millerdev | Owner: nobody
Type: New feature | Status: new
Component: Core (Other) | Version: 1.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------
* cc: unai@… (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/14094#comment:10>
Django
------------------------------+------------------------------------
Reporter: millerdev | Owner: nobody
Type: New feature | Status: new
Component: Core (Other) | Version: 1.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------
Comment (by bendavis78):
I'm curious why this ticket is 4 years old. Everyone seems to be in
agreement that this should change, and it seems like a simple one (unless
I'm mistaken), and it's backward compatible.
I completely agree with aaugustin (comment:6). The max_length argument
should be reserved for use cases where someone wants to enforce the
length. Instead I'm having to pick arbitrary values for things that don't
need enforcing. It adds indirection to my models because it implies that
the value ''should'' be enforced when that's not really the case.
For databases like MySQL that require a length, the db field should be
created as VARCHAR(255) by default. The documentation already has a note
that says "some backends have restrictions on max length", so we would
just need to expand on that to explain the behavior when max_length is
None.
Also, using TextField is not the right answer -- it has a different
meaning in Django, regardless of how it's stored in the db.
--
Ticket URL: <https://code.djangoproject.com/ticket/14094#comment:11>
Django
------------------------------+------------------------------------
Reporter: millerdev | Owner: nobody
Type: New feature | Status: new
Component: Core (Other) | Version: 1.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------
Comment (by aaugustin):
The answer may be disappointing -- no one was sufficiently interested in
this issue to write a patch, that's all...
--
Ticket URL: <https://code.djangoproject.com/ticket/14094#comment:12>
Django
------------------------------+------------------------------------
Reporter: millerdev | Owner: nobody
Type: New feature | Status: new
Component: Core (Other) | Version: 1.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------
Comment (by bendavis78):
Ugh. I took a stab at it and fell into the black hole that is the Django
ORM. I'll try to summarize what I found here in case anyone is interested.
Fixing the validation was easy, just needed to delete a few lines of code.
Now, we could just override CharField's db_type() method to return
`"varchar"` and be done with it, but that doesn't seem like the right
solution to me, especially since CharField is a core field type. So, I
sought out to find the right way to fix it, but only uncovered more
hurdles.
The problem starts with how django db backends define their basic mappings
from django's "internal types" to the actual SQL definition. The mappings
all look something like `"varchar(%(max_length)s)"` and are used to format
a string given the field's `__dict__`. So the first hurdle was that
`Field.db_type()` was returning `"varchar(None)"`. I made some changes to
get it to replace `None` values with `''`. But then I found that
`"varchar()"` is invalid syntax in postgres -- you have to use just
`"varchar"` alone.
So, like Dante, I ventured deeper. The `db_type()` method relies on the
backend's `DatabaseCreation` class, which is where these mappings are
defined. One option I thought of was to have two internal types for
CharField:
{{{
#!python
data_types = {
#...
'CharField': 'varchar(%(max_length)s)',
'CharFieldNoLimit': 'varchar',
#..
}
}}}
But, there's no precident right now for having two internal types for the
same model field. Besides, these mappings don't help us differentiate
between "data type" and "modifier", which is what really needed to happen.
So, ''I ventured even deeper''.
The base `Field` defines `db_type()`, `db_parameters()`, and
`db_type_suffix()`. I would argue that `Field` shouldn't even implement
these methods, as they produce back-end specific results. Those methods
should really live in a back-end specific class. That's where I pretty
much threw my hands up. Django's whole schema system is in flux right now
due to migrations, and it's not really clear what the right answer is.
Well, that was fun. C'est la vie.
--
Ticket URL: <https://code.djangoproject.com/ticket/14094#comment:13>
Django
------------------------------+------------------------------------
Reporter: millerdev | Owner: nobody
Type: New feature | Status: new
Component: Core (Other) | Version: 1.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------
Comment (by erikr):
Thanks for digging into that. However, I think this isn't the biggest
problem. If I read the
[[http://docs.oracle.com/cd/B28359_01/server.111/b28318/datatype.htm#CNCPT1825|oracle
docs]] correctly, specifying a length is mandatory for `VARCHAR2`, and the
maximum is 4000 bytes. For
[[http://dev.mysql.com/doc/refman/5.7/en/char.html|MySQL]], it's 65K
bytes, for PostgreSQL and SQLite it's very large.
Therefore, we can't set it to an unlimited length for all databases: we
must pick something up to 4000 as maximum for Oracle. I don't think it's
acceptable to only require `max_length` for Oracle backends. Essentially,
I only see one option: default `max_length` to to the Oracle maximum. This
works the same in all databases, and will work for many cases. Anyone else
can still override it trivially. I'm not too enthusiastic about this
solution though.
Another challenge with this solution that, if I remember correctly,
`max_length` defines characters, whereas the oracle limit is 4000 bytes.
How characters translate to bytes depends on characters and encoding.
However, this might be solved already somewhere: after all, if you define
`max_length` on a field now, that value must not end up as the limit in
`varchar2(n)` due to the same concern.
I should add that I know very little about Oracle, so it's entirely
possible this is not the complete story for Oracle.
To summarise:
* An explicit `max_length` defined somewhere (either in your own code or
in Django) is always required for Oracle compatibility, if I read the docs
correctly.
* The maximum size can be no longer than 4000 bytes, which may not fit
4000 characters, depending on encoding. We may already have code in place
that resolves this concern.
* My suggestion is to give `max_length` on `CharField` a default that
matches 4000 bytes, but I'm not sure whether that's the best solution.
--
Ticket URL: <https://code.djangoproject.com/ticket/14094#comment:14>
Django
------------------------------+------------------------------------
Reporter: millerdev | Owner: nobody
Type: New feature | Status: new
Component: Core (Other) | Version: 1.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------
Comment (by charettes):
> My suggestion is to give max_length on CharField a default that matches
4000 bytes, but I'm not sure whether that's the best solution.
What about providing a backend specific `max_length` just like we do with
`IntegerField` and friends? See #12030 and
1506c71a95cd7f58fbc6363edf2ef742c58d2487.
--
Ticket URL: <https://code.djangoproject.com/ticket/14094#comment:15>
Django
------------------------------+------------------------------------
Reporter: millerdev | Owner: nobody
Type: New feature | Status: new
Component: Core (Other) | Version: 1.2
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------
Comment (by erikr):
Replying to [comment:15 charettes]:
> What about providing a backend specific `max_length` just like we do
with `IntegerField` and friends? See #12030 and
1506c71a95cd7f58fbc6363edf2ef742c58d2487.
Yes, that makes sense to me as well.
--
Ticket URL: <https://code.djangoproject.com/ticket/14094#comment:16>
Django
Reporter: millerdev | Owner: nobody
Type: New feature | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage: Accepted
Keywords: | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by aaugustin):
* component: Core (Other) => Database layer (models, ORM)
--
Ticket URL: <https://code.djangoproject.com/ticket/14094#comment:17>
Django
-------------------------------------+-------------------------------------
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by ar45):
* cc: ar45 (added)
* owner: nobody => ar45
* has_patch: 0 => 1
* version: 1.2 => master
* status: new => assigned
Comment:
I started to work on this here
[https://github.com/django/django/compare/master...ar45:max_length_optional]
I would appriciate any comments.
--
Ticket URL: <https://code.djangoproject.com/ticket/14094#comment:18>
Django
-------------------------------------+-------------------------------------
Reporter: millerdev | Owner: ar45
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by aaugustin):
Perhaps you should wait until the discussion on django-developers comes to
a consensus; new ideas are still being proposed.
--
Ticket URL: <https://code.djangoproject.com/ticket/14094#comment:19>
Django
-------------------------------------+-------------------------------------
Reporter: millerdev | Owner: ar45
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
Comment:
The consensus on the mailing list seems to be that making `max_length`
optional isn't a good idea.
--
Ticket URL: <https://code.djangoproject.com/ticket/14094#comment:20>
Django
-------------------------------------+-------------------------------------
Reporter: millerdev | Owner: ar45
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by srkunze):
Recently, I helped a friend bootstrapping a new Django project.
This issue (TextField vs CharField without max_length) again popped up
because I had trouble explaining why exactly there needs to be a limit.
Because I have some reasonable experience with Django I know about the
internal relationship between models and forms which again lead to some
questionable design decisions in the past.
My point here is there is not easy way for (new) Django users to decide
for either a CharField or a TextField.
CharField -> one line text data but limited
TextField -> multi-line text data
What most projects actually need is
CharField -> one line, unlimited text data
--
Ticket URL: <https://code.djangoproject.com/ticket/14094#comment:21>
Django
-------------------------------------+-------------------------------------
Reporter: millerdev | Owner: ar45
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by geekscrapy):
Agree that this should be made optional, but at the very least the docs
for 1.9 need amending as it describes this attribute as optional.
--
Ticket URL: <https://code.djangoproject.com/ticket/14094#comment:22>
Django
-------------------------------------+-------------------------------------
Reporter: millerdev | Owner: ar45
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by timgraham):
Please be more specific about the docs inaccuracy. I see
[https://docs.djangoproject.com/en/1.9/ref/models/fields/#charfield the
following]:
CharField has one extra required argument: ... CharField.max_length
Perhaps you're looking at the
[https://docs.djangoproject.com/en/1.9/ref/forms/fields/#charfield form
field docs] where the argument is indeed option.
--
Ticket URL: <https://code.djangoproject.com/ticket/14094#comment:23>
Django
-------------------------------------+-------------------------------------
| Podrigal
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Rich Rauenzahn (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/14094#comment:24>
Django
-------------------------------------+-------------------------------------
Reporter: Daniel Miller | Owner: Aron
| Podrigal
Type: New feature | Status: assigned
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Anvesh Mishra (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/14094#comment:25>
Django
-------------------------------------+-------------------------------------
| Torres
Type: New feature | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* cc: Adrian Torres (added)
* needs_better_patch: 1 => 0
* owner: Aron Podrigal => Adrian Torres
Comment:
I have submitted a patch at https://github.com/django/django/pull/16302
--
Ticket URL: <https://code.djangoproject.com/ticket/14094#comment:26>
Django
-------------------------------------+-------------------------------------
Reporter: Daniel Miller | Owner: Adrian
| Torres
Type: New feature | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/14094#comment:27>
Django
-------------------------------------+-------------------------------------
Reporter: Daniel Miller | Owner: Adrian
| Torres
Type: New feature | Status: assigned
Component: Database layer | Version: dev
(models, ORM) |
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):
* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/14094#comment:28>
Django
-------------------------------------+-------------------------------------
Reporter: Daniel Miller | Owner: Adrian
| Torres
Component: Database layer | Version: dev
(models, ORM) |
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"7eee1dca420ee7c4451d24cd32fa08aed0dcbb36" 7eee1dc]:
{{{
#!CommitTicketReference repository=""
revision="7eee1dca420ee7c4451d24cd32fa08aed0dcbb36"
Fixed #14094 -- Added support for unlimited CharField on PostgreSQL.
Co-authored-by: Mariusz Felisiak <felisiak...@gmail.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/14094#comment:29>