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.

Sign up for GitHub

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

Jump to bottom

Simplify AllEntries, AllField and AllFieldMapper: #17613

Merged
merged 1 commit into from
Apr 11, 2016
Merged

Simplify AllEntries, AllField and AllFieldMapper: #17613

merged 1 commit into from
Apr 11, 2016

Conversation

jimczi
Copy link
Contributor

@jimczi jimczi commented Apr 8, 2016

  • Create one AllField field per field eligible for _all.
  • Add a positionIncrementGap (with a size of 100, not configurable) between
    each entry in order to distinguish fields when doing phrase query on _all.

@jimczi
Simplify AllEntries, AllField and AllFieldMapper:
c565718 
  * Create one AllField field per field eligible for _all.
  * Add a positionIncrementGap (with a size of 100, not configurable) between
  each entry in order to distinguish fields when doing phrase query on _all.
@jimczi jimczi added >enhancement :Search Foundations/Mapping Index mappings, including merging and defining field types v5.0.0-alpha2 labels Apr 8, 2016
@jpountz
Copy link
Contributor

jpountz commented Apr 9, 2016

This looks like a great cleanup! Does this change have any implications at runtime (I am wondering for instance if highlighting still works the same way)?

@jimczi
Copy link
Contributor Author

jimczi commented Apr 11, 2016

thanks @jpountz
Highlighting would work exactly like a multi-valued string field. Though the _all field is not stored by default (and is not stored in _source either).

@jpountz
Copy link
Contributor

jpountz commented Apr 11, 2016

I played a bit with highlighting. We used to get something like this before:

{
  "took": 31,
  "timed_out": false,
  "_shards": {
    "total": 5,
    "successful": 5,
    "failed": 0
  },
  "hits": {
    "total": 1,
    "max_score": 0.49999997,
    "hits": [
      {
        "_index": "index",
        "_type": "t",
        "_id": "1",
        "_score": 0.49999997,
        "_source": {
          "foo": "the quick fox jumps over the lazy dog",
          "bar": "the lazy dog ate the big cat"
        },
        "highlight": {
          "_all": [
            "the quick fox jumps over the <em>lazy</em> <em>dog</em> the <em>lazy</em> <em>dog</em> ate the big cat "
          ]
        }
      }
    ]
  }
}

and now we have:

{
  "took": 59,
  "timed_out": false,
  "_shards": {
    "total": 5,
    "successful": 5,
    "failed": 0
  },
  "hits": {
    "total": 1,
    "max_score": 0.49999997,
    "hits": [
      {
        "_index": "index",
        "_type": "t",
        "_id": "1",
        "_score": 0.49999997,
        "_source": {
          "foo": "the quick fox jumps over the lazy dog",
          "bar": "the lazy dog ate the big cat"
        },
        "highlight": {
          "_all": [
            "the quick fox jumps over the <em>lazy</em> <em>dog</em>",
            "the <em>lazy</em> <em>dog</em> ate the big cat"
          ]
        }
      }
    ]
  }
}

which I think is much nicer.

LGTM

@jimczi jimczi merged commit 2713a08 into elastic:master Apr 11, 2016
@jimczi jimczi deleted the all_field branch April 11, 2016 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
>enhancement :Search Foundations/Mapping Index mappings, including merging and defining field types v5.0.0-alpha2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jimczi @jpountz