Skip to content

Fix bug: rename annotation_mask #61

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

Merged
merged 1 commit into from
Jul 21, 2018

Conversation

knqyf263
Copy link
Contributor

Abstract

When the name of an existing field on the model is used as the alias name, annotate function renames the fields.

# temporarily rename the fields that have the same
# name as a field name, we'll rename them back after
# the function in the base class ran
new_annotations = {}
renames = {}
for name, value in annotations.items():
if name in fields:
new_name = '%s_new' % name
new_annotations[new_name] = value
renames[new_name] = name
else:
new_annotations[name] = value

After that, rename_annotations renames the fields to the original name again.

for old_name, new_name in annotations.items():
annotation = self.annotations.get(old_name)
if not annotation:
raise SuspiciousOperation((
'Cannot rename annotation "{old_name}" to "{new_name}", because there'
' is no annotation named "{old_name}".'
).format(old_name=old_name, new_name=new_name))
del self.annotations[old_name]
self.annotations[new_name] = annotation

However, annotation_select_mask is not changed.
As a result, the renamed fields are lost because _annotation_select_cache is generated by annotation_select_mask.
https://github.com/django/django/blob/952f05a6db2665d83c04075119285f2164b03432/django/db/models/sql/query.py#L2022-L2027

In addition, though self.annotations is OrderedDict, order is destroyed in the following code.

del self.annotations[old_name]
self.annotations[new_name] = annotation

Sample

https://github.com/knqyf263/psqlextra_fix

In test_annotation_bug, description is not selected correctly.
https://github.com/knqyf263/psqlextra_fix/blob/master/users/tests.py#L10-L14

In test_union_exception, ValueError exception is thrown.
This is because the number of fields does not match the number of converters due to annotation_select_mask bug.
https://github.com/knqyf263/psqlextra_fix/blob/master/users/tests.py#L16-L23

Fix

  • Chang to keep order of self.annotations.
  • Also rename self.annotation_select_mask.

I'm not sure if this fix is correct, please review me!

@Photonios
Copy link
Member

Awesome! Thanks a lot 👍

@Photonios Photonios merged commit 6836467 into SectorLabs:master Jul 21, 2018
@knqyf263
Copy link
Contributor Author

Thanks! I’m looking forward to release new version!

@knqyf263 knqyf263 deleted the fix/annotation_bug branch July 21, 2018 12:45
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