[Django] #26351: get_or_create called twice on a model with CharField and field value over max

Django

unread,

Mar 13, 2016, 6:34:02 AM3/13/16

to django-...@googlegroups.com

#26351: get_or_create called twice on a model with CharField and field value over
max_length creates 2 objects
-------------------------------------+-------------------------------------
Reporter: zhebrak | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.9
(models, ORM) | Keywords: get_or_create,
Severity: Normal | max_length
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
{{{#!python
class Post(models.Model):
title = models.CharField(max_length=10)

search_title = 'A' * 11

Post.objects.get_or_create(title=search_title)
Post.objects.get_or_create(title=search_title)
}}}

This code creates two objects because lookup is trying to find non-
truncated title and fails to get one so it creates a new object with a
truncated title. The second call does exactly the same creating another
object.

I don't know if it supposed to work this way or not but this behavior
seems unclear to me.

--
Ticket URL: <https://code.djangoproject.com/ticket/26351>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,

Mar 13, 2016, 11:28:04 PM3/13/16

to django-...@googlegroups.com

#26351: get_or_create called twice on a model with CharField and field value over
max_length creates 2 objects
-------------------------------------+-------------------------------------
Reporter: zhebrak | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.9
(models, ORM) |

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by mdunhem):

* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0

Comment:

What database are you using? That should fail validation and raise an
exception rather than truncating the title field. This may really depend
on your database type.

--
Ticket URL: <https://code.djangoproject.com/ticket/26351#comment:1>

Django

unread,

Mar 14, 2016, 5:21:21 AM3/14/16

to django-...@googlegroups.com

#26351: get_or_create called twice on a model with CharField and field value over
max_length creates 2 objects
-------------------------------------+-------------------------------------
Reporter: zhebrak | Owner: nobody
Type: Bug | Status: new
Component: Database layer | Version: 1.9
(models, ORM) |

Severity: Normal | Resolution:
Keywords: get_or_create, | Triage Stage:
max_length | Unreviewed
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by zhebrak):

Replying to [comment:1 mdunhem]:

> What database are you using? That should fail validation and raise an
exception rather than truncating the title field. This may really depend
on your database type.

We use MySQL (5.1, 5.5). By default it truncates any given string by
max_length for the sake of not giving errors. It's not obvious though,
especially when using get_or_create.

--
Ticket URL: <https://code.djangoproject.com/ticket/26351#comment:2>

Django

unread,

Mar 14, 2016, 8:22:00 AM3/14/16

to django-...@googlegroups.com

#26351: Recommend enabling MySQL's STRICT_TRANS_TABLES to prevent silent truncation

-------------------------------------+-------------------------------------
Reporter: zhebrak | Owner: nobody

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by timgraham):

* type: Bug => Cleanup/optimization
* component: Database layer (models, ORM) => Documentation
* stage: Unreviewed => Accepted

Comment:

I think there's not much Django can do except to recommend enabling
`STRICT_TRANS_TABLES` to avoid silent truncation in the MySQL database
notes in `docs/ref/databases.txt`. It's enabled by default for MySQL 5.7+.

--
Ticket URL: <https://code.djangoproject.com/ticket/26351#comment:3>

Django

unread,

Mar 14, 2016, 9:38:27 AM3/14/16

to django-...@googlegroups.com

#26351: Recommend enabling MySQL's STRICT_TRANS_TABLES to prevent silent truncation

-------------------------------------+-------------------------------------
Reporter: zhebrak | Owner: nobody

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by claudep):

#15940 is related.

--
Ticket URL: <https://code.djangoproject.com/ticket/26351#comment:4>

Django

unread,

Mar 14, 2016, 9:54:34 AM3/14/16

to django-...@googlegroups.com

#26351: Recommend enabling MySQL's STRICT_TRANS_TABLES to prevent silent truncation

-------------------------------------+-------------------------------------
Reporter: zhebrak | Owner: nobody

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by claudep):

See also the [https://github.com/adamchainz/django-
mysql/commit/5ea46822c403f85964f3f9aba0b412bcfb167563
`strict_mode_warning` check] in django-mysql.

--
Ticket URL: <https://code.djangoproject.com/ticket/26351#comment:5>

Django

unread,

Mar 14, 2016, 3:08:57 PM3/14/16

to django-...@googlegroups.com

#26351: Recommend enabling MySQL's STRICT_TRANS_TABLES to prevent silent truncation

-------------------------------------+-------------------------------------
Reporter: zhebrak | Owner: nobody

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by claudep):

