Skip to content
This repository has been archived by the owner on Feb 7, 2019. It is now read-only.

Going through VersionedForeignKey for filter() has strange edge cases with as_of() #45

Closed
arild-haugstad opened this issue Jan 16, 2015 · 2 comments

Comments

@arild-haugstad
Copy link

This test should illustrate the issue:

class FilterOnRelationTest(TestCase):
    def test_filter_on_relation(self):
        team = Team.objects.create(name='team')
        player = Player.objects.create(name='player', team=team)
        t1 = get_utc_now()
        sleep(0.1)
        l1 = len(Player.objects.as_of(t1).filter(team__name='team'))
        team.clone()
        l2 = len(Player.objects.as_of(t1).filter(team__name='team'))
        self.assertEqual(l1, l2)

For the first Player.objects.as_of()-query, we get the player object; for the second, we get no results.

My understanding is that VersionedForeignKey.get_extra_restriction() adds WHERE-clauses as appropriate wrt. version_start_date and version_end_date for the active querytime. (Or for the "current" version if there is no active querytime.)
With the JOIN constructed by Django, we get a join on primary key, which would match only the newest version, and if the newest version is not the appropriate one for the querytime the WHERE-clause discards it, and we get no result where one was expected.

Unfortunately, I don't have any good solution to this; modifying logic that sets up joins so that as_of()-queries might join on identity instead of id (as the existing get_extra_restriction() should ensure that we get only one) where appropriate, without breaking table creation, without breaking the currently working queries through versioned many-to-many relations, would seem to require overriding a lot of methods on Query with only minor changes... That might work, though it sounds rather messy... Hopefully, you can come up with a better idea?

@maennel
Copy link
Contributor

maennel commented Jan 27, 2015

Hi @arild-haugstad,

thanks for your contribution. This is just to inform you that I've taken up your test case and I'm currently working on a solution to this issue. As you've mentioned, there are quite a couple of potential conflicts with existing tests.

I'll keep you updated as soon as I've got something usable.
-Manuel

@maennel
Copy link
Contributor

maennel commented Jan 29, 2015

@arild-haugstad I've written a fix in PR #50 for the problem you reported. I hope this enables you in proceeding to use CleanerVersion!
Thanks for contributing,
-Manuel

@maennel maennel closed this as completed Jan 29, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants