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

AnyOfFilter shouldn't impose distinct by default unless joining on another table #61

Conversation

rodrigoalmeidaee
Copy link
Contributor

Context

Currently, AnyOfFilter, since it inherits from MultipleChoiceFilter, sets distinct=True by default on queries. This is for a good reason: when filtering on a field that may traverse relationships, rows may get duplicated as result.

Issues

More often that not, AnyOfFilter is used to filter on the table own fields, in which case distinct is not necessary as rows won't duplicate by definition (they all have different primary keys 😅). It is unclear whether PostgreSQL can figure the same thing out on its own (I've done a simple test locally against a table and it seems it doesn't, at least for the case I tested). In case it can't, there are possible performance gains of not using distinct when filtering own table fields.

Solution

distinct becomes a three-valued attribute for AnyOfFilter:

  • True: uses distinct()
  • False: doesn't use distinct()
  • None (the default): infer based on the field_name: if it contains a navigation (identified by the presence of a __), use distinct.

Note: lookups are also double underscored and will become "false positives" (triggering distinct when not needed), but this matches the current behavior (which triggers distinct all the time) and is not a regression.

@rodrigoalmeidaee rodrigoalmeidaee changed the title AnyOfFilter shouldn't impose distinct by default unless joining on another tableFeature/any of without distinct AnyOfFilter shouldn't impose distinct by default unless joining on another table Oct 17, 2024
@rodrigoalmeidaee rodrigoalmeidaee merged commit 13842e9 into flamingo-run:main Oct 18, 2024
7 of 9 checks passed
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