-
Notifications
You must be signed in to change notification settings - Fork 17
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
(dsl): Support Filter
aggregation
#349
(dsl): Support Filter
aggregation
#349
Conversation
7ebeec7
to
cb25165
Compare
subAggregations.get(aggName) match { | ||
case Some(aggRes) => | ||
Try(aggRes.asInstanceOf[A]) match { | ||
case Failure(_) => Left(DecodingException(s"Aggregation with name $aggName was not of type you provided.")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This check will probably not work here because of type erasure: at runtime the type A is not known here, so asInstanceOf
will allow anything and a cast error will most likely happen later in user code. An example to demonstrate the case: https://scastie.scala-lang.org/kyL2fVL2S8eQByJb4aS1Yg
5726f35
to
0627b78
Compare
filter
aggregationfilter
aggregation
filter
aggregationFilter
aggregation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You will have to update this PR with the newly added aggregations.
``` | ||
|
||
You can create a `Filter` aggregation using the `filterAggregation` method in the following manner: | ||
```scala |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add the import for term
query.
@@ -143,6 +144,31 @@ private[elasticsearch] final case class Cardinality(name: String, field: String, | |||
} | |||
} | |||
|
|||
sealed trait FilterAggregation extends SingleElasticAggregation with WithSubAgg[FilterAggregation] with WithAgg |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reorder traits (WithSubAgg and WithAgg).
value => { | ||
ZIO.succeed(new AggregateResult(value.aggs.map { case (key, response) => | ||
(key, toResult(response)) | ||
})) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Omit {
and }
.
private[elasticsearch] final case class StatsAggregationResponse( | ||
count: Int, | ||
min: Double, | ||
max: Double, | ||
avg: Double, | ||
sum: Double | ||
) extends AggregationResponse | ||
|
||
private[elasticsearch] object StatsAggregationResponse { | ||
implicit val decoder: JsonDecoder[StatsAggregationResponse] = DeriveJsonDecoder.gen[StatsAggregationResponse] | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please keep it ordered.
@@ -31,6 +31,16 @@ object AggregationResponse { | |||
AvgAggregationResult(value) | |||
case CardinalityAggregationResponse(value) => | |||
CardinalityAggregationResult(value) | |||
case FilterAggregationResponse(docCount, subAggregations) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Omit {
and }
.
subAggregations = Chunk.empty, | ||
query = query |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep params ordered.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In other examples too.
@@ -104,6 +104,67 @@ object HttpExecutorSpec extends IntegrationSpec { | |||
Executor.execute(ElasticRequest.createIndex(firstSearchIndex)), | |||
Executor.execute(ElasticRequest.deleteIndex(firstSearchIndex)).orDie | |||
), | |||
test("aggregate using filter aggregation") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test("aggregate using filter aggregation") { | |
test("aggregate using filter aggregation with max aggregation as a sub aggregation") { |
@@ -104,6 +104,67 @@ object HttpExecutorSpec extends IntegrationSpec { | |||
Executor.execute(ElasticRequest.createIndex(firstSearchIndex)), | |||
Executor.execute(ElasticRequest.deleteIndex(firstSearchIndex)).orDie | |||
), | |||
test("aggregate using filter aggregation") { | |||
val expectedResponse = ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
val expectedResponse = ( | |
val expectedResult = ( |
``` | ||
|
||
If you want to add aggregation (on the same level), you can use `withAgg` method: | ||
```scala |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
``` | ||
|
||
If you want to add another sub-aggregation, you can use `withSubAgg` method: | ||
```scala |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here add also for maxAggregation
too.
a6efadf
to
30edae8
Compare
.refreshTrue | ||
) | ||
query = term(field = TestDocument.stringField, value = secondDocumentUpdated.stringField.toLowerCase) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Omit empty line.
filterAggregation(name = "aggregation", query = query).withSubAgg( | ||
maxAggregation("subAggregation", TestDocument.intField) | ||
) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Omit empty line.
@@ -120,6 +224,7 @@ private[elasticsearch] final case class SumAggregationResponse(value: Double) ex | |||
|
|||
private[elasticsearch] object SumAggregationResponse { | |||
implicit val decoder: JsonDecoder[SumAggregationResponse] = DeriveJsonDecoder.gen[SumAggregationResponse] | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Omit empty line.
modules/library/src/main/scala/zio/elasticsearch/executor/response/AggregationResponse.scala
Show resolved
Hide resolved
filterAggregation("aggregation", query).withSubAgg(minAggregation("subAggregation", TestDocument.intField)) | ||
val aggregationWithMultipleSubAggregations = filterAggregation("aggregation", query) | ||
.withSubAgg(maxAggregation("maxSubAggregation", TestDocument.intField)) | ||
.withSubAgg(minAggregation("minSubAggregation", TestDocument.intField)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Put other numeric field (not intField
) for minAggregation.
filterAggregation("aggregation", query).withSubAgg(minAggregation("subAggregation", TestDocument.intField)) | ||
val aggregationWithMultipleSubAggregations = filterAggregation("aggregation", query) | ||
.withSubAgg(maxAggregation("maxSubAggregation", TestDocument.intField)) | ||
.withSubAgg(minAggregation("minSubAggregation", TestDocument.intField)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
c058f2c
to
5914223
Compare
4a374b4
to
422b24b
Compare
Part of #118