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

Added draft DEP for external query languages support by Django ORM #40

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Nepherhotep
Copy link

Hey @jarshwah @akaariai

Here is a DEP describing the possible API for third-party query language support by Django ORM. If you like the idea it would be great if we can arrange phone/skype call to discuss some technical details, as the proposal touches code which I don't fully understand - basically it's stuff related to joins.

Best regards,
Alexey

@jarshwah
Copy link
Member

jarshwah commented Apr 2, 2017

Hi @Nepherhotep

Have you thought about ExpressionBuilders being Resolvable rather than a separate build_expression method? Since Resolvable isn't really explained in the docs, it is simply a class that has a resolve_expression method that takes the arguments query=None, allow_joins=True, reuse=None, summarize=False, for_save=False and returns an Expression. The F class is a Resolvable, but isn't an expression itself.

Left right here - Django would not actually need to change any code at all. Since the library would have access to the current query object, it'd know the entire state of the query, have access to the model, and the Meta class of that model.

If above serves our purposes, then I think it'd be worth looking more closely into build_filter, and attempting to break it down into smaller helper methods that would allow a query library to extract the data it needs, and to push through the required joins. As you've written, Django is already doing the heavy lifting.

The ExpressionBuilder library would then maintain state internally of the property accesses and method calls, keeping an AST of sorts internally. For each property it'd be able to call query.model.get_field(field_name) for validation, and convert that into an F object (or a Col/Ref directly, skipping a further resolve step). Method calls it would do a lookup in the transforms dict. Finally, the last access would be represented by the operators, and would map to a lookup in the lookups dict.

A different library could also just convert the internal representation into a kwarg, and let django handle the resolving of each part - incurring more overhead but would seemingly be quicker to iterate on and get feedback for.

@schinckel
Copy link

I wonder if this might make the stuff @jarshwah and I have discussed before about allowing, for example, an Exists subquery to be used as a filter directly, instead of the annotate, filter dance we currently have to do.

@Nepherhotep
Copy link
Author

I believe Resolvable will work, the critical thing here - ExpressionBuilder should not be of the whole Expression interface, as that class is quite complicated and it would be more difficult to extended it for third-party libraries (due to possible method conflicts, for example).
As to transforms/lookups dicts, I'm not sure I fully understand. Once we manually create transforms/lookups hierarchy by QL, do we need to lookup them somewhere in query internal state at all? Assuming QL libraries will not encode transforms/lookups into field paths, I believe we don't.
Also Django API should encapsulate join functionality inside and not delegate this job to third-party libraries.

@coady
Copy link

coady commented Mar 10, 2018

You might be interested in django-model-values' F expressions, as a reference implementation of essentially the same idea. The only difference is I choose to just subclass F, so the intermediate objects are themselves F expressions.

@jarshwah
Copy link
Member

Cool library @coady! I'll definitely be taking a closer look.

@auvipy
Copy link

auvipy commented Nov 12, 2019

A mix of both approaches might be great

@auvipy
Copy link

auvipy commented Dec 19, 2019

Hi @Nepherhotep

Have you thought about ExpressionBuilders being Resolvable rather than a separate build_expression method? Since Resolvable isn't really explained in the docs, it is simply a class that has a resolve_expression method that takes the arguments query=None, allow_joins=True, reuse=None, summarize=False, for_save=False and returns an Expression. The F class is a Resolvable, but isn't an expression itself.

Left right here - Django would not actually need to change any code at all. Since the library would have access to the current query object, it'd know the entire state of the query, have access to the model, and the Meta class of that model.

If above serves our purposes, then I think it'd be worth looking more closely into build_filter, and attempting to break it down into smaller helper methods that would allow a query library to extract the data it needs, and to push through the required joins. As you've written, Django is already doing the heavy lifting.

The ExpressionBuilder library would then maintain state internally of the property accesses and method calls, keeping an AST of sorts internally. For each property it'd be able to call query.model.get_field(field_name) for validation, and convert that into an F object (or a Col/Ref directly, skipping a further resolve step). Method calls it would do a lookup in the transforms dict. Finally, the last access would be represented by the operators, and would map to a lookup in the lookups dict.

A different library could also just convert the internal representation into a kwarg, and let django handle the resolving of each part - incurring more overhead but would seemingly be quicker to iterate on and get feedback for.

I want to give this approach a shot you provide a green signal.

@auvipy
Copy link

auvipy commented Jan 23, 2021

we may also take https://github.com/thedrow/django-natural-query into consideration

Base automatically changed from master to main February 25, 2021 16:04
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.

5 participants