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

Multi Term Vectors needs filter params? #315

Closed
saxsir opened this issue Jul 1, 2016 · 9 comments
Closed

Multi Term Vectors needs filter params? #315

saxsir opened this issue Jul 1, 2016 · 9 comments

Comments

@saxsir
Copy link

saxsir commented Jul 1, 2016

Which version of Elastic are you using?

[ ] elastic.v2 (for Elasticsearch 1.x)
[x] elastic.v3 (for Elasticsearch 2.x)

Please describe the expected behavior

Multi Term Vectors API might be possible to use filter params.

https://www.elastic.co/guide/en/elasticsearch/reference/2.3/docs-multi-termvectors.html

See the termvectors API for a description of possible parameters.

It's not implemented yet. I want to contribute if it is needed.

However, there're not specs in https://github.com/elastic/elasticsearch/blob/master/rest-api-spec/src/main/resources/rest-api-spec/api/mtermvectors.json

@olivere
Copy link
Owner

olivere commented Jul 1, 2016

If they're not in the REST spec, they're probably not officially supported. However, I cannot see a filter field/param on the docs page either. The Java source code is the ultimate source of truth.

Am 01.07.2016 um 11:46 schrieb Shohei Sasamoto notifications@github.com:

Which version of Elastic are you using?

[ ] elastic.v2 (for Elasticsearch 1.x)
[x] elastic.v3 (for Elasticsearch 2.x)

Please describe the expected behavior

Multi Term Vectors API might be possible to use filter params.

https://www.elastic.co/guide/en/elasticsearch/reference/2.3/docs-multi-termvectors.html

See the termvectors API for a description of possible parameters.

However, it's not implemented yet. I want to contribute if it is needed.

However, there're not specs in https://github.com/elastic/elasticsearch/blob/master/rest-api-spec/src/main/resources/rest-api-spec/api/mtermvectors.json


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

@saxsir
Copy link
Author

saxsir commented Jul 1, 2016

Thanks for your reply.

The Java source code is the ultimate source of truth.

ok, i got it. thanks.

@saxsir saxsir closed this as completed Jul 1, 2016
@saxsir
Copy link
Author

saxsir commented Jul 4, 2016

Sorry, reopened this issue because I found that The Java source code implemented filter params on mtermvectors.

elastic/elasticsearch#12311
elastic/elasticsearch#12312

I will contribute to add filter on multi term vectors API implementation.

@saxsir saxsir reopened this Jul 4, 2016
@olivere
Copy link
Owner

olivere commented Jul 4, 2016

@saxsir Can you file an issue at their site trying to find out if a) the filter param is officially supported and if so, b) where documentation can be found and c) if it's missing in the REST API spec?

@saxsir
Copy link
Author

saxsir commented Jul 4, 2016

@olivere Thank you for your reply.

a), b)
In this page, there is written the description below.

See the termvectors API for a description of possible parameters.

So mtermvectors API should possible to use params that same as termvectors API.
I think this is an official documentation.

c)
I'm not sure.

Also in the termvectors spec file, there is not spec that supported filter params.

But The Java library supported it, and also this olivere/elastic golang client library supported it .

So I have a question, where did you find the documentation that termvectors API support filter params (because there is not spec file, I think..)?

@olivere
Copy link
Owner

olivere commented Jul 4, 2016

Ah... they're referring to the Termvectors API there. Now I see it. Thanks.

Perhaps it once supported it, but they deprecated it at some point. At some point I found the REST API spec to be missing many things, so I tend to use the Java API as reference. Things got a bit better over time. It's confusing that there's no filter in the REST specification, isn't it? We should clarify, no?

The REST API spec is a nice approach, but it's not even 50% of the effort in writing a client for ES. As it is now, it's only usable together with the high-level documentation and the source code, I'm afraid. The spec only specifies the request side of things, and only the request parameters. For client-side developers like me, this is dissatisfying: We have no idea about the structure of the body and the response. As an author of an ES client you have two options: a) Dig into the source code and find out yourself, or b) simply expose a body parameter and let every user of your client find out. But I digress.

If filter is in the Java API and not explicitly deprecated in their breaking changes, I tend to include it.

@saxsir
Copy link
Author

saxsir commented Jul 4, 2016

there's no filter in the REST specification

No, no specification but it's implemented. I asked them about it while ago.
elastic/elasticsearch#19243

If filter is in the Java API and not explicitly deprecated in their breaking changes, I tend to include it.

OK, thanks a lot. I will contribute it in few days.

@olivere
Copy link
Owner

olivere commented Sep 1, 2016

I took another look at this today. I think Term Vectors API and Multi-Term Vectors API need to be refactored, then we can add this. Many of the parameters are passed in the query string. Although this works for simple types, it's really not working well (or is at least bad style) for structures like the filter settings. The docs for Term Vectors API and Multi-Term Vectors API illustrate how we should really use the API, i.e. pass parameters in the body of the request.

Furthermore, the parameters should be extracted from TermvectorsService into a TermvectorsRequest and then be reused both in TermvectorsService and in MultiTermvectorsService.

I will do all of this eventually. It's a matter of time. Please bear with me.

@olivere
Copy link
Owner

olivere commented Mar 24, 2018

Closing it for it is too old. Elastic v6 supports the filter in the Term Vectors API.

@olivere olivere closed this as completed Mar 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants