[Django] #22064: Invalid related_name passes silently

14 views
Skip to first unread message

Django

unread,
Feb 16, 2014, 7:26:49 AM2/16/14
to django-...@googlegroups.com
#22064: Invalid related_name passes silently
----------------------------------------------+---------------------
Reporter: mondone | Owner: mondone
Type: Uncategorized | Status: new
Component: Database layer (models, ORM) | Version: 1.6
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 1 | UI/UX: 0
----------------------------------------------+---------------------
Invalid related_name e.g. 'user sets' passes silently. It leads to
unpredictable problems. It is not a bug, nor new feature, just
RelatedField.check() little iprovement.

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

Django

unread,
Feb 16, 2014, 7:33:01 AM2/16/14
to django-...@googlegroups.com
#22064: Invalid related_name passes silently
-------------------------------------+-------------------------------------
Reporter: mondone | Owner: mondone
Type: | Status: new
Cleanup/optimization | Version: 1.6
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 1 |
-------------------------------------+-------------------------------------
Changes (by bmispelon):

* needs_better_patch: => 0
* needs_docs: => 0
* type: Uncategorized => Cleanup/optimization
* needs_tests: => 0
* stage: Unreviewed => Accepted


Comment:

Good idea, that sounds like a perfect use-case for the checks framework.

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

Django

unread,
Feb 16, 2014, 7:37:24 AM2/16/14
to django-...@googlegroups.com
#22064: Invalid related_name passes silently
-------------------------------------+-------------------------------------
Reporter: mondone | Owner: mondone
Type: | Status: new
Cleanup/optimization | Version: master

Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 1 |
Easy pickings: 1 |
-------------------------------------+-------------------------------------
Changes (by mondone):

* version: 1.6 => master
* needs_tests: 0 => 1


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

Django

unread,
Feb 16, 2014, 5:41:47 PM2/16/14
to django-...@googlegroups.com
#22064: Invalid related_name passes silently
-------------------------------------+-------------------------------------
Reporter: mondone | Owner: mondone

Type: | Status: new
Cleanup/optimization | Version: master
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 1 |
Easy pickings: 1 |
-------------------------------------+-------------------------------------

Comment (by mondone):

https://github.com/mondone/django/tree/ticket_22064
work in progress (tests)

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

Django

unread,
Mar 14, 2014, 9:35:32 AM3/14/14
to django-...@googlegroups.com
#22064: Invalid related_name passes silently
-------------------------------------+-------------------------------------
Reporter: mondone | Owner: elhippie
Type: | Status: assigned

Cleanup/optimization | Version: master
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 1 |
Easy pickings: 1 |
-------------------------------------+-------------------------------------
Changes (by elhippie):

* owner: mondone => elhippie
* status: new => assigned


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

Django

unread,
May 20, 2014, 12:12:54 PM5/20/14
to django-...@googlegroups.com
#22064: Invalid related_name passes silently
-------------------------------------+-------------------------------------
Reporter: mondone | Owner: elhippie
Type: | Status: assigned
Cleanup/optimization | Version: master
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 0 | UI/UX: 0
Needs tests: 1 |
Easy pickings: 1 |
-------------------------------------+-------------------------------------

Comment (by Akshay Katyal <ktyaks@…>):

Is anyone working on this? Been about 3 months since the last commit on
the branch.

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

Django

unread,
Sep 5, 2014, 1:24:32 PM9/5/14
to django-...@googlegroups.com
#22064: Invalid related_name passes silently
-------------------------------------+-------------------------------------
Reporter: mondone | Owner: elhippie
Type: | Status: assigned
Cleanup/optimization | Version: master
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 1
Has patch: 1 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 1 |
-------------------------------------+-------------------------------------
Changes (by timgraham):

* needs_better_patch: 0 => 1
* has_patch: 0 => 1
* needs_tests: 1 => 0


Comment:

The commit in comment 3 looks like a good start. There's a comment there
that says, "There is a problem with regex performance, it must be solved
before pull request" so if someone wants to take a look, that would be
helpful.

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

Django

unread,
Sep 29, 2014, 2:39:07 PM9/29/14
to django-...@googlegroups.com
#22064: Invalid related_name passes silently
-------------------------------------+-------------------------------------
Reporter: mondone | Owner: elhippie
Type: | Status: assigned
Cleanup/optimization | Version: master
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 1
Has patch: 1 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 1 |
-------------------------------------+-------------------------------------

Comment (by aericson):

Curious if the problem with regex is the fact that the commit uses two
regexes or is it that he uses regex at all?

I mean would it be preferable to have a faster less readable regex-free
solution or a slower more readable using regex?

Please read this as a general question from someone new here rather than
as specific to this ticket.

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

Django

unread,
Oct 3, 2014, 11:53:30 PM10/3/14
to django-...@googlegroups.com
#22064: Invalid related_name passes silently
-------------------------------------+-------------------------------------
Reporter: mondone | Owner: elhippie
Type: | Status: assigned
Cleanup/optimization | Version: master
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 1
Has patch: 1 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 1 |
-------------------------------------+-------------------------------------

Comment (by carljm):

The checks framework is not a runtime code path or a fast path at all, so
readability definitely trumps performance here. Barring catastrophic
performance issues, which a complex regex can be susceptible to.

Looking at the draft patch commit, a few comments:

1. I don't see why the "ends with +" case needs any regex at all. Any
related-name ending with "+" is valid, since it won't be used as a Python
identifier anyway. (I'm not sure why we even support "ends with +" rather
than just "+" -- what's the purpose of including anything before the
"+"?). So a simple `.endswith('+')` check should be sufficient there.

2. I'd suggest that we also check the `related_name` against
`keyword.iskeyword()`, since using a Python keyword as a related name
could also cause trouble.

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

Django

unread,
Oct 4, 2014, 3:50:12 PM10/4/14
to django-...@googlegroups.com
#22064: Invalid related_name passes silently
-------------------------------------+-------------------------------------
Reporter: mondone | Owner: elhippie
Type: | Status: assigned
Cleanup/optimization | Version: master
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 1
Has patch: 1 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 1 |
-------------------------------------+-------------------------------------

Comment (by aericson):

Thanks for your comments I'll be submitting a patch.

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

Django

unread,
Oct 4, 2014, 8:10:34 PM10/4/14
to django-...@googlegroups.com
#22064: Invalid related_name passes silently
-------------------------------------+-------------------------------------
Reporter: mondone | Owner: aericson

Type: | Status: assigned
Cleanup/optimization | Version: master
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 0

Has patch: 1 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 1 |
-------------------------------------+-------------------------------------
Changes (by aericson):

* owner: elhippie => aericson
* needs_better_patch: 1 => 0


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

Django

unread,
Oct 4, 2014, 8:13:11 PM10/4/14
to django-...@googlegroups.com
#22064: Invalid related_name passes silently
-------------------------------------+-------------------------------------
Reporter: mondone | Owner: aericson
Type: | Status: assigned
Cleanup/optimization | Version: master
Component: Database layer | Resolution:
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 1 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 1 |
-------------------------------------+-------------------------------------

Comment (by aericson):

Assigned it to myself as it seems to be inactive for months.
Submitted a new patch.

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

Django

unread,
Oct 6, 2014, 4:06:46 PM10/6/14
to django-...@googlegroups.com
#22064: Invalid related_name passes silently
-------------------------------------+-------------------------------------
Reporter: mondone | Owner: aericson
Type: | Status: closed
Cleanup/optimization | Version: master
Component: Database layer | Resolution: fixed

(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 1 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 1 |
-------------------------------------+-------------------------------------
Changes (by André Ericson <de.ericson@…>):

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


Comment:

In [changeset:"1e5e2a470768549117ac4696ce3e8b4e51d65664"]:
{{{
#!CommitTicketReference repository=""
revision="1e5e2a470768549117ac4696ce3e8b4e51d65664"
Fixed #22064 -- Add check for related_name

Validates that related_name is a valid Python id or ends with a '+' and
it's not a keyword. Without a check it passed silently leading to
unpredictable problems.

Thanks Konrad Świat for the initial work.
}}}

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

Django

unread,
Oct 6, 2014, 4:06:46 PM10/6/14
to django-...@googlegroups.com
#22064: Invalid related_name passes silently
-------------------------------------+-------------------------------------
Reporter: mondone | Owner: aericson
Type: | Status: closed
Cleanup/optimization | Version: master
Component: Database layer | Resolution: fixed
(models, ORM) | Triage Stage: Accepted
Severity: Normal | Needs documentation: 0
Keywords: | Patch needs improvement: 0
Has patch: 1 | UI/UX: 0
Needs tests: 0 |
Easy pickings: 1 |
-------------------------------------+-------------------------------------

Comment (by Carl Meyer <carl@…>):

In [changeset:"6f6e7d01dce94668e178b26da547c4643ed3a6cc"]:
{{{
#!CommitTicketReference repository=""
revision="6f6e7d01dce94668e178b26da547c4643ed3a6cc"
Merge pull request #3308 from aericson/ticket_22064

Fixed #22064 -- Add check for related_name
}}}

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

Reply all
Reply to author
Forward
0 new messages