spring-data-elasticsearch icon indicating copy to clipboard operation
spring-data-elasticsearch copied to clipboard

Add support for named parameters in string queries

Open diamondT opened this issue 3 years ago • 2 comments

hi,

it would be nice to have support for named parameters in @Query queries, similarly to e.g. spring-data-jpa:

@Query("{\"match\": {\"name\": {\"query\": \":name\"}}}")
Page<Book> findByName(String name,Pageable pageable);

instead of

@Query("{\"match\": {\"name\": {\"query\": \"?0\"}}}")
Page<Book> findByName(String name,Pageable pageable);

looking for patterns with : would be tricky because of the ES query syntax so we could make that behavior explicit instead; let the user define if a query is using positional or named parameters with a new flag.

created a WIP here: https://github.com/diamondT/spring-data-elasticsearch/commit/cd3d2cc940975f692535433fca86d8a939ef075a if this is something worth pursuing i can polish a bit more and send a PR.

thanks,

diamondT avatar Jun 05 '22 17:06 diamondT

ping @mp911de: do we have parameter resolution by name in other Spring Data modules? Do we have the names of the parameters at parameter resolving time?

sothawo avatar Jun 14 '22 20:06 sothawo

Yes, there's access to named parameters via o.s.d.r.q.Parameter.getName() (isNamedParameter) and Parameters. We generally reference named parameters via :<the name>. In several modules we resolve the parameter by looking it up via name and then using the parameter index to access the actual value from ParameterAccessor.getBindableValue(int).

mp911de avatar Jun 15 '22 05:06 mp911de

With the implementation of #2083 it is now possible to use a named query like this:

@Query("""
        {
            "match": {
            "text": {
                "query": "#{#foo}"
                }
            }
        }
        """)
SearchHits<Foo>searchByFoo(String foo);

sothawo avatar Feb 28 '24 18:02 sothawo

The implementation from the PR is a little complicated. SpelQueryContext is capable of finding and replacing query parameters (or its successor https://github.com/spring-projects/spring-data-commons/pull/3050). Over time, we introduced a JSON parser to MongoDB to resolve query expressions eagerly. Also, "\"" + targetElement + "\"" asks for injection troubles.

mp911de avatar Feb 29 '24 08:02 mp911de

Will have a look at the injection issue.

As for the SpEL stuff: It's hard to know what's available in spring-data-commons without following every PR/change, I didn't find anything about the ValueExpression or SpelQueryContext in the docs (https://docs.spring.io/spring-data/data-commons/docs/current/reference/html/) is there some other source?

Looking at that merge(https://github.com/spring-projects/spring-data-commons/pull/3050), if I understand it correctly, ValueExpression converts the query to replace the spel parts by binding parameters that need to be supported by the store? This is something Elasticsearch does not have, we need to replace the spel parts in the query with the verbatim values from the parameters.

sothawo avatar Feb 29 '24 17:02 sothawo

I looked into the SpelQueryContext today, found it may not be suitable for elasticsearch query. (Or maybe I misunderstood something in SpelQueryContext?)

With SpelQueryContext the json query should be written as:

@Query("""
        {
            "match": {
            "text": {
                "query": :#{#foo}
                }
            }
        }
        """)
SearchHits<Foo>searchByFoo(String foo);

Indeed the :#{#foo} or ?#{#foo} part in elasticsearch query should be quoted with ", thus the actual value for #foo should be escaped. When it comes to collection, things get more complicated(prepend/append with [/] and consider the escape too). That's the reason SpEL expression evaluation looks like this.

If I'm wrong, please correct me, thanks in advanced.

PLUS: the official docs for spring-data-commons are indeed imperfect, and when searching on the web, most of the information is about the subprojects of spring-data-commons, which makes learning spring-data-commons difficult too.

puppylpg avatar Feb 29 '24 18:02 puppylpg