DSL: Allow to merge Searches

[tmaier]

This would allow to split long and ugly methods into smaller parts...

[karmi]

@tmaier, can you please sketch out an example of what you're after in (pseudo-)code?

[tmaier]

A search query gets long and complex.
I have the query itself which might have some conditions when which field will be searched or when to use "AND" or "PHRASE" in a query.
I might also enable or disable some ore all aggregates. Or just return the aggregates.
Enable or disable highlighting etc.

The current design would expect me to do this all within one method and one very long block.

A normal search method looks like this

      def self.search(q, filters = {}, highlight: true, aggregations: true, fulltext: false)
        qry = Elasticsearch::DSL::Search::Query.new do
          # 250 lines of blocks and many if statements
        end
      end

My current design allows already certain encapsulation and "mix-and-match":

      def self.search(q, filters = {}, highlight: true, aggregations: true, fulltext: false)
        qry = Elasticsearch::DSL::Search::Query.new do
          # ...
        end
        qry_filtered = Elasticsearch::DSL::Search::Query.new do
          filtered do
            query qry
            filter do
              # ...
            end
          end
        end if filters.present?
        qry_highlight = Elasticsearch::DSL::Search::Highlight.new do
          # ...
        end if highlight && q.present?
        qry_aggregations = Elasticsearch::DSL::Search.search do
          aggregation :authors do
            # ...
          end
          aggregation :editors do
            # ...
          end
        end.aggregations if aggregations

        search_definition = Elasticsearch::DSL::Search.search
        search_definition.query(qry_filtered || qry) # Use either the filtered query or the query without filter
        search_definition.highlight(qry_highlight) if qry_highlight
        # I had to cheat a bit to be able to define multiple aggregations and assign them to the Search
        search_definition.aggregations = qry_aggregations if qry_aggregations
        # There is not setter for #sort
        search_definition.sort do
          by :title_untouched, order: 'asc'
        end unless q.present?
        search_definition.source false

        __elasticsearch__.search(search_definition)
      end

What I would like:

qry_aggregations = Elasticsearch::DSL::Search.search { ... } 
search_definition = Elasticsearch::DSL::Search.search { ... }
search_definition.merge!(qry_aggregations)

Then I would also start to move this all in a dedicated PORO so that I can get back and have only a low number of LOC per method (https://github.com/bbatsov/ruby-style-guide#short-methods)

def self.search(query, *features)
  __elasticsearch__.search(MySearch.new(query, *features).definition)
end

[karmi]

Thanks for the clarification and the code example!

While I do think that we could improve "reaching inside" the search definitions to allow easier decomposition (the Python DSL library does a better job here, for instance), I can't dedicate much time for fleshing it out, currently. However, I think that what you are after can be achieved in two ways.

The first option would be to continue on the path you've set out, which is a good approach in my opinion. In fact, you can create an empty Elasticsearch::DSL::Search::Search, and then continually amend the definition based on conditions -- this doesn't have to be in one method. I was playing with something like this:

s = Elasticsearch::DSL::Search::Search.new

@query   = 'foo'
@filters = ['one']
@highlight = true

def add_filters(s)
  if @filters.any?
    q = s.query { |q| q.filtered { |q| q.filter { |q| q.terms tags: @filters }; q.query { |q|  q.match title: @query } } }
  else
    q = s.query { |q| q.match title: @query }
  end
end

def add_highlight(s)
  s.highlight { fields ['title'] } if @highlight
end

add_filters(s)
add_highlight(s)

s.to_hash
# => {:query=>{:filtered=>{:query=>{:match=>{:title=>"foo"}}, :filter=>{:terms=>{:tags=>["one"]}}}}, :highlight=>{:fields=>{:title=>{}}}}

Notice, that the most pressing need for manipulating the search definition right now is the filtered query, which will be removed in the next ES version, and replaced by a filter clause in the bool query.

It is in fact possible to achieve the merging of multiple different parts of the search definition by using their Hash representation:

s1 = Elasticsearch::DSL::Search.search { query { match title: 'foo' } }
s2 = Elasticsearch::DSL::Search.search { aggregation(:tags) { terms field: 'tags' } }
s1.to_hash
# => {:query=>{:match=>{:title=>"foo"}}}
s2.to_hash
# => {:aggregations=>{:tags=>{:terms=>{:field=>"tags"}}}}
s1.to_hash.merge(s2.to_hash)
# => {:query=>{:match=>{:title=>"foo"}}, :aggregations=>{:tags=>{:terms=>{:field=>"tags"}}}}

Of course, this removes any possibility of doing something else with the parts, but it might be enough for your purposes. What do you think?

[karmi]

Hi, has the previous answer been any help?

[tmaier]

HI @karmi, thanks for your answer. I found now time to look into that again.

I really like that filtered is deprecated for filter.

In general I could live with the hash merging way. I just think that this would require a deep merge method for the hash. This would then allow to define in one s1 the search query and in s2 the filter query and merge them afterwards.