Skip to content

Conversation

egegunes
Copy link
Contributor

@egegunes egegunes commented May 8, 2019

This fixes #714

@coveralls
Copy link

Pull Request Test Coverage Report for Build 1101

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 94.565%

Totals Coverage Status
Change from base Build 1095: 0.0%
Covered Lines: 1218
Relevant Lines: 1288

💛 - Coveralls

Copy link
Contributor

@auvipy auvipy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And it would be nice to add test for the proposed change/

"scope": scope,
"expires": expires,
})
access_token, _created = AccessToken.objects.update_or_create(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why select_related is removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update_or_create uses select_for_update in the background if model exists. It's not supported to use select_for_update with nullable relations as mentioned in Django docs.

So, I could change the line to this:

access_token, _created = AccessToken.objects.select_related().update_or_create(...)

But calling select_related with no arguments only follows non nullable relations, nullable relations must be specified (see docs). Since user and application are nullable relations, there is no point to call select_related with no arguments, and we can't specify this fields because of update_or_create.

@egegunes egegunes changed the title Fix not supported FOR UPDATE query for Postgresql, fixes #714 Fix not supported FOR UPDATE query, fixes #714 May 9, 2019
@auvipy auvipy merged commit 846ab0b into django-oauth:master May 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FOR UPDATE cannot be applied to the nullable side of an outer join
3 participants