#35269 closed New feature (invalid)
GeneratedFields can't be defined on RelatedFields
Reported by: | Perrine L. | Owned by: | nobody |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | 5.0 |
Severity: | Normal | Keywords: | |
Cc: | Perrine L. | Triage Stage: | Unreviewed |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
It is not possible to define a GeneratedField with a ForeignKey or a ManyToManyField as an output_field.
When those RelatedFields are defined as output_field, an error is raised
AttributeError: 'ForeignKey' object has no attribute 'opts'
line 190, in _check_relation_model_exists or line 494, in related_query_name (cf. https://github.com/django/django/blob/3d4fe39bac082b835a2d82b717b6ae88ea70ea15/django/db/models/fields/related.py)
At least with Postgres as a backend, there doesn't seem to be any technical impossibility to make a GeneratedField on a ForeignKey or ManyToMany fields : https://www.postgresql.org/docs/current/ddl-generated-columns.html
Change History (9)
comment:1 by , 10 months ago
Resolution: | → needsinfo |
---|---|
Status: | new → closed |
comment:2 by , 9 months ago
Hello, I think I have developed something that would do for this bug. With RelatedField on a GeneratedField it is possible to use all wonderful Django ORM lookup features.
https://github.com/feroda/play-django-generatedfk
Now my implementation requires a separate field (I have called NoopForeignKey https://github.com/feroda/play-django-generatedfk/blob/main/noopfk/fields.py), in addition to the GeneratedField, but I think it is not too much work to integrate the feature in a dedicated field to be used as output_field
(maybe: models.GeneratedForeignKey?) and I am almost confident that it could be a benefit also for referencing F() foreign key fields that should not provide only the value of the foreign key, but also the RelatedManager too.
What do you think about it?
comment:3 by , 9 months ago
Resolution: | needsinfo |
---|---|
Status: | closed → new |
follow-up: 6 comment:4 by , 9 months ago
Resolution: | → invalid |
---|---|
Status: | new → closed |
Hi Perrine, this ticket should only be re-opened if you do as Mariusz requested and provide a valid column DDL definition that you believe is not currently supported.
I believe this will not be possible as the linked documentation states and Mariusz mentioned, GeneratedFields
cannot reference anything other than the current row in any way and being a ForeignKey
is a reference to an external model.
Thank you for the sample project and I can see what you're trying to do. As what you're suggesting would be a new feature to Django, the recommended path forward is to first propose and discuss the idea with the community and gain consensus that this would be a desirable addition to Django. To do that, please consider starting a new conversation on the Django Forum, where you'll reach a wider audience and likely get extra feedback.
I'll close this ticket, but if there is a community agreement for the feature request, you are welcome to create a ticket for the new feature and point to the forum topic. For more details, please see the documented guidelines for requesting features.
comment:5 by , 9 months ago
Type: | Bug → New feature |
---|
comment:6 by , 9 months ago
Hello Sarah,
thanks for the answer
Replying to Sarah Boyce:
Hi Perrine,
Perrine?!? Sorry...I think you are answering me, and not Perrine. I hope to be right.
this ticket should only be re-opened if you do as Mariusz requested and provide a valid column DDL definition that you believe is not currently supported.
I have opened the issue without a DDL because the matter is just a way on how Django give an accessor to ForeignKey attributes, mainly very useful lookup. And this can be done (at least in PostgreSQL) even without constraints. So no DDL really required for this feature.
I believe this will not be possible as the linked documentation states and Mariusz mentioned,
GeneratedFields
cannot reference anything other than the current row in any way and being aForeignKey
is a reference to an external model.
Yes, the GeneratedField can reference only info of its own row. I have had the problem when I wanted to created a generated field with a CASE WHEN and apply a Now(), aka CURRENT_TIMESTAMP as a default value. This cannot be done.
But in this case the ForeignKey is just and ID and it is computed from data of its own row. It is really the same as a BigIntegerField (or a CharField if you use id like ssn), but it gives you a quicker way to access to the related model.
So, I really appreciate your answer, but my humble opinion disagrees with the position that implementing this feature would imply that GeneratedField relies in external data.
Thank you for the sample project and I can see what you're trying to do. As what you're suggesting would be a new feature to Django, the recommended path forward is to first propose and discuss the idea with the community and gain consensus that this would be a desirable addition to Django. To do that, please consider starting a new conversation on the Django Forum, where you'll reach a wider audience and likely get extra feedback.
I'll close this ticket, but if there is a community agreement for the feature request, you are welcome to create a ticket for the new feature and point to the forum topic. For more details, please see the documented guidelines for requesting features.
I leave the ticket closed, but I have answered here to return you with a feedback, and to link the ticket in the forum where I will go.
Thank you very much!
follow-up: 8 comment:7 by , 9 months ago
You don't need any GeneratedField
subclass to achieve what you're after, simply use ForeignObject
which is the accessor base of ForeignKey
.
from django.db.models.fields.related_fields import ForeignObject class Event(models.Model): last_updated_by_id = models.GeneratedField( expression=Greatest("created_by", "confirmed_by", "canceled_by"), output_field=models.BigIntegerField(), db_index=True, db_persist=True, ) last_updated_by = ForeignObject( User, models.SET_NULL, from_fields=["last_updated_by_id"], to_fields=["id"], related_name="last_updated_events_set", null=True, )
I also believe there is no reason to add this feature in core as with these two primitives it should be trivial to implement a ForeignGeneratedField(ForeignObject)
that does exactly what you're after in a third-party application for the rare use case this is useful.
comment:8 by , 9 months ago
Replying to Simon Charette:
You don't need any
GeneratedField
subclass to achieve what you're after, simply useForeignObject
which is the accessor base ofForeignKey
.
Thank you very much for the snippet Simon, I am going to try for sure!
I also believe there is no reason to add this feature in core as with these two primitives it should be trivial to implement a
ForeignGeneratedField(ForeignObject)
that does exactly what you're after in a third-party application for the rare use case this is useful.
I am not sure this is a rare case, but you could be right.
If not, it would be useful to have it imho because needs internal knowledge.
Let's try and see if I am able to build a 3rd party app. Thanks
comment:9 by , 6 months ago
We ran into the same issue. We tried to switch the responsible user of a task depending on some simple business logic that could be evaluated by the GeneratedField.
The ForeignObject approach is working for us.
For anyone stumbling on this discussion later on, we observed a caching issue with ForeignObject. The cache of the field doesn't get refreshed when refresh_from_db
gets called.
We had to explicitly override refresh_from_db
on the Task model
def refresh_from_db(self, using=None, fields=None): super(Task, self).refresh_from_db(using=using, fields=fields) responsible_user_field = self._meta.get_field("responsible_user") if responsible_user_field.is_cached(self): responsible_user_field.delete_cached_value(self)
Related fields were never intended to be used as
output_field
s. Moreover, as far as I'm aware,GeneratedField
s cannot refer fields from external models, so I'm not exactly sure what kind of column you want to add. Please add a sample project that reproduces your issue, and provide a valid column DDL definition that you believe is not currently supported.