Use typed_keys parameter to prefix suggester names by type in search responses#23080
Use typed_keys parameter to prefix suggester names by type in search responses#23080tlrx merged 2 commits intoelastic:masterfrom
typed_keys parameter to prefix suggester names by type in search responses#23080Conversation
This pull request adds a new parameter to the REST Search API named `prefixed_name`. When set to true, the suggester names in the search response will be prefixed with a prefix that reflects their type.
|
|
||
| private static final Set<String> RESPONSE_PARAMS = Collections.singleton("typed_keys"); | ||
| public static final String TYPED_KEYS_PARAM = "typed_keys"; | ||
| private static final Set<String> RESPONSE_PARAMS = Collections.singleton(TYPED_KEYS_PARAM); |
| builder.startArray(name); | ||
| if (params.paramAsBoolean(RestSearchAction.TYPED_KEYS_PARAM, false)) { | ||
| // Concatenates the type and the name of the aggregation (ex: top_hits#foo) | ||
| builder.startArray(String.join(InternalAggregation.TYPED_KEYS_DELIMITER, getType(), getName())); |
There was a problem hiding this comment.
Just a thought: maybe instead of referring to the Delimiter in Aggregations we can have an own constant in Suggest and either declare the same symbol there or delegate to InternalAggregation.TYPED_KEYS_DELIMITER? The delimiter doesn't need to be the same in aggs and suggestions, only the parsing part needs to refer to it (although its good to have the same like we do now I think)
There was a problem hiding this comment.
why would we want to use the same parameter but not the same delimiter? aren't we making things more complicated than they are?
There was a problem hiding this comment.
Thanks @cbuescher but I feel like this will complicates things, I prefer to leave it as it is.
| @@ -0,0 +1,84 @@ | |||
| [[returning-suggesters-type]] | |||
| === Returning the type of the suggester | |||
There was a problem hiding this comment.
At first I was wondering if we should document this parameter since it is only for internal use, but then I saw you also did this for aggregations, so I agree it might be useful for anybody who needs to parse the response.
There was a problem hiding this comment.
Yes, I think it is still useful to document it correctly, that might be useful for others clients too.
There was a problem hiding this comment.
This has had me stuck for a day, as to why my aggs all of a sudden had a prefix to the name. Is there a way to disable?
There was a problem hiding this comment.
this is concerning, this change was backwards compatible meaning that typed_keys defaults to false, and only the java high-level REST client enables them. Can you describe what you have experienced?
|
In the description you say "suggester names", but it affect agg names too, no? |
|
@clintongormley We added the typed_keys parameter support for aggregation in #22965, which is already merged. This PR is for the same thing for Suggesters. |
|
@clintongormley @tlrx I think what is confusing in the title here is that the parameter is not "added". it is just applied to suggestions too in the search response. |
typed_keys parameter to prefix suggester names by type in search responses
|
Thanks @javanna @cbuescher ! I updated the description and the title of the PR/commit. |
* master: Cleanup RestGetAliasesAction.java Use `typed_keys` parameter to prefix suggester names by type in search responses (elastic#23080) [Docs] Remove unnecessary // TEST[continued] in search-template doc
|
Is there a way to disable typed keys? |
|
@sqlboy Suggesters names are only prefixed by their types when the |
This pull request reuses the
typed_keysparameter added in #22965, but this time it applies it to suggesters. When set to true, the suggester names in the search response will be prefixed with a prefix that reflects their type.