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

Consolidate usage of FluentDescriptorDictionary #8349

Open
Tracked by #8356
niemyjski opened this issue Sep 16, 2024 · 3 comments
Open
Tracked by #8356

Consolidate usage of FluentDescriptorDictionary #8349

niemyjski opened this issue Sep 16, 2024 · 3 comments
Labels
8.x Relates to 8.x client version Category: Question Usability

Comments

@niemyjski
Copy link
Contributor

Elastic.Clients.Elasticsearch version: 8.15.6

Elasticsearch version: 8.15.1

.NET runtime version: 8.x

Operating system version: Any

Description of the problem including expected versus actual behavior:

In the past we built up queries and then assigned them to the descriptor. This is useful in our scenario but also in several apps when you want to conditionally build up queries based on method parameters over various control statements.

var result = await processor.BuildAggregationsAsync($"date:(field5~{interval})");
await Client.SearchAsync<MyType>(d => d.Index(index).Aggregations(result));

//  ElasticQueryParserTests.cs(712, 94): [CS0201] Only assignment, call, increment, decrement, await, and new object expressions can be used as a statement

var result = await processor.BuildQueryAsync("field1:value1", new ElasticQueryVisitorContext().UseSearchMode());
await Client.SearchAsync<MyType>(d => d.Index(index).Query(_ => result));
//   ElasticQueryParserTests.cs(717, 29): [CS0201] Only assignment, call, increment, decrement, await, and new object expressions can be used as a statement

Expected behavior

I should be able to apply built up query and aggregation descriptors to the fluent api

Reference: FoundatioFx/Foundatio.Parsers#84

@flobernd
Copy link
Member

Hi @niemyjski,

this works fine for Query, if you omit the _ =>:

var query = new QueryDescriptor<Person>()
    .Term(t => t
        .Field("user.id")
        .Value("kimchy")
    );

var response = await client.SearchAsync<Person>(s => s
    .Query(query)
);

we always generate multiple overloads to support these kind of assignments:

public SearchRequestDescriptor<TDocument> Query(QueryDsl.Query? query)
public SearchRequestDescriptor<TDocument> Query(QueryDescriptor<TDocument> descriptor)
public SearchRequestDescriptor<TDocument> Query(Action<QueryDsl.QueryDescriptor<TDocument>> configure)

For aggregations it's slightly different, but this should work:

var aggs = new FluentDescriptorDictionary<string, AggregationDescriptor<Person>>();
aggs = aggs.Add("name1", agg => agg.Avg(avg => avg.Field("test")));
aggs = aggs.Add("name2", agg => agg.Min(min => min.Field("test")));

var response = await client.SearchAsync<Person>(s => s
    .Aggregations(_ => aggs)
);

As discussed in the other issue, it would be good to have an overload that directly accepts a regular collection of <string, Aggregation<T>> and <string, AggregationDescriptor<T>>.

Please let me know, if we can close this issue.

@niemyjski
Copy link
Contributor Author

niemyjski commented Sep 17, 2024

Feels like if you can do .Query(query) than this should work too .Aggregations(aggregations) (I need to confirm looks like I have a typing issue on my side, will confirm and close if we are good.

Copy link
Contributor

This issue is stale because it has been open 5 days with no activity. Remove stale label or comment or this will be closed in 2 days.

@flobernd flobernd changed the title 8.15.6 - AggregationDescriptor / QueryDescriptor assignment Consolidate usage of FluentDescriptorDictionary Sep 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.x Relates to 8.x client version Category: Question Usability
Projects
None yet
Development

No branches or pull requests

2 participants