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

nGram and edgeNGram names don't throw an error with custom filters on ES 7+ #50360

Closed
ankane opened this issue Dec 19, 2019 · 3 comments · Fixed by #50376
Closed

nGram and edgeNGram names don't throw an error with custom filters on ES 7+ #50360

ankane opened this issue Dec 19, 2019 · 3 comments · Fixed by #50376
Assignees
Labels
>bug :Search Relevance/Analysis How text is split into tokens Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch

Comments

@ankane
Copy link

ankane commented Dec 19, 2019

Elasticsearch version (bin/elasticsearch --version): 7.0.0 - 7.5.1

Plugins installed: []

JVM version (java -version): n/a

OS version (uname -a if on a Unix-like system): n/a

Description of the problem including expected versus actual behavior:

Hi, according to the docs, the nGram and edgeNGram token filter should throw an error in ES 7 (since they've been replaced with ngram and edge_ngram). However, this doesn't appear to be the case if you use them in custom filters.

Steps to reproduce:

curl -X PUT "localhost:9200/my_index?pretty" -H 'Content-Type: application/json' -d'
{
  "settings": {
    "analysis": {
      "analyzer": {
        "bad_analyzer": { 
          "type": "custom",
          "tokenizer": "standard",
          "filter": [
            "lowercase",
            "bad_filter"
          ]
        }
      },
      "filter": {
        "bad_filter": {
          "type": "nGram"
        }
      }
    }
  }
}
'
curl -X GET "localhost:9200/my_index/_analyze?pretty" -H 'Content-Type: application/json' -d'
{
  "analyzer": "bad_analyzer", 
  "text":     "Is this déjà vu?"
}
'
@cbuescher cbuescher added :Search Relevance/Analysis How text is split into tokens :Search Foundations/Mapping Index mappings, including merging and defining field types labels Dec 19, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-search (:Search/Mapping)

@cbuescher cbuescher removed the :Search Foundations/Mapping Index mappings, including merging and defining field types label Dec 19, 2019
@cbuescher cbuescher self-assigned this Dec 19, 2019
@cbuescher
Copy link
Member

@ankane thanks for reporting, this should indeed also work for custom filters. I'll open a PR to add these checks to 7.x shortly.

@cbuescher cbuescher added the >bug label Dec 19, 2019
cbuescher pushed a commit to cbuescher/elasticsearch that referenced this issue Dec 20, 2019
The camel-case `nGram` and `edgeNGram` filter names were deprecated in 6. We
currently throw errors on new indices when they are used. However these errors
are currently only thrown for pre-configured filters, adding them as custom
filters doesn't trigger the warning and error. This change adds the appropriate
exceptions for `nGram` and `edgeNGram` respectively.

Closes elastic#50360
cbuescher pushed a commit that referenced this issue Dec 20, 2019
The camel-case `nGram` and `edgeNGram` filter names were deprecated in 6. We
currently throw errors on new indices when they are used. However these errors
are currently only thrown for pre-configured filters, adding them as custom
filters doesn't trigger the warning and error. This change adds the appropriate
exceptions for `nGram` and `edgeNGram` respectively.

Closes #50360
@ankane
Copy link
Author

ankane commented Dec 20, 2019

Thanks @cbuelter!

cbuescher pushed a commit to cbuescher/elasticsearch that referenced this issue Dec 20, 2019
The camel-case `nGram` and `edgeNGram` filter names were deprecated in 6. We
currently throw errors on new indices when they are used. However these errors
are currently only thrown for pre-configured filters, adding them as custom
filters doesn't trigger the warning and error. This change adds the appropriate
deprecation warnings for `nGram` and `edgeNGram` respectively on version 7
indices.

Relates elastic#50360
cbuescher pushed a commit that referenced this issue Dec 20, 2019
The camel-case `nGram` and `edgeNGram` filter names were deprecated in 6. We
currently throw errors on new indices when they are used. However these errors
are currently only thrown for pre-configured filters, adding them as custom
filters doesn't trigger the warning and error. This change adds the appropriate
deprecation warnings for `nGram` and `edgeNGram` respectively on version 7
indices.

Relates #50360
SivagurunathanV pushed a commit to SivagurunathanV/elasticsearch that referenced this issue Jan 23, 2020
…#50376)

The camel-case `nGram` and `edgeNGram` filter names were deprecated in 6. We
currently throw errors on new indices when they are used. However these errors
are currently only thrown for pre-configured filters, adding them as custom
filters doesn't trigger the warning and error. This change adds the appropriate
exceptions for `nGram` and `edgeNGram` respectively.

Closes elastic#50360
@javanna javanna added the Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch label Jul 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Search Relevance/Analysis How text is split into tokens Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants