elasticsearch-dsl-rs icon indicating copy to clipboard operation
elasticsearch-dsl-rs copied to clipboard

Add support for span queries

Open jenrik opened this issue 3 years ago • 3 comments
trafficstars

Adds support for Elasticsearch's span queries, https://www.elastic.co/guide/en/elasticsearch/reference/current/span-queries.html

This PR is still a work-in-progress.

Closes #100

jenrik avatar Jun 15 '22 20:06 jenrik

First of all, thank you for contributing! You're really on a good track, I've left some comments and suggestions.

buinauskas avatar Jun 27 '22 08:06 buinauskas

Things look pretty sharp, I believe you got the logic nailed, so these are the common changes across all of the queries:

  • Make sure to implement ShouldSkip correctly for queries that contain a collection of subqueries, e.g. SpanNotQuery, it's better to skip them
  • Default to None for optional parameters, rely on setters. This gives a more pleasant DSL to use without requiring to specify a lot of default params
  • Prefer to accept traits that query can be implemented From<_> something, this allows to have a more concise syntax and experience

I'm overall really happy with this contribution, there might be things that will need some slight tweaking, but they're not obvious right now. I'll approve this once you're confident with the changes.

buinauskas avatar Jul 01 '22 11:07 buinauskas

FYI this will impact your changes https://github.com/vinted/elasticsearch-dsl-rs/pull/160 slightly.

buinauskas avatar Jul 13 '22 09:07 buinauskas

I figured this is already obsolete and no longer in progress, so I've taken your work and implemented span queries myself in https://github.com/vinted/elasticsearch-dsl-rs/pull/207.

Thank you 🙇🏻

buinauskas avatar Dec 03 '22 16:12 buinauskas