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.

Sign up for GitHub

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

Jump to bottom

Remove interfaces for aggregation results #82273

Open
nik9000 opened this issue Jan 5, 2022 · 3 comments
Open

Remove interfaces for aggregation results #82273

nik9000 opened this issue Jan 5, 2022 · 3 comments
Labels
:Analytics/Aggregations Aggregations Meta >non-issue Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) >tech debt

Comments

@nik9000
Copy link
Member

nik9000 commented Jan 5, 2022

These interfaces existed to separate the "internal" results objects from the transport client so it wouldn't "see" then internal results. After 8.0 we don't support the transport client any more so we can drop the interfaces and rename the results. So, for example, we'd delete Filters interface and rename InternalFilters to Filters or FiltersResult or FiltersAggregationResult or something.
@nik9000 nik9000 added >non-issue :Analytics/Aggregations Aggregations needs:triage Requires assignment of a team area label labels Jan 5, 2022
@elasticmachine elasticmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Jan 5, 2022
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-analytics-geo (Team:Analytics)

@nik9000 nik9000 mentioned this issue Jan 5, 2022
Closed
@not-napoleon not-napoleon removed the needs:triage Requires assignment of a team area label label Jan 6, 2022
@nik9000 nik9000 self-assigned this Jan 6, 2022
nik9000 added a commit to nik9000/elasticsearch that referenced this issue Jan 6, 2022
@nik9000
Fix _score access in double scripted terms agg
6e82ce1 
When we're aggregation on a script that contains `score` with the value
type hint `double` we were always using a score of `0.0`. This passes
the score into the script properly.

This also updates `terms` agg tests in accordance with elastic#82273. I found
this bug while updating tests for elastic#82273 but wanted to split out the bug
fix. But I wanted to include all of the updates to these tests for
issue elastic#82273 because they add extra type checks for similar issues.
@nik9000 nik9000 mentioned this issue Jan 6, 2022
Merged
nik9000 added a commit that referenced this issue Jan 31, 2022
@nik9000
Fix _score access in double scripted terms agg (#82331)
6383d22 
When we're aggregation on a script that contains `score` with the value
type hint `double` we were always using a score of `0.0`. This passes
the score into the script properly.

This also updates `terms` agg tests in accordance with #82273. I found
this bug while updating tests for #82273 but wanted to split out the bug
fix. But I wanted to include all of the updates to these tests for
issue #82273 because they add extra type checks for similar issues.
nik9000 added a commit to nik9000/elasticsearch that referenced this issue Jan 31, 2022
@nik9000
Remove interface for transport client
bf5fee1 
This removes the `Sum` interface that was important for the transport
client and renames `InternalSum` to `Sum`. The transport client isn't a
think any more so we don't need to keep these interfaces around any
more.

Related to elastic#82273
@nik9000 nik9000 mentioned this issue Jan 31, 2022
Merged
elasticsearchmachine pushed a commit that referenced this issue Feb 1, 2022
@nik9000
Remove interface for transport client (#83342)
5143521 
This removes the `Sum` interface that was important for the transport
client and renames `InternalSum` to `Sum`. The transport client isn't a
think any more so we don't need to keep these interfaces around any
more.

Related to #82273
nik9000 added a commit to nik9000/elasticsearch that referenced this issue Feb 3, 2022
@nik9000
Remove interface for transport client
6636c6f 
This removes the `Max` interface that was important for the transport
client and renames `InternalMax` to `Max`. The transport client isn't a
think any more so we don't need to keep these interfaces around any
more.

Related to elastic#82273
@nik9000 nik9000 mentioned this issue Feb 3, 2022
Merged
elasticsearchmachine pushed a commit that referenced this issue Feb 28, 2022
@nik9000
Remove interface for transport client (#83486)
8cbcace 
This removes the `Max` interface that was important for the transport
client and renames `InternalMax` to `Max`. The transport client isn't a
think any more so we don't need to keep these interfaces around any
more.

Related to #82273
@nik9000
Copy link
Member Author

nik9000 commented Mar 1, 2022 •
edited
Loading

  • Aggregations
    • Bucketing
      • Adjacency matrix
      • Auto-interval date histogram
      • Categorize text
      • Children
      • Composite
      • Date histogram
      • Date range
      • Diversified sampler
      • Filter
      • Filters
      • Geo-distance
      • Geohash grid
      • Geotile grid
      • Global
      • Histogram
      • IP range
      • Missing
      • Multi Terms
      • Nested
      • Parent
      • Range
      • Rare terms
      • Reverse nested
      • Sampler
      • Significant terms
      • Significant text
      • Terms
      • Variable width histogram
    • Metric
    • Pipeline
      • Average bucket
      • Bucket script
      • Bucket count K-S test
      • Bucket correlation
      • Bucket selector
      • Bucket sort
      • Cumulative cardinality
      • Cumulative sum
      • Derivative
      • Extended stats bucket
      • Inference bucket
      • Max bucket
      • Min bucket
      • Moving function
      • Moving percentiles
      • Normalize
      • Percentiles bucket
      • Serial differencing
      • Stats bucket
      • Sum bucket

nik9000 added a commit to nik9000/elasticsearch that referenced this issue Mar 3, 2022
@nik9000
Remove interface for transport client
27daabe 
This removes the `Min` interface that was important for the transport
client and renames `InternalMin` to `Min`. The transport client isn't a
thing any more so we don't need to keep these interfaces around any
more.

Related to elastic#82273
@nik9000 nik9000 mentioned this issue Mar 3, 2022
Merged
nik9000 added a commit that referenced this issue Mar 21, 2022
@nik9000
Remove interface for transport client (#84640)
dc8ec42 
This removes the `Min` interface that was important for the transport
client and renames `InternalMin` to `Min`. The transport client isn't a
thing any more so we don't need to keep these interfaces around any
more.

Related to #82273
nik9000 added a commit to nik9000/elasticsearch that referenced this issue Oct 19, 2022
@nik9000
Move derivative agg to module
d32e8ed 
This cleans up the derivative pipeline agg in a few days:
1. Moves it to the `aggregations` module ala elastic#90283
2. Drops the ceremonial interface from the result ala elastic#82273
3. Adds comprehensive REST layer tests for it ala elastic#26220
4. Removes some `ESIntegTestCase tests` that duplicated those in the
   `AggregatorTestCase`.
@nik9000 nik9000 mentioned this issue Oct 19, 2022
Merged
nik9000 added a commit that referenced this issue Oct 31, 2022
@nik9000
Move derivative agg to module (#91014)
1474d79 
This cleans up the derivative pipeline agg in a few days:
1. Moves it to the `aggregations` module ala #90283
2. Drops the ceremonial interface from the result ala #82273
3. Adds comprehensive REST layer tests for it ala #26220
4. Removes some `ESIntegTestCase tests` that duplicated those in the
   `AggregatorTestCase`.
martijnvg added a commit to martijnvg/elasticsearch that referenced this issue Nov 7, 2022
@martijnvg
Move time_series aggregation to aggregations module.
2821ace 
This also drops the TimeSeries ceremonial interface.

Note that this change also moves a search cancellation test to aggregations module. It does so by creating a base search cancallation base class that both server and this module share.

Relates to elastic#90283
Relates to elastic#82273
@martijnvg martijnvg mentioned this issue Nov 7, 2022
Merged
elasticsearchmachine pushed a commit that referenced this issue Nov 8, 2022
@martijnvg
Move time_series aggregation to aggregations module. (#91356)
1e186bb 
This also drops the TimeSeries ceremonial interface.

Note that this change also moves a search cancellation test to
aggregations module. It does so by creating a base search cancallation
base class that both server and this module share.

Relates to #90283 Relates to #82273
martijnvg added a commit to martijnvg/elasticsearch that referenced this issue Jan 4, 2023
@martijnvg
Move median_absolute_deviation aggregation to aggregations module.
56da5cc 
The `InternalAggregationTestCase`, `AbstractNumericMetricTestCase` and
`AggregationTestScriptsPlugin` classes were moved to test/framework module,
so that both server and aggregations module can use these base classes.

This also drops the MedianAbsoluteDeviation ceremonial interface.

Relates to elastic#90283
Relates to elastic#82273
@martijnvg martijnvg mentioned this issue Jan 4, 2023
Draft
@wchaparro wchaparro added the Meta label Jan 5, 2024
@nik9000 nik9000 removed their assignment Jul 1, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-analytical-engine (Team:Analytics)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
:Analytics/Aggregations Aggregations Meta >non-issue Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) >tech debt
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@nik9000 @not-napoleon @wchaparro @elasticmachine @elasticsearchmachine