Conversation
|
@Mitep, are you still working on this? |
I didn't, I'll try to rebase it and see where we at |
Mitep
force-pushed
the
task-add-support-for-range-agg
branch
2 times, most recently
from
58de7eb to
11f344d
Compare
Mitep
force-pushed
the
task-add-support-for-range-agg
branch
from
11f344d to
c641f1a
Compare
website/sidebars.js
Outdated
| 'overview/aggregations/elastic_aggregation_filter', | ||
| 'overview/aggregations/elastic_aggregation_max', | ||
| 'overview/aggregations/elastic_aggregation_min', | ||
| 'overview/aggregations/elastic_aggregation_range', |
There was a problem hiding this comment.
Let's keep this alphabetically sorted.
| assert(aggregationTs.toJson)(equalTo(expectedTs.toJson)) && | ||
| assert(aggregationWithMissing.toJson)(equalTo(expectedWithMissing.toJson)) | ||
| }, | ||
| test("range") { |
There was a problem hiding this comment.
Let's keep this alphabetically sorted.
There was a problem hiding this comment.
Also, can you provide "type-safe" tests? Using TestDocument.stringField instead of "testField".
| equalTo(Min(name = "aggregation", field = "intField", missing = Some(20.0))) | ||
| ) | ||
| }, | ||
| test("range") { |
There was a problem hiding this comment.
Please keep this alphabetically sorted, and provide type-safe examples.
| private[elasticsearch] sealed trait RangeAggregationResult extends AggregationResult | ||
|
|
||
| private[elasticsearch] final case class RegularRangeAggregationBucketResult( | ||
| key: String, | ||
| to: Option[Double], | ||
| from: Option[Double], | ||
| docCount: Int | ||
| ) | ||
|
|
||
| private[elasticsearch] final case class KeyedRangeAggregationBucketResult( | ||
| to: Option[Double], | ||
| from: Option[Double], | ||
| docCount: Int | ||
| ) | ||
|
|
||
| private[elasticsearch] final case class RegularRangeAggregationResult( | ||
| buckets: Chunk[RegularRangeAggregationBucketResult] | ||
| ) extends RangeAggregationResult | ||
|
|
||
| private[elasticsearch] final case class KeyedRangeAggregationResult( | ||
| buckets: Map[String, KeyedRangeAggregationBucketResult] | ||
| ) extends RangeAggregationResult | ||
|
|
There was a problem hiding this comment.
Keep this alphabetically sorted.
| * a [[RIO]] effect that, when executed, will produce the aggregation as instance of | ||
| * [[result.RangeAggregationResult]]. | ||
| */ | ||
| def asRangeAggregation(name: String): RIO[R, Option[RangeAggregationResult]] = |
There was a problem hiding this comment.
Keep this alphabetically sorted.
| MaxAggregationResponse.decoder.decodeJson(data.toString).map(field.split("#")(1) -> _) | ||
| case str if str.contains("min#") => | ||
| MinAggregationResponse.decoder.decodeJson(data.toString).map(field.split("#")(1) -> _) | ||
| case str if str.contains("range#") => |
| range: SingleRange, | ||
| ranges: SingleRange* |
There was a problem hiding this comment.
Why is this named SingleRange? Why not Range?
There was a problem hiding this comment.
Cannot have two case classes with the same name inside the same package. I don't like the SingleRange name either, but I can't think of a better approach right now.
| private[elasticsearch] final case class SingleRange( | ||
| from: Option[Double], | ||
| to: Option[Double], | ||
| key: Option[String] | ||
| ) { self => | ||
| def from(value: Double): SingleRange = self.copy(from = Some(value)) | ||
| def to(value: Double): SingleRange = self.copy(to = Some(value)) | ||
| def key(value: String): SingleRange = self.copy(key = Some(value)) | ||
| } | ||
|
|
||
| object SingleRange { | ||
|
|
||
| def from(value: Double): SingleRange = | ||
| SingleRange(from = Some(value), to = None, key = None) | ||
|
|
||
| def to(value: Double): SingleRange = | ||
| SingleRange(from = None, to = Some(value), key = None) | ||
|
|
||
| def apply(from: Double, to: Double): SingleRange = | ||
| SingleRange(from = Some(from), to = Some(to), key = None) | ||
|
|
||
| } |
There was a problem hiding this comment.
I would say we have to create an API for this.
| keyed: Option[Boolean] | ||
| ) extends RangeAggregation { self => | ||
|
|
||
| def keyed(value: Boolean): Range = |
There was a problem hiding this comment.
Dose it make sense to have keyed and notKeyed instead of having keyed(value: Boolean)?
There was a problem hiding this comment.
I tried, but we cannot have keyed as def while having keyed as field, so I put asKeyed as def name.
2 participants
No description provided.