* has_patch: 0 => 1

Comment:

A [https://github.com/django/django/pull/6292 possible resolution] by
implementing a database check (partially copied from django-mysql).

--
Ticket URL: <https://code.djangoproject.com/ticket/26351#comment:6>

Django

unread,

Mar 14, 2016, 7:44:49 PM3/14/16

to django-...@googlegroups.com

#26351: Recommend enabling MySQL's STRICT_TRANS_TABLES to prevent silent truncation

-------------------------------------+-------------------------------------
Reporter: zhebrak | Owner: nobody

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by timgraham):

* needs_better_patch: 0 => 1

Comment:

This would be extending the system check framework in a new direction
(currently it's mostly limited to static code analysis). While I don't see
a big problem with this, I'll raise the alternative of a separate
`testconnections` command as proposed in #24475. Anyway, I left some
comments for improvement on the patch as currently proposed.

--
Ticket URL: <https://code.djangoproject.com/ticket/26351#comment:7>

Django

unread,

Mar 15, 2016, 7:04:00 AM3/15/16

to django-...@googlegroups.com

#26351: Recommend enabling MySQL's STRICT_TRANS_TABLES to prevent silent truncation

-------------------------------------+-------------------------------------
Reporter: zhebrak | Owner: nobody

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by adamchainz):

* cc: me@… (added)

--
Ticket URL: <https://code.djangoproject.com/ticket/26351#comment:8>

Django

unread,

Mar 15, 2016, 11:34:40 AM3/15/16

to django-...@googlegroups.com

#26351: Recommend enabling MySQL's STRICT_TRANS_TABLES to prevent silent truncation

-------------------------------------+-------------------------------------
Reporter: zhebrak | Owner: nobody

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by mdunhem):

Should we also add a note in the documentation?

--
Ticket URL: <https://code.djangoproject.com/ticket/26351#comment:9>

Django

unread,

Mar 17, 2016, 10:15:19 AM3/17/16

to django-...@googlegroups.com

#26351: Recommend enabling MySQL's STRICT_TRANS_TABLES to prevent silent truncation

-------------------------------------+-------------------------------------
Reporter: zhebrak | Owner: nobody

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by collinanderson):

As a cross reference, there are warnings generated when there's
truncation, and they used to be errors, but only in debug mode. #23871

--
Ticket URL: <https://code.djangoproject.com/ticket/26351#comment:10>

Django

unread,

Mar 19, 2016, 9:06:41 AM3/19/16

to django-...@googlegroups.com

#26351: Recommend enabling MySQL's STRICT_TRANS_TABLES to prevent silent truncation

-------------------------------------+-------------------------------------
Reporter: zhebrak | Owner: nobody

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by claudep):

* needs_better_patch: 1 => 0

Comment:

I've updated the pull request and separated it in two commits. In the
first one, I've extended the `DatabaseValidation` class to host database-
related checks. I also limited database-related checks to only the
`migrate` command so we don't get those warnings/errors for each check
run.

--
Ticket URL: <https://code.djangoproject.com/ticket/26351#comment:11>

Django

unread,

Mar 19, 2016, 9:17:13 AM3/19/16

to django-...@googlegroups.com

#26351: Recommend enabling MySQL's STRICT_TRANS_TABLES to prevent silent truncation

-------------------------------------+-------------------------------------
Reporter: zhebrak | Owner: nobody

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by shaib):

If MySql has already made `STRICT_TRANS_TABLES` the default, I suggest we
do the same. In a few versions, when MySql 5.5 and 5.6 reach EOL, we'll be
there anyway.

This comment is almost orthogonal to the solution of database checks,
which is a generally needed feature. The only point of contact is the
check for `STRICT_TRANS_TABLES` itself, since if we default to it, a user
who explicitly chose otherwise doesn't need to be warned about their
choice.

--
Ticket URL: <https://code.djangoproject.com/ticket/26351#comment:12>

Django

unread,

Mar 19, 2016, 9:58:48 AM3/19/16

to django-...@googlegroups.com

#26351: Recommend enabling MySQL's STRICT_TRANS_TABLES to prevent silent truncation

-------------------------------------+-------------------------------------
Reporter: zhebrak | Owner: nobody

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by claudep):

I'm +0 about forcing `STRICT_TRANS_TABLES`. I think the main advantage of
the check over forcing the setting is that we don't load each connection
creation on runtime with the cruft of SELECTing the variable to check its
current state. The other minor advantage is that people are still free to
configure their database at their will, even if I don't see now a use case
for that.

--
Ticket URL: <https://code.djangoproject.com/ticket/26351#comment:13>

Django

unread,

Mar 23, 2016, 11:39:23 AM3/23/16

to django-...@googlegroups.com

#26351: Recommend enabling MySQL's STRICT_TRANS_TABLES to prevent silent truncation

-------------------------------------+-------------------------------------
Reporter: zhebrak | Owner: nobody

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by timgraham):

I guess it's a good point about whether we should add the check now that
MySQL is changing the default of `STRICT_TRANS_TABLES`. As Shai said,
presumably going forward then, this check will only trigger for users
configuring the database otherwise, so I wonder if it adds any value
except for users of older versions of MySQL?

--
Ticket URL: <https://code.djangoproject.com/ticket/26351#comment:14>

Django

unread,

Mar 23, 2016, 12:14:35 PM3/23/16

to django-...@googlegroups.com

#26351: Recommend enabling MySQL's STRICT_TRANS_TABLES to prevent silent truncation

-------------------------------------+-------------------------------------
Reporter: zhebrak | Owner: nobody

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by claudep):

I think having the check is a good mean to warn users that their MySQL
configuration (being for an old MySQL or a MySQL with custom config) might
interfere with their Django project running properly. Then, if the choice
is deliberate, they can safely disable the warning.

--
Ticket URL: <https://code.djangoproject.com/ticket/26351#comment:15>

Django

unread,

Mar 25, 2016, 1:35:17 PM3/25/16

to django-...@googlegroups.com

#26351: Recommend enabling MySQL's STRICT_TRANS_TABLES to prevent silent truncation

-------------------------------------+-------------------------------------
Reporter: zhebrak | Owner: nobody

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by adamchainz):

It's also valuable for MariaDB users where it's not yet the default.

--
Ticket URL: <https://code.djangoproject.com/ticket/26351#comment:16>

Django

unread,

Mar 25, 2016, 5:25:31 PM3/25/16

to django-...@googlegroups.com

#26351: Recommend enabling MySQL's STRICT_TRANS_TABLES to prevent silent truncation

-------------------------------------+-------------------------------------
Reporter: zhebrak | Owner: nobody

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by timgraham):

How does the proposed patch interact with #15940? It's not clear to me
whether this solves the concerns of that ticket or if something else might
need to be done to address it after adding this check?

--
Ticket URL: <https://code.djangoproject.com/ticket/26351#comment:17>

Django

unread,

Mar 26, 2016, 5:22:54 AM3/26/16

to django-...@googlegroups.com

#26351: Recommend enabling MySQL's STRICT_TRANS_TABLES to prevent silent truncation

-------------------------------------+-------------------------------------
Reporter: zhebrak | Owner: nobody

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by claudep):

I think it does. IMHO the most important thing is to make people aware of
the possible truncation issue. What they do next is their business.

--
Ticket URL: <https://code.djangoproject.com/ticket/26351#comment:18>

Django

unread,

Mar 26, 2016, 6:17:54 AM3/26/16

to django-...@googlegroups.com

#26351: Recommend enabling MySQL's STRICT_TRANS_TABLES to prevent silent truncation

-------------------------------------+-------------------------------------
Reporter: zhebrak | Owner: nobody

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by timgraham):

Okay. The `docs/ref/databases.txt` note in your patch recommends
`STRICT_TRANS_TABLES` as opposed to `STRICT_ALL_TABLES` or `'sql_mode':
'traditional'` as mentioned in the other ticket. If there's a reason to
prefer one or the other, I didn't see it discussed. Not a MySQL user
myself, just wanted to check about that.
[http://dev.mysql.com/doc/refman/5.7/en/sql-mode.html MySQL SQL mode
reference docs].

--
Ticket URL: <https://code.djangoproject.com/ticket/26351#comment:19>

Django

unread,

Mar 26, 2016, 11:57:59 AM3/26/16

to django-...@googlegroups.com

#26351: Recommend enabling MySQL's STRICT_TRANS_TABLES to prevent silent truncation

-------------------------------------+-------------------------------------
Reporter: zhebrak | Owner: nobody

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by claudep):

I updated the docs to mention various modes, and also added a link to the
MySQL docs. I also accepted `traditional` as an accepted strict mode.

--
Ticket URL: <https://code.djangoproject.com/ticket/26351#comment:20>

Django

unread,

Mar 29, 2016, 11:12:49 AM3/29/16

