Filter on fk rels #50
Conversation
…M2M-relationships in order to be able to make the difference between the two of them. This enables us to JOIN on the identity field instead of the id field in case of a VersionedForeignKey
… time; extended unittest documentation
| @@ -557,8 +584,8 @@ def create_versioned_many_to_many_intermediary_model(field, cls, field_name): | |||
| return type(str(name), (Versionable,), { | |||
| 'Meta': meta, | |||
| '__module__': cls.__module__, | |||
| from_: VersionedForeignKey(cls, related_name='%s+' % name), | |||
| to_field_name: VersionedForeignKey(to, related_name='%s+' % name), | |||
| from_: VersionedForeignKey(cls, related_name='%s+' % name, auto_created=name), | |||
There was a problem hiding this comment.
Is this abusing the auto_created property? I'm not really familiar with auto_created. I'm just concerned if it will cause problems in some situations if it's not being used as Django expects; what comes to mind is migrations.
There was a problem hiding this comment.
I'm not entirely sure whether it's ABusing the auto_created property (which implies, that I'm not super familiar with the auto_created property).
Resources that I've found are:
- https://docs.djangoproject.com/en/1.7/howto/custom-model-fields/#writing-a-field-subclass (which says: auto_created: True if the field was automatically created, as for the OneToOneField used by model inheritance. For advanced use only.)
- http://django-model-internals-reference.readthedocs.org/en/latest/auto_created.html (which would probably be a good resource, but which is still tagged as a TODO and empty otherwise)
Depending on what 'created' means, our use case may not be that wrong.
Also considering Django sets the auto_created flag on intermediary models in M2M relationships, why not setting it on its ForeignKeys? (It's true that I'm a bit worried that Django doesn't set that flag on ForeignKeys being part of auto_created intermediary models.)
However, side effects would need to be stress tested. Migrations are a good keyword.
There was a problem hiding this comment.
Looking through Django's source code, I think it's safe. And the only other alternative that I see would be to add another property to the VersionedForeignKey class (e.g. is_m2m_field).
I'm surprised that we can't tell that it's an intermediary table somehow.
Added TestCase specified in issue #45 and bugfixed the code.
Now, ForeignKey relations get joined on the
identityproperty of objects, such that time-constraint keep their effect of selecting a version of an object.Also fixed a bug which had as an effect that time-constraints got added multiple time to a queryset (in fact, everytime
get_compiler()on aVersionedQuerySetgot called. Even if doing this doesn't invalidate the produced SQL, it gets hardly readable over time.Finally, improved some comments on unit tests, in order to be able to recognize the absolute state of the system at any point in time t0-tn.