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

Remove DFS_QUERY_AND_FETCH as a search type #22787

Merged
merged 2 commits into from
Jan 26, 2017
Merged

Remove DFS_QUERY_AND_FETCH as a search type #22787

merged 2 commits into from
Jan 26, 2017

Conversation

s1monw
Copy link
Contributor

@s1monw s1monw commented Jan 25, 2017

This commit removes the search type dfs_query_and_fetch without a
replacement. We don't allow to use this type via REST since 2.x
but still keep it around for no particular reason. There we no users
complaining about the availability. This should now be removed from the
codebase. query_and_fetch is still used internally to safe a roundtrip
if there is only one shard but it can't be used via the rest interface.

@s1monw
Remote DFS_QUERY_AND_FETCH as a search type
6ec25a2 
This commit removes the search type `dfs_query_and_fetch` without a
replacement. We don't allow to use this type via REST since 2.x
but still keep it around for no particular reason. There we no users
complaining about the availability. This should now be removed from the
codebase. `query_and_fetch` is still used internally to safe a roundtrip
if there is only one shard but it can't be used via the rest interface.
@s1monw s1monw added :Search/Search Search-related issues that do not fall into other categories >breaking >breaking-java review v6.0.0-alpha1 labels Jan 25, 2017
jpountz
jpountz approved these changes Jan 25, 2017
View reviewed changes
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.

LGTM

@s1monw
Copy link
Contributor Author

s1monw commented Jan 25, 2017

@elasticmachine test this please

@clintongormley clintongormley changed the title Remote DFS_QUERY_AND_FETCH as a search type Remove DFS_QUERY_AND_FETCH as a search type Jan 25, 2017
@s1monw s1monw merged commit 281250d into elastic:master Jan 26, 2017
@s1monw s1monw deleted the trash_dfs_query_and_fetch branch January 26, 2017 08:14
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Jan 26, 2017
@jasontedor
Merge branch 'master' into replica-sequence-number-recovery
137ffb4 
* master: (47 commits)
  Remove non needed import
  use expectThrows instead of manually testing exception
  Fix checkstyle and a test
  Update after review
  Read ec2 discovery address from aws instance tags
  Invalidate cached query results if query timed out (elastic#22807)
  Add remaining generated painless API
  Generate reference links for painless API (elastic#22775)
  [TEST] Fix ElasticsearchExceptionTests
  Add parsing method for ElasticsearchException.generateThrowableXContent() (elastic#22783)
  Improve connection closing in `RemoteClusterConnection` (elastic#22804)
  Docs: Cluster allocation explain should be on one page
  Remove DFS_QUERY_AND_FETCH as a search type (elastic#22787)
  Add repository-url module and move URLRepository (elastic#22752)
  fix date-processor to a new default year for every new pipeline execution. (elastic#22601)
  Add tests for top_hits aggregation (elastic#22754)
  [TEST] Added this for 93a28b0 submitted via elastic#22772
  Fix typo in comment in OsProbe.java
  Add new ruby search library to community clients doc (elastic#22765)
  RangeQuery WITHIN case now normalises query (elastic#22431)
  ...
@s1monw s1monw added the v5.3.0 label Jan 27, 2017
@s1monw
Copy link
Contributor Author

s1monw commented Jan 27, 2017

I will port this to 5.x as well. documentation for this stuff has been removed long ago and it's impossible to use it with REST anyway. @jpountz @clintongormley objections?

s1monw added a commit to s1monw/elasticsearch that referenced this pull request Jan 27, 2017
@s1monw
Remove DFS_QUERY_AND_FETCH as a search type (elastic#22787)
5e1672c 
This commit removes the search type `dfs_query_and_fetch` without a
replacement. We don't allow to use this type via REST since 2.x
but still keep it around for no particular reason. There we no users
complaining about the availability. This should now be removed from the
codebase. `query_and_fetch` is still used internally to safe a roundtrip
if there is only one shard but it can't be used via the rest interface.
@jpountz
Copy link
Contributor

jpountz commented Jan 27, 2017

No objections, we removed the documentation for it a long time ago.

@clintongormley
Copy link
Contributor

+1 I can't imagine why anybody would be using this in java land

@dadoonet
Copy link
Member

@clintongormley And we wrote that in Java doc 2 years ago: https://www.elastic.co/guide/en/elasticsearch/client/java-api/current/java-search.html

Although the Java API defines the additional search types QUERY_AND_FETCH and DFS_QUERY_AND_FETCH, these modes are internal optimizations and should not be specified explicitly by users of the API.

So to me it's really clear that people should never use that.

@Mpdreamz Mpdreamz mentioned this pull request Feb 15, 2017
Closed
@gmarz
Copy link
Contributor

gmarz commented Feb 16, 2017

We should also remove dfs_query_then_fetch as an option from the REST spec, but since that will leave query_then_fetch as the only option, should we remove the search_type parameter altogether?

@clintongormley
Copy link
Contributor

@gmarz dfs_query_AND_fetch, not dfs_query_THEN_fetch

@gmarz
Copy link
Contributor

gmarz commented Feb 16, 2017

@clintongormley ah! thanks -- totally misread this

mtdavidson added a commit to mtdavidson/Elastica that referenced this pull request Jun 3, 2017
@mtdavidson
Remove DFS_QUERY_AND_FETCH as a search type
9769b8a 
 - elastic/elasticsearch#22787
 - https://www.elastic.co/guide/en/elasticsearch/reference/5.3/release-notes-5.3.0.html
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jpountz jpountz jpountz approved these changes

Assignees
No one assigned
Labels
>breaking >breaking-java :Search/Search Search-related issues that do not fall into other categories v5.3.0 v6.0.0-alpha1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@s1monw @jpountz @clintongormley @dadoonet @gmarz