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

select_related, out of scope? #18

Open
SafaAlfulaij opened this issue May 16, 2021 · 3 comments
Open

select_related, out of scope? #18

SafaAlfulaij opened this issue May 16, 2021 · 3 comments

Comments

@SafaAlfulaij
Copy link
Contributor

I was wondering what features will be considered out of scope and too much for this library, and what features isn't.
For example, in the test suite, how can we query ReferenceModel and select the tree model with it's tree fields?

@matthiask
Copy link
Member

It's hard to say. It depends on how complex it will be. I really want to keep the API and the implementation small and simple. If fear that adding the CTE to a query where the tree model isn't the base model of the queryset will complicate things, maybe a lot and introduce tricky edge cases which will have to be fixed when new Django versions are released. The beauty of django-tree-queries is that it mostly works (or worked) the same for Django 1.8 up to 3.2.

(The ReferenceModel case could also be handled by querying the tree model and prefetching the reference model instances, maybe using a Prefetch() instance. I know that this is just an example – I'm writing this down just as an illustration of my thinking in this case.)

@SafaAlfulaij
Copy link
Contributor Author

It's hard to say. It depends on how complex it will be. I really want to keep the API and the implementation small and simple. If fear that adding the CTE to a query where the tree model isn't the base model of the queryset will complicate things, maybe a lot and introduce tricky edge cases which will have to be fixed when new Django versions are released. The beauty of django-tree-queries is that it mostly works (or worked) the same for Django 1.8 up to 3.2.

I tried it, and most of the new code is copied from TreeCompiler.as_sql, with modified extra (to not sort or filter), and a custom join to the CTE table.
Still, it is complicating stuff, and can break if Django update their internals, and doesn't support nested relations, and will brake with some edge cases.
I'll continue checking and see how it fits.

(The ReferenceModel case could also be handled by querying the tree model and prefetching the reference model instances, maybe using a Prefetch() instance. I know that this is just an example – I'm writing this down just as an illustration of my thinking in this case.)

This actually might be the proper solution (although it does another query), since it adds the fields to the actual tree model, rather than adding them to the reference-by model (how annotate works generally).

@matthiask
Copy link
Member

We have had a few issues with subqueries, annotations etc. not working properly in the last three years; not many, but too many to think that something like supporting select_related would be simple enough to be achievable without complicating everything too much.

So, I think that this feature request is out of scope, and we should instead add documentation on how to work within the current constraints.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants