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

Add boolean similarity to built in similarity types #26613

Merged
merged 3 commits into from
Sep 13, 2017

Conversation

brusic
Copy link
Contributor

@brusic brusic commented Sep 12, 2017

The new boolean similarity is not added to the builtin similarities, so trying to use it will fail:

PUT /test_sim_boolean
{
  "settings": {
    "index": {
      "similarity": {
        "default": {
          "type": "boolean"
        }
      }
    }
  }
}
PUT test_sim_boolean
{
  "settings": {
    "similarity": {
      "boolean_sim": {
        "type": "boolean"
      }
    }
  }
}
{
   "error": {
      "root_cause": [
         {
            "type": "illegal_argument_exception",
            "reason": "Unknown Similarity type [boolean] for [default]"
         }
      ],
      "type": "illegal_argument_exception",
      "reason": "Unknown Similarity type [boolean] for [default]"
   },
   "status": 400
}

The workaround is to simply use the similarity directly in every field that requires it

PUT /test_sim_boolean
{
    "mappings": {
      "foo": {
        "properties": {
          "bar": {
            "type": "text",
            "similarity": "boolean"
          }
        }
      }
    }
  }
}
  • First commit contains the fix.
  • Second commit is a simple rename of the local variable. I can remove this commit if you really like the previous name or I can squash it.
  • Third commit: after analyzing the code in order to future proof it, it is clear based on the three data points that built-in similarities include all of the default ones

@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

Copy link
Contributor

@jpountz jpountz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, the change looks good to me.

@jpountz
Copy link
Contributor

jpountz commented Sep 13, 2017

@elasticmachine please test it

@jpountz jpountz merged commit 9e05b32 into elastic:master Sep 13, 2017
@jpountz jpountz added the >bug label Sep 13, 2017
jasontedor added a commit to jpountz/elasticsearch that referenced this pull request Sep 13, 2017
* master: (21 commits)
  Ensure module is bundled before installing in tests
  Add boolean similarity to built in similarity types (elastic#26613)
  [Tests] Remove skip tests in search/30_limits.yml
  Let search phases override max concurrent requests
  Add a soft limit for the number of requested doc-value fields (elastic#26574)
  Support for accessing Azure repositories through a proxy (elastic#23518)
  Add beta tag to MSI Windows Installer (elastic#26616)
  Fix Lucene version of 5.6.1.
  Remove azure deprecated settings (elastic#26099)
  Handle the 5.6.0 release
  Allow plugins to validate cluster-state on join (elastic#26595)
  Remove index mapper dynamic settings (elastic#25734)
  update AWS SDK for ECS Task IAM support in discovery-ec2 (elastic#26479)
  Azure repository: Accelerate the listing of files (used in delete snapshot) (elastic#25710)
  Build: Remove norelease from forbidden patterns (elastic#26592)
  Fix reference to painless inside expression engine (elastic#26528)
  Build: Move javadoc linking to root build.gradle (elastic#26529)
  Test: Remove leftover static bwc test case (elastic#26584)
  Docs: Remove remaining references to file and native scripts (elastic#26580)
  Snapshot fallback should consider build.snapshot
  ...
jasontedor added a commit to synhershko/elasticsearch that referenced this pull request Sep 14, 2017
* master: (39 commits)
  [Docs] Correct typo in removal_of_types.asciidoc (elastic#26646)
  [Docs] "The the" is a great band, but ... (elastic#26644)
  Move all repository-azure classes under one single package (elastic#26624)
  [Docs] Update link in removal_of_types.asciidoc (elastic#26614)
  Fix percolator highlight sub fetch phase to not highlight query twice (elastic#26622)
  Refactor bootstrap check results and error messages
  Add BootstrapContext to expose settings and recovered state to bootstrap checks (elastic#26628)
  [Tests] Removing skipping tests in search rest tests
  Initialize checkpoint tracker with allocation ID
  Move non-core mappers to a module. (elastic#26549)
  [Docs] Clarify size parameter in Completion Suggester doc (elastic#26617)
  Add soft limit on allowed number of script fields in request (elastic#26598)
  Remove MapperService#dynamic. (elastic#26603)
  Fix incomplete sentences in parent-join docs (elastic#26623)
  More efficient encoding of range fields. (elastic#26470)
  Ensure module is bundled before installing in tests
  Add boolean similarity to built in similarity types (elastic#26613)
  [Tests] Remove skip tests in search/30_limits.yml
  Let search phases override max concurrent requests
  Add a soft limit for the number of requested doc-value fields (elastic#26574)
  ...
@lcawl lcawl removed the v6.1.0 label Dec 12, 2017
@clintongormley clintongormley added :Search/Search Search-related issues that do not fall into other categories and removed :Similarities labels Feb 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Search/Search Search-related issues that do not fall into other categories v5.6.1 v6.0.0-rc1 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants