Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

{{#toLowercase}} mustache function for search templates #60566

Closed
JohT opened this issue Aug 2, 2020 · 4 comments
Closed

{{#toLowercase}} mustache function for search templates #60566

JohT opened this issue Aug 2, 2020 · 4 comments
Labels
:Core/Infra/Scripting Scripting abstractions, Painless, and Mustache >enhancement Team:Core/Infra Meta label for core/infra team team-discuss

Comments

@JohT
Copy link

JohT commented Aug 2, 2020

Outline:
As described in this search template reference example there is a mustache function to convert a string to JSON: {{#toJson}}statuses{{/toJson}}
It would be great to have another mustache function to convert a parameter value string to lowercase:
{{#toLowercase}}genre_prefix{{/toLowercase}}

I'd like to provide a pull request for this feature when discussion leads to an approve for implementation.

Possibly related issues:
Could be an alternate approach for #28894.
Could also be related to #53603.

Use-case:
The Terms Aggregations Filter property "include" supports (among others) regexp queries. These queries are case-sensitive.
A commonly known solution to get case-insensitive results is a "lowercase" Normalizer. The only thing left to do is to use lowercase characters in the regex. I haven't found a solution to do this inside elasticsearch though.

A great way to hide the complexity of some queries and make them exchangeable are Search Templates. Since they come with variables and functions, "toLowercase" could be integrated to get parametrized case-insensitive term aggregations (among others) build into an easy to use search template.

Advantages:

  • Should be easy to realize (and to test), probably in CustomMustacheFactory
  • May provide a workaround for some other issues regarding case-sensitive properties like "wildcard", "include", "exclude",...
  • No speed change for those already using Search Templates to decouple query complexity from code.
  • No index change for those who already use a lowercase normalizer.

Steps to reproduce:

DELETE test

PUT test
{
    "settings": {
        "analysis": {
            "normalizer": {
                "lowercase_normalizer": {
                    "type": "custom",
                    "filter": [
                        "lowercase"
                    ]
                }
            }
        }
    },
    "mappings": {
        "properties": {
            "genre": {
                "type": "keyword",
                "normalizer": "lowercase_normalizer"
            }
        }
    }
}

PUT test/_doc/1?refresh
{
  "genre": "Rock"
}

// stores the search template that queries all genres starting with the prefix given as parameter 
POST _scripts/count_per_genre_v1
{
    "script": {
        "lang": "mustache",
        "source": {
            "size": 0,
            "aggs": {
                "genres": {
                    "terms": {
                        "field": "genre",
                        "include": "{{genre_prefix}}.*"
                    }
                }
            }
        }
    }
}

// no results because of mixed-case parameter
GET test/_search/template
{
    "id": "count_per_genre_v1",
    "params": {
        "genre_prefix": "Ro"
    }
}

// results as expected because of manually up-front lowercased parameter
GET test/_search/template
{
    "id": "count_per_genre_v1",
    "params": {
        "genre_prefix": "ro"
    }
}

Proposed solution example:

POST _scripts/count_per_genre_v2
{
    "script": {
        "lang": "mustache",
        "source": {
            "size": 0,
            "aggs": {
                "genres": {
                    "terms": {
                        "field": "genre",
                        "include": "{{#toLowercase}}genre_prefix{{/toLowercase}}.*"
                    }
                }
            }
        }
    }
}
@JohT JohT added >enhancement needs:triage Requires assignment of a team area label labels Aug 2, 2020
@gwbrown gwbrown added :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache and removed needs:triage Requires assignment of a team area label labels Aug 5, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (:Core/Infra/Scripting)

@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label Aug 5, 2020
@rjernst
Copy link
Member

rjernst commented Aug 13, 2020

We have historically tried to limit the additions we make to languages like mustache. While I understand the desire to have a workaround for the mentioned issues, I do not think search templates are the right place to provide those workarounds. The linked issues are valid on their own, and users should not have to use templates to solve those issues. Given those thoughts above, do you have any other compelling reason for needing a lowercase tag in mustache? It seems the examples provided in this issue can easily be worked around by lowercasing on the client side when generating the search request.

@JohT
Copy link
Author

JohT commented Aug 14, 2020

historically tried to limit the additions we make to languages like mustach

Totally understand. I've recently seen a video on YouTube, where it gets mentioned how conscious new features are discussed here. I can only agree. And so i'm absolutely fine, if this issue gets closed because of that reason.

users should not have to use templates to solve those issue

Good point.

can easily be worked around by lowercasing on the client side

Of course. If i'm not missing something, the same would also apply for functions like {{#url}} or{{#join}}.

do you have any other compelling reason for needing a lowercase tag in mustache?

I only came across the use case i've mentioned above. {{toLowercase}} could be used wherever parameters needs to be lower case. The caller, who sets the parameters, does not need to think about it. If the search template is used in multiple places, it would also be easier to do it once and not have to think about it wherever the call is done.

@rjernst
Copy link
Member

rjernst commented Sep 16, 2020

We discussed this issue, and agreed with my original thinking, that opening Mustache up to more customizations is not something we want to do at this time. We will keep this in mind as future needs of mustache come up in new contexts, but for now the particular need about handling wildcard prefixes in templates should be worked around on the client side. As there are no further actions to take here, I hope you don't mind that I close this issue.

@rjernst rjernst closed this as completed Sep 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Scripting Scripting abstractions, Painless, and Mustache >enhancement Team:Core/Infra Meta label for core/infra team team-discuss
Projects
None yet
Development

No branches or pull requests

4 participants