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

(dsl): Support Extended stats aggregation #363

Merged

Conversation

vanjaftn
Copy link
Contributor

@vanjaftn vanjaftn commented Nov 13, 2023

Part of #118


If you want to add aggregation (on the same level), you can use `withAgg` method:
```scala
val multipleAggregations: MultipleAggregations = extendedStatsAggregation(name = "extendedStatsAggregation", field = Document.intField).withAgg(extendedStatsAggregation(name = "extendedStatsAggregation2", field = Document.doubleField))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
val multipleAggregations: MultipleAggregations = extendedStatsAggregation(name = "extendedStatsAggregation", field = Document.intField).withAgg(extendedStatsAggregation(name = "extendedStatsAggregation2", field = Document.doubleField))
val multipleAggregations: MultipleAggregations = extendedStatsAggregation(name = "extendedStatsAggregation1", field = Document.intField).withAgg(extendedStatsAggregation(name = "extendedStatsAggregation2", field = Document.doubleField))


/**
* Sets the `sigma` parameter for the [[zio.elasticsearch.aggregation.ExtendedStatsAggregation]]. The`sigma` parameter
* controls how many standard deviations plus/minus from the mean should std_deviation_bounds object display
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* controls how many standard deviations plus/minus from the mean should std_deviation_bounds object display
* controls how many standard deviations plus/minus from the mean should std_deviation_bounds object display.

* @param value
* the value to use for sigma parameter
* @return
* an instance of the [[zio.elasticsearch.aggregation.ElasticAggregation]] enriched with the `sigma` parameter.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe put reference for this aggregation instead of global ElasticAggregation.

@@ -75,6 +105,38 @@ private[elasticsearch] object CardinalityAggregationResponse {
DeriveJsonDecoder.gen[CardinalityAggregationResponse]
}

case class StdDeviationBounds private[elasticsearch] (
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
case class StdDeviationBounds private[elasticsearch] (
private[elasticsearch] case class StdDeviationBounds private[elasticsearch] (

Comment on lines 134 to 135
implicit val boundsDecoder: JsonDecoder[StdDeviationBounds] =
DeriveJsonDecoder.gen[StdDeviationBounds]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put this in separate object.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And rename it to decoder.

lower_sampling: Double
)

private[elasticsearch] final case class ExtendedStatsAggregationResponse(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use Camel case. You can use this @jsonField("...") for specify name of json field.

@@ -17,6 +17,7 @@
package zio.elasticsearch.result

import zio.Chunk
import zio.elasticsearch.executor.response.StdDeviationBounds
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Create StdDeviationBoundsResult with same params, and map StdDeviationBounds to this new class. We are doing this duplication of code in the favour of privacy of the response layer (we don't want users to know about this layer).

@@ -104,6 +104,48 @@ object HttpExecutorSpec extends IntegrationSpec {
Executor.execute(ElasticRequest.createIndex(firstSearchIndex)),
Executor.execute(ElasticRequest.deleteIndex(firstSearchIndex)).orDie
),
test("aggregate using extended stats aggregation ttt") {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
test("aggregate using extended stats aggregation ttt") {
test("aggregate using extended stats aggregation") {

@vanjaftn vanjaftn force-pushed the task-support-extended-stats-aggregation branch from 2a1a584 to 0049424 Compare November 15, 2023 09:02
Comment on lines 49 to 69
ExtendedStatsAggregationResult(
count,
min,
max,
avg,
sum,
sumOfSquares,
variance,
variancePopulation,
varianceSampling,
stdDeviation,
stdDeviationPopulation,
stdDeviationSampling,
StdDeviationBoundsResult(
stdDeviationBoundsResponse.upper,
stdDeviationBoundsResponse.lower,
stdDeviationBoundsResponse.upperPopulation,
stdDeviationBoundsResponse.lowerPopulation,
stdDeviationBoundsResponse.upperSampling,
stdDeviationBoundsResponse.lowerSampling
)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Name paramters since there are a lot of them.

@dbulaja98 dbulaja98 merged commit 1cb75b7 into lambdaworks:main Nov 15, 2023
13 checks passed
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

Successfully merging this pull request may close these issues.

2 participants