to django-...@googlegroups.com

#26351: Recommend enabling MySQL's STRICT_TRANS_TABLES to prevent silent truncation

-------------------------------------+-------------------------------------
Reporter: zhebrak | Owner: nobody

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by timgraham):

* needs_better_patch: 0 => 1

Comment:

Left some comments for improvement.

--
Ticket URL: <https://code.djangoproject.com/ticket/26351#comment:21>

Django

unread,

Apr 1, 2016, 3:17:22 AM4/1/16

to django-...@googlegroups.com

#26351: Recommend enabling MySQL's STRICT_TRANS_TABLES to prevent silent truncation

-------------------------------------+-------------------------------------
Reporter: zhebrak | Owner: nobody

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by claudep):

* needs_better_patch: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/26351#comment:22>

Django

unread,

Apr 1, 2016, 8:18:49 PM4/1/16

to django-...@googlegroups.com

#26351: Recommend enabling MySQL's STRICT_TRANS_TABLES to prevent silent truncation

-------------------------------------+-------------------------------------
Reporter: zhebrak | Owner: nobody

Type: | Status: new
Cleanup/optimization |
Component: Documentation | Version: 1.9
Severity: Normal | Resolution:

Keywords: get_or_create, | Triage Stage: Ready for
max_length | checkin

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by timgraham):

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/26351#comment:23>

Django

unread,

Apr 8, 2016, 2:35:47 PM4/8/16

to django-...@googlegroups.com

#26351: Recommend enabling MySQL's STRICT_TRANS_TABLES to prevent silent truncation

-------------------------------------+-------------------------------------
Reporter: zhebrak | Owner: nobody

Type: | Status: closed

Cleanup/optimization |
Component: Documentation | Version: 1.9

Severity: Normal | Resolution: fixed

Keywords: get_or_create, | Triage Stage: Ready for
max_length | checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Claude Paroz <claude@…>):

* status: new => closed
* resolution: => fixed

Comment:

In [changeset:"f9a2a7db173b53f9cb0cd2d3def80d4eed72631c" f9a2a7d]:
{{{
#!CommitTicketReference repository=""
revision="f9a2a7db173b53f9cb0cd2d3def80d4eed72631c"
Fixed #26351 -- Added MySQL check to warn about strict mode option

Thanks Adam Chainz for the initial implementation in django-mysql.
Thanks Adam Chainz, Tim Graham, and Shai Berger for the reviews.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/26351#comment:25>

Django

unread,

Apr 8, 2016, 2:35:47 PM4/8/16

to django-...@googlegroups.com

#26351: Recommend enabling MySQL's STRICT_TRANS_TABLES to prevent silent truncation

-------------------------------------+-------------------------------------
Reporter: zhebrak | Owner: nobody

Type: | Status: new

Cleanup/optimization |
Component: Documentation | Version: 1.9
Severity: Normal | Resolution:

Keywords: get_or_create, | Triage Stage: Ready for
max_length | checkin
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Claude Paroz <claude@…>):

In [changeset:"0d3c616fbb2f49fa7ff6809e5a6777275352b35b" 0d3c616]:
{{{
#!CommitTicketReference repository=""
revision="0d3c616fbb2f49fa7ff6809e5a6777275352b35b"
Refs #26351 -- Added check hook to support database-related checks

Thanks Tim Graham and Shai Berger for the reviews.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/26351#comment:24>

Django

unread,

Apr 12, 2016, 9:04:51 AM4/12/16

to django-...@googlegroups.com

#26351: Recommend enabling MySQL's STRICT_TRANS_TABLES to prevent silent truncation
-------------------------------------+-------------------------------------
Reporter: zhebrak | Owner: nobody

Type: | Status: closed

Cleanup/optimization |
Component: Documentation | Version: 1.9

Severity: Normal | Resolution: fixed

Comment (by Tim Graham <timograham@…>):

In [changeset:"93deb1691eb27dc89135511fb0c10e077c8baca7" 93deb16]:
{{{
#!CommitTicketReference repository=""
revision="93deb1691eb27dc89135511fb0c10e077c8baca7"
Fixed #26492 -- Fixed "maximum recursion depth exceeded" migrate error.

A regression introduced in 0d3c616fbb2f49fa7ff6809e5a6777275352b35b;
refs #26351.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/26351#comment:26>

Reply all

Reply to author

Forward

[Django] #26351: get_or_create called twice on a model with CharField and field value over max_length creates 2 objects

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django

Django