Opened 7 years ago
Closed 7 years ago
#29505 closed Bug (fixed)
SchemaEditor prohibits setting a field's default value to a callable that'll be used as the default
| Reported by: | Eugene Pakhomov | Owned by: | nobody |
|---|---|---|---|
| Component: | Migrations | Version: | 2.0 |
| Severity: | Normal | Keywords: | default |
| Cc: | Triage Stage: | Accepted | |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
I'm trying to use https://github.com/cognitect/transit-python and I created a custom KeywordField for this.
The problem is that transit.transit_types.Keyword is a callable. Now, even if I override _get_default in KeywordField, it still won't work because django.db.backends.base.schema.BaseDatabaseSchemaEditor#effective_default checks the value again.
I think a simple fix would be to remove this check from effective_default and immediately make a call where it's necessary (datetime.date etc).
Attachments (1)
Change History (9)
comment:1 by , 7 years ago
| Component: | Database layer (models, ORM) → Migrations |
|---|---|
| Easy pickings: | unset |
comment:2 by , 7 years ago
Suppose I have some custom type like this one
class MyStr(str): def __call__(self, other): return self + other
and a corresponding custom field type that inherits from Django's TextField and implements all necessary methods so that MyStr is used everywhere instead of the regular str.
Currently, it's not possible without manually creating a DatabaseWrapper with a custom DatabaseSchemaEditor because even if you handle the fact that instances of MyStr are callable in the custom field type, they will still be called in django.db.backends.base.schema.BaseDatabaseSchemaEditor#effective_default.
Attached a patch that fixes this issue. Tests appear to be fine.
by , 7 years ago
| Attachment: | 29505.diff added |
|---|
comment:3 by , 7 years ago
| Resolution: | → wontfix |
|---|---|
| Status: | new → closed |
| Summary: | Impossible to set a Field's default value to a callable → Impossible to set a Field's default value to a callable that takes an argument |
I believe the proposed patch would break callable defaults. In effective_default(), field.get_default() may return a callable and Field.get_prep_value() won't work properly if it receives a callable instead of a value. I guess test coverage is lacking.
I think you should change your __call__() method to make other an optional argument.
comment:4 by , 7 years ago
As I said in the issue description - the types under question are not under my control, so I cannot change __call__(). And my concern doesn't have to do anything with my particular situation - I have already solved it by setting a custom DatabaseSchemaEditor. My concern is that this issue is still there, and other people will have to spend some time debugging it.
field.get_default() may return a callable
You're incorrect. Django itself doesn't do that. And Model#__init__() implementation calls get_default() without checking whether it's result is callable or not because the default implementation of get_default() always does this check. If get_default() could return a callable, Model#__init__() would not work correctly - it would set the corresponding field in all instances to the callable, without calling it to get the default value.
comment:5 by , 7 years ago
Also, the change in title is incorrect.
Technically, it's possible to set the default value to something that is a callable without arguments, yes. But it's impossible to also have this value as the default, instead of just have it be called for a default.
Consider my example above but without the other argument. You still won't be able to set an instance of MyStr as the default value.
comment:6 by , 7 years ago
| Has patch: | set |
|---|---|
| Needs tests: | set |
| Resolution: | wontfix |
| Status: | closed → new |
| Summary: | Impossible to set a Field's default value to a callable that takes an argument → SchemaEditor prohibits setting a field's default value to a callable that'll be used as the default |
| Triage Stage: | Unreviewed → Accepted |
| Type: | Uncategorized → Bug |
After a second look, I agree with your analysis. The patch looks okay, and the test will probably go in tests/schema.
I don't understand the issue. Please explain in more detail and perhaps offer a patch to explain.