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

Missing && operator between aggregations #8133

Closed
gpetrou opened this issue Apr 17, 2024 · 5 comments
Closed

Missing && operator between aggregations #8133

gpetrou opened this issue Apr 17, 2024 · 5 comments

Comments

@gpetrou
Copy link
Contributor

gpetrou commented Apr 17, 2024

Elastic.Clients.Elasticsearch version: 8.13.5

Elasticsearch version: 8.11

.NET runtime version: 8.0

Operating system version:

Description of the problem including expected versus actual behavior:
It looks like in 8.13.x versions we cannot use && operator between aggregations. Has this been replaced by something else? If yes, what is that? If no, are you planning to add this back?

Steps to reproduce:
1.
2.
3.

Expected behavior
A clear and concise description of what you expected to happen.

Provide ConnectionSettings (if relevant):

Provide DebugInformation (if relevant):

@gpetrou gpetrou added 8.x Relates to 8.x client version Category: Bug labels Apr 17, 2024
@flobernd
Copy link
Member

Hi @gpetrou! With the new generator design, aggregation variants do not derive from a common base-class anymore (the same thing as for the Query variants).

I'm fine with adding back this functionality to Aggregation, but this will still require to wrap the individual aggregation variants into the container first, before using the operator.

var aggregations = Aggregation.Min(...) && Aggregation.Max(...);

would work fine in this case, but the previously available syntax won't:

var aggregations = new MinAggregation{...} && new MaxAggregation{...};

@flobernd
Copy link
Member

Well, nevermind. I remember why I did not re-implement this 😋

In the versions prior to 8.12, the name of the aggregation was pulled into the actual variant. For example, MinAggregation has a constructor that accepts the aggregation name.

In the versions starting with 8.13, this got changed to be consistent with the resulting JSON structure (the aggregation name is a key in a dictionary and the aggregation variant is the corresponding value).

The old design caused many issues, because it required a lot of hacks during serialization (have a look at AggregationDictionary).

In consequence, this means that we can not compose a dictionary of aggregations from the individual aggregation variants using the && operator, because there is no way to "store" the aggregation name.

But in my opinion, the reason for introducing && in NEST, is no longer relevant as well. In NEST, aggregation dictionaries had to be initialized like:

Aggregations = new AggregationDictionary
{
    {"average_per_child", new AverageAggregation("average_per_child", ...)},
    {"max_per_child", new MaxAggregation("max_per_child", ...)},
    {"min_per_child", new MinAggregation("min_per_child", ...)},
}

and the && syntax simplified that a little bit:

Aggregations =
    new AverageAggregation("average_per_child", ...)
    && new MaxAggregation("max_per_child", ...)
    && new MinAggregation("min_per_child", ...)

In the latest version of the 8.x client, initialization is more straightforward anyways:

Aggregations = new Dictionary<string, Aggregation>
{
    {"average_per_child", Aggregation.Average(...)},
    {"max_per_child", new MaxAggregation(...)},
    {"min_per_child", new MinAggregation(...)},
}

@flobernd
Copy link
Member

I hope you understand, that I can not bring this functionality back for technical reasons. I will make sure to document that in the release notes of 8.13.0.

@gpetrou
Copy link
Contributor Author

gpetrou commented Apr 17, 2024

I see. OK. Thanks.

@a-witkowski
Copy link

@flobernd, can you show me how to initialize Aggregation with aggregations in the latest 8.13.* client?

How "translate" this query in C#:

{
    "from": 0,
    "size": 100,
      "collapse": {
        "field": "uniqueId"
    },
    "query": {
        "match": {
            "Content": "test word"
        }
    },
    "aggregations": {
        "my-terms aggs": {
            "terms": {
                "field": "seqId",
                "size": 100
            },
          	"aggregations": {
        		"my-cardinality-agg": {
          			"cardinality": {
            			"field": "uniqueId"
          			}
       			}
      		}
        }
    }
}
  • i must set and use "my-terms aggs" and "my-cardinality-agg" in code
  • uniqueId and seqId are different fields
  • search is performed on aliases that may contain duplicates

I tried

Aggregation.Terms(new TermsAggregation
{
  Field = Infer.Field("seqId"),
  Size = 100
}).Aggregations = new Dictionary<string, Aggregation>
{
  {
    "my-cardinality-agg",
    new CardinalityAggregation { Field = "uniqueId" }
  }
}

but how to set up name "my-terms aggs" in TermsAggregation in that case?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants