Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not include annotation fields when selects specific fields #634

Merged

Conversation

devxoul
Copy link
Contributor

@devxoul devxoul commented Apr 29, 2022

After #633 got merged, I found a non-standard behavior. By default, with using values() or values_list() with specific fields, the queryset should not include annotated fields.

  • queryset.annotate(foo=bar).values(): {'id': 10, 'foo': 'bar'} # should include annotated fields
  • queryset.annotate(foo=bar).values('id'): {'id': 10} # should NOT include annotated fields

This PR ensures the Django's default behavior of values() and values_list() with fields.

@devxoul devxoul force-pushed the devxoul/values-select-specific branch from a4e2f36 to ea8375e Compare April 29, 2022 23:04
@devxoul devxoul changed the title Do not include annotation fields when selects specific fields [WIP] Do not include annotation fields when selects specific fields Apr 29, 2022
@devxoul devxoul force-pushed the devxoul/values-select-specific branch from f91012f to ea8375e Compare April 29, 2022 23:17
@devxoul devxoul force-pushed the devxoul/values-select-specific branch from ea8375e to ce2c0a4 Compare April 30, 2022 00:21
@devxoul devxoul changed the title [WIP] Do not include annotation fields when selects specific fields Do not include annotation fields when selects specific fields Apr 30, 2022
@@ -414,8 +414,9 @@ def raw_values(self, *fields):
def _values(self, *original, **kwargs):
if not kwargs.pop('prepare', False):
return super(MultilingualQuerySet, self)._values(*original, **kwargs)
selects_all = kwargs.pop('selects_all', False)
Copy link
Collaborator

Choose a reason for hiding this comment

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

i think selects_all should be defined explicitly in function definition, becouse otherwise it could be handed over to super()

Copy link
Collaborator

Choose a reason for hiding this comment

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

or, at least, this line should be above 415, if we want to match original definition.

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.

2 participants