search

jillesvangurp / kt-search

Multi platform kotlin client for Elasticsearch & Opensearch with easily extendable Kotlin DSLs for queries, mappings, bulk, and more.
MIT License
107 stars 23 forks source link

[BUG] Multi-word configuration properties for match queries don't work #26

Closed yassenb closed 1 year ago

yassenb commented 1 year ago

Describe the bug

Setting MatchQueryConfig.maxExpansions doesn't work

To Reproduce

Steps to reproduce the behavior: Configure a match query with maxExpansions = 100 (any number works) and execute the query. An error "[match] query does not support [maxExpansions]" is returned because the client passes "maxExpansions": 100 as part of the JSON sent to the OpenSearch server

Expected behavior

"max_expansions": 100 should be passed to the OpenSearch server

Your context

latest version 1.99.14 on Ubuntu 22.04

Will you be able to help with a pull request?

Sure, I can create a PR

Overall I think the default for the JsonDsl constructor should be namingConvention = PropertyNamingConvention.ConvertToSnakeCase. That's the naming convention for OpenSearch after all. Naming things camelCase should be the exception, not the rule

jillesvangurp commented 1 year ago

right, I'll have a look at that one. Thanks for reporting

jillesvangurp commented 1 year ago

Wow, this was broken in a lot of places. I changed the naming convention to default to asIs at some point without realizing I had to fix this all over the DSL. Fixed by #28

yassenb commented 1 year ago

Thanks but why was the default switched to "asIs"? Look at the many places where you need to override it and potential future extensions will have to be careful to override as well. Why not default to snake case when this is the default for OpenSearch/ElasticSearch?

jillesvangurp commented 1 year ago

I agree, this is the better fix. I changed this because I see uses for JsonDsl beyond the search client. But it is a bit error prone to deal with so I'm fixing this properly with a separate pull request.