Opened 5 months ago

Closed 4 months ago

Last modified 4 months ago

#28188 closed Bug (fixed)

Field._get_default() prevents model fields from being pickled

Reported by: Adam Alton Owned by: GitHub <noreply@…>
Component: Database layer (models, ORM) Version: 1.11
Severity: Release blocker Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Adam Alton)

The changes introduced in d2a26c1a90e837777dabdf3d67ceec4d2a70fb86 mean that in cases where a model field has a default and the default is not a callable, the field instance cannot be pickled.

The offending line is:

  return lambda: self.default

This problem only exists in Django version >= 1.11.

The reason for that lambda is because the code has been refactored so that all the logic for getting the default value has been moved into the _get_default method, which allows it to be cached with the @cached_property decorator. And then the get_default method expects the value returned from _get_default to always be a callable. That requirement for the value to always be a callable is the reason for wrapping the value with lambda in the case where it's not already a callable.

My proposed solution is to simply move the if callable() check back out into the get_default method. This would mean that we lose a very minor bit of efficiency because we have to run that one if statement each time that get_default is called, but the rest of the logic would remain inside the cached _get_default method.

I have made a commit here which contains my proposed fix and a test to prevent regression: https://github.com/adamalton/django/commit/ee6aafacfd01f2603943e8c4183805fd4a298355

Change History (5)

comment:1 Changed 5 months ago by Tim Graham

Component: UncategorizedDatabase layer (models, ORM)
Severity: NormalRelease blocker
Summary: Field._get_default prevents model fields from being pickledField._get_default() prevents model fields from being pickled
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug

comment:2 Changed 5 months ago by Adam Alton

Description: modified (diff)
Owner: changed from nobody to Adam Alton
Status: newassigned

comment:3 Changed 5 months ago by Adam Alton

Has patch: set
Owner: Adam Alton deleted
Status: assignednew

PR

I've made the pull request to merge into the stable/1.11.x branch (which is what I branched from). I hope that's the right thing to do. Let me know if it's not.

comment:4 Changed 4 months ago by GitHub <noreply@…>

Owner: set to GitHub <noreply@…>
Resolution: fixed
Status: newclosed

In a9874d48:

Fixed #28188 -- Fixed crash when pickling model fields.

Regression in d2a26c1a90e837777dabdf3d67ceec4d2a70fb86.

Thanks Adam Alton for the report and test, and Adam Johnson for
suggesting the fix.

comment:5 Changed 4 months ago by Tim Graham <timograham@…>

In 74b0837:

[1.11.x] Fixed #28188 -- Fixed crash when pickling model fields.

Regression in d2a26c1a90e837777dabdf3d67ceec4d2a70fb86.

Thanks Adam Alton for the report and test, and Adam Johnson for
suggesting the fix.

Backport of a9874d48b1b9d91988b9f299726ec4f559fb2f75 from master

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