Support multi_match query type

jurismarches / luqum

A lucene query parser generating ElasticSearch queries and more !

Other

187 stars 42 forks source link

Support multi_match query type #41

Closed emptyflash closed 5 years ago

emptyflash commented 5 years ago

This solves a problem I had trying to use multi_match queries, figured I'd see if it made sense to merge it up stream.

It enables setting up a field to be converted to a multi_match query in order to automatically query its subfields as well. E.g.:

es_query_builder = ElasticsearchQueryBuilder(
    field_options={
        'title': {
            'match_type': 'multi_match',
            'type': 'bool_prefix',
            'fields': ['title', 'title.ngram', 'title.raw']
        }
    }
)

muli_match has a property called type so I needed to rename the special field_option type case to match_type. If match_type isn't used it falls back to type for backward compatibility.

alexgarel commented 5 years ago

Hi @emptyflash thanks for the PR.

Right now I'm on vacation but I will try to look into this next week.

On a quick glance your modifications looks ok to me.

Tests are failing because of pep8 / flake8 (make quality to see problems) Could you fix it ?

coveralls commented 5 years ago

Coverage remained the same at 100.0% when pulling 9c4630d850cf70ece1a02ffb64b818c28b9552d2 on emptyflash:master into 1d1b9eb68c4a4155bd8d5490dd3d0ca76a30aa23 on jurismarches:master.

emptyflash commented 5 years ago

Definitely no rush, I'm also on vacation starting today :sunglasses:

emptyflash commented 5 years ago

Hey @alexgarel any update on this? I think this would be a minor version bump, so I went ahead and changed that in luqum/__init__.py

alexgarel commented 5 years ago

Sorry @emptyflash, I just let time goes !

The code is all good.

May I ask you a çtest for the use case you have (using multi_match) ? It will prevent breaking it later.

Also you may update the doc to propose "match_type" rather than "type", even if we keep backward compatibility.

emptyflash commented 5 years ago

Sorry for the delay @alexgarel, I updated the docs and added a test case. Let me know if there's anything I missed, or any more test cases I can add.