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

Completion suggestion should also consider text if prefix/regex is missing #23451

Merged
merged 1 commit into from
Mar 16, 2017

Conversation

cbuescher
Copy link
Member

In cases where the user specifies only the text option on the top level
suggest element (either via REST or the java api), this gets transferred to the
text property in the SuggestionSearchContext. CompletionSuggestionContext
currently requires prefix or regex to be specified, otherwise errors. We should
use the global text property as a fall back if provided in this case.

Note that when text is set on the CompletionSuggestionBuilder directly, we already
overwrite a missing prefix property in SuggestionBuilder#populateCommonFields.
This, however, currently doesn't work with the global text overwrite taking place in
SuggestBuilder#build

Closes to #23340

…efix/regex missing

In cases where the user specifies only the `text` option on the top level
suggest element (either via REST or the java api), this gets transferred to the
`text` property in the SuggestionSearchContext. CompletionSuggestionContext
currently requires prefix or regex to be specified, otherwise errors. We should
use the global `text` property as a fall back if provided in this case.

Closes to elastic#23340
@cbuescher cbuescher changed the title Completion suggestion should also consider text if prefix/rege is missing Completion suggestion should also consider text if prefix/regex is missing Mar 2, 2017
@cbuescher
Copy link
Member Author

@areek could you please take a look at this?

Copy link
Contributor

@areek areek left a comment

Choose a reason for hiding this comment

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

Thanks @cbuescher for catching the bug! LGTM :)

@cbuescher cbuescher merged commit 96a92da into elastic:master Mar 16, 2017
@cbuescher
Copy link
Member Author

@areek thanks for the review

cbuescher added a commit that referenced this pull request Mar 16, 2017
…efix/regex missing (#23451)

In cases where the user specifies only the `text` option on the top level
suggest element (either via REST or the java api), this gets transferred to the
`text` property in the SuggestionSearchContext. CompletionSuggestionContext
currently requires prefix or regex to be specified, otherwise errors. We should
use the global `text` property as a fallback if neither prefix nor regex is provided.

Closes to #23340
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Mar 17, 2017
* master:
  Clear the interrupt flag before joining
  Migrate to max line length of 100
  Docs: Corrected path to elasticsearch-plugin (elastic#23622)
  Docs: Add comma to reverse nested agg snippet
  Fix third-party audit task for Gradle 3.4 (elastic#23612)
  Adapter action future should restore interrupts
  Update scripting.asciidoc
  Unmark reindex as experimental
  CompletionSuggestionContext#toQuery() should also consider text if prefix/regex missing (elastic#23451)
  Docs: Specify that byte units use powers of 1024 (elastic#23574)
  Remove Settings.settingsBuilder (elastic#23575)
  Change params._source to params['_source'] in example.
  Fix example in documentation for Painless using _source. (elastic#21322)
  Remove extra line from license header
  Fix num docs to be positive in bucket deferring collector test
  Mapping: Fix NPE with scaled floats stats when field is not indexed (elastic#23528)
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Mar 17, 2017
* single-node-discovery:
  Clear the interrupt flag before joining
  Migrate to max line length of 100
  Docs: Corrected path to elasticsearch-plugin (elastic#23622)
  Docs: Add comma to reverse nested agg snippet
  Fix third-party audit task for Gradle 3.4 (elastic#23612)
  Adapter action future should restore interrupts
  Update scripting.asciidoc
  Unmark reindex as experimental
  CompletionSuggestionContext#toQuery() should also consider text if prefix/regex missing (elastic#23451)
  Docs: Specify that byte units use powers of 1024 (elastic#23574)
  Remove Settings.settingsBuilder (elastic#23575)
  Change params._source to params['_source'] in example.
  Fix example in documentation for Painless using _source. (elastic#21322)
  Remove extra line from license header
  Fix num docs to be positive in bucket deferring collector test
  Mapping: Fix NPE with scaled floats stats when field is not indexed (elastic#23528)
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Mar 17, 2017
* master:
  Clear the interrupt flag before joining
  Migrate to max line length of 100
  Docs: Corrected path to elasticsearch-plugin (elastic#23622)
  Docs: Add comma to reverse nested agg snippet
  Fix third-party audit task for Gradle 3.4 (elastic#23612)
  Adapter action future should restore interrupts
  Update scripting.asciidoc
  Unmark reindex as experimental
  CompletionSuggestionContext#toQuery() should also consider text if prefix/regex missing (elastic#23451)
  Docs: Specify that byte units use powers of 1024 (elastic#23574)
  Remove Settings.settingsBuilder (elastic#23575)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants