Skip to content

Conversation

@jimczi
Copy link
Contributor

@jimczi jimczi commented Oct 27, 2017

Closes #27134

Copy link
Member

@cbuescher cbuescher left a comment

Choose a reason for hiding this comment

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

@jimczi I'm not too familiar with span_not so I took a look and tried to understand it from the docs. I think it makes more sense like this. I left some small suggestions.

=== Span Not Query

Removes matches which overlap with another span query. The span not
Removes matches which overlap with another span query. or which are
Copy link
Member

Choose a reason for hiding this comment

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

The sentence full-stop should be replaced by a comma I think.


Removes matches which overlap with another span query. The span not
Removes matches which overlap with another span query. or which are
within x tokens before or y tokens after another SpanQuery. The span not
Copy link
Member

Choose a reason for hiding this comment

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

To me, having x and y mentioned without later reference is a bit confusing. If this is what pre/post is refering to, I'd either mention those parameters here (e.g. "within a certain amount of tokens (determined by the pre parameter) before") even if that gets a bit longer.

@jimczi
Copy link
Contributor Author

jimczi commented Oct 27, 2017

Thanks @cbuescher ! I pushed some changes to address your comment.

Copy link
Member

@cbuescher cbuescher left a comment

Choose a reason for hiding this comment

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

LGTM

@jimczi jimczi merged commit a4105c6 into elastic:master Oct 30, 2017
@jimczi jimczi deleted the docs/span_not branch October 30, 2017 10:29
jasontedor added a commit to olcbean/elasticsearch that referenced this pull request Oct 30, 2017
* master: (63 commits)
  [Docs] Fix note in bucket_selector
  [Docs] Fix indentation of examples (elastic#27168)
  [Docs] Clarify `span_not` query behavior for non-overlapping matches (elastic#27150)
  [Docs] Remove first person "I" from getting started (elastic#27155)
  [Docs] Correct link target for datatype murmur3 (elastic#27143)
  Fix division by zero in phrase suggester that causes assertion to fail
  Enable Docstats with totalSizeInBytes for 6.1.0
  Adds average document size to DocsStats (elastic#27117)
  Upgrade Painless from ANTLR 4.5.1-1 to  ANTLR 4.5.3. (elastic#27153)
  Exists template needs a template name (elastic#25988)
  [Tests] Fix occasional test failure due to two random values being the same
  Fix beidermorse phonetic token filter for unspecified `languageset` (elastic#27112)
  Fix max score tracking with field collapsing (elastic#27122)
  [Doc] Add Ingest CSV Processor Plugin to plugin as a community plugin (elastic#27105)
  Removed the beta tag from cross-cluster search
  fixed typo in ConstructingObjectParse (elastic#27129)
  Allow for the Painless Definition to have multiple instances (elastic#27096)
  Apply missing request options to the expand phase (elastic#27118)
  Only pull SegmentReader once in getSegmentInfo (elastic#27121)
  Fix BWC for discovery stats
  ...
jasontedor added a commit that referenced this pull request Oct 30, 2017
* 6.x:
  Refactor internal engine
  [Docs] Fix note in bucket_selector
  Added release notes for 6.0.0-rc2
  [Docs] Fix indentation of examples (#27168)
  [Docs] Clarify `span_not` query behavior for non-overlapping matches (#27150)
  [Docs] Remove first person "I" from getting started (#27155)
jasontedor added a commit that referenced this pull request Oct 30, 2017
* master:
  Refactor internal engine
  [Docs] #26541: add warning regarding the limit on the number of fields that can be queried at once in the multi_match query.
  [Docs] Fix note in bucket_selector
  [Docs] Fix indentation of examples (#27168)
  [Docs] Clarify `span_not` query behavior for non-overlapping matches (#27150)
  [Docs] Remove first person "I" from getting started (#27155)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>docs General docs changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants