Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#26321 closed Bug (fixed)

Missing "for_save" in examples of custom expression

Reported by: Tomasz Nowak Owned by: nobody
Component: Documentation Version: 1.9
Severity: Normal Keywords: expressions
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

Example at ​https://docs.djangoproject.com/en/dev/ref/models/expressions/#writing-your-own-query-expressions doesn't work:

Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "xxx/django/db/models/query.py", line 909, in annotate
    clone.query.add_annotation(annotation, alias, is_summary=False)
  File "xxx/django/db/models/sql/query.py", line 974, in add_annotation
    summarize=is_summary)
  File "xxx/django/db/models/aggregates.py", line 19, in resolve_expression
    c = super(Aggregate, self).resolve_expression(query, allow_joins, reuse, summarize)
  File "xxx/django/db/models/expressions.py", line 534, in resolve_expression
    c.source_expressions[pos] = arg.resolve_expression(query, allow_joins, reuse, summarize, for_save)
TypeError: resolve_expression() takes at most 5 arguments (6 given)

Adding "for_save" parameter to the method signature and calls fixes this issue.
Please find the patch attached.

Attachments (1)

fixed_postgres_custom_agg_example.patch​ (1.5 KB ) - added by Tomasz Nowak 10 years ago.

Download all attachments as: .zip

Change History (7)

by Tomasz Nowak, 10 years ago

Attachment: fixed_postgres_custom_agg_example.patch​ added

comment:1 by Tomasz Nowak, 10 years ago

Easy pickings: set

comment:2 by Josh Smeaton, 10 years ago

Keywords: expressions added; postgres aggregate removed
Summary: Missing "for_save" in examples of custom PostgreSQL aggregates → Missing "for_save" in examples of custom expression
Triage Stage: Unreviewed → Accepted

I've renamed the case because this is about custom expressions, not specifically about postgres or aggregates (which don't actually use for_save). Patch looks mostly fine to me. Would you mind submitting it as a PR on GitHub? That'll allow review and the automated tests to run.

comment:3 by Tim Graham, 10 years ago

Triage Stage: Accepted → Ready for checkin

for_save was added in 65246de7b1d70d25831ab394c4f4a75813f629fe, so I'll backport to the 1.8 docs. No need to submit a PR really.

comment:4 by Tim Graham <timograham@…>, 10 years ago

Resolution: → fixed
Status: new → closed

In de8a11ba:

Fixed #26321 -- Added missing "for_save" parameter in expressions example.

Thanks tomaszn for the patch.

comment:5 by Tim Graham <timograham@…>, 10 years ago

In 377ca697:

[1.9.x] Fixed #26321 -- Added missing "for_save" parameter in expressions example.

Thanks tomaszn for the patch.

Backport of de8a11ba18d5902c668d4db47c38c9c6bdf9c1da from master

comment:6 by Tim Graham <timograham@…>, 10 years ago

In 8b891cf:

[1.8.x] Fixed #26321 -- Added missing "for_save" parameter in expressions example.

Thanks tomaszn for the patch.

Backport of de8a11ba18d5902c668d4db47c38c9c6bdf9c1da from master

Note: See TracTickets for help on using tickets.
Back to Top