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

Custom query fields #3

Open
holvianssi opened this issue Apr 11, 2017 · 5 comments
Open

Custom query fields #3

holvianssi opened this issue Apr 11, 2017 · 5 comments

Comments

@holvianssi
Copy link

First, I just tried this library, and this is just amazing. Really nice work!

I've been thinking that it would be nice to have some form of custom fields available. For example, you might want to filter on user's full name when you have first_name and last_name in the model, or you might want to filter on some aggregate (say, num_books >= 10).

It seems this would require changes to both the schema and build_filter parts. I'll probably write a quick hack for this so that it's possible to check what exactly this requires.

@stebunovd
Copy link
Member

I got similar feature requests from other people as well, and I personally think this could be a powerful feature if implemented properly. I've looked through your PR, and the idea of extracting DjangoQLField seems to be promising. I'd like to play with your version a little bit, maybe try some variants for the API.

thank you for contributing!

@holvianssi
Copy link
Author

holvianssi commented Apr 12, 2017

The DjangoQLField approach is very likely a good solution on high level. Moving a bit more logic to DjangoQLField from schema would likely make it even more powerful. As an example, it would be nice that you could use the display value of a field's choices in the search string, and then convert that to the internal representation for querying. This should be doable by an ui_to_internal(value) method of DjangoQLField.

Now, the actual implementation will probably need a bit more tuning. I believe it would be better to have something like DjangoQLField, and then Int, String, Boolean, ... fields subclassing the DjangoQLField. Now, things like schema.validate() could be implemented as a high level validator in schema.validate(), and then field type specific validation would be done by calling field.validate().

@stebunovd
Copy link
Member

alright, version 0.8.0 is out, with Custom Search Fields

This is based on many of your ideas and your PR, I just explored the concept further and provided ability for users to perform fully custom lookups. From my perspective it covers majority of use cases that users may want, but that would be great to hear your thoughts too. I'm closing this for now, but if you think that the problem is not resolved well enough - please feel free to re-open it.

thanks again for contributing!

@holvianssi
Copy link
Author

Thanks, that was fast!

I have one idea for this still... add a method of something like Field.participate_in_query(self, queryset, path, operator, value). The method returns a queryset, and shouldn't call filter() or exclude() on the queryset.

The idea would be that this method is called only if the field is used as part of the query. A common use case would be doing an annotation to the query. It would be better to do the annotation only when the field is used in the query, instead of doing it always in admin's get_queryset().

There are some advantages doing it this way. First, if you have multiple fields requiring annotations, then the queryset can become quite heavy even if you don't need all of the annotations. Still, this would allow one to use djangoql searches on any queryset, without the need to add annotations to the query manually. Finally, if you use the new Exists expression from Django 1.11, and the user's query is something like commented_since_days=3, your implementation could look like this:

n_days_ago = timezone.now() - timedelta(days=3)
recent_comments = Comment.objects.filter(
    post=OuterRef('pk'),
    created_at__gte=n_days_ago,
)
qs = qs.annotate(commented_since_days=Exists(recent_comments))
qs.filter(commented_since_days=True)

doing this without access to the queryset is impossible, as the annotation is dynamic. With the proposed addition it should be possible to do this type of query.

@stebunovd
Copy link
Member

Good catch about avoiding unnecessary annotations when a field is not used in the query. Let's re-open this

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

No branches or pull requests

2 participants