Skip to content

ExponentialHistograms: refactor tests to use builder, add copy method #134037

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
Merged
merged 11 commits into from
Sep 4, 2025
Merged

ExponentialHistograms: refactor tests to use builder, add copy method #134037

merged 11 commits into from
Sep 4, 2025

Conversation

@JonasKunz
Copy link
Contributor

Follow up of #133969 (comment).

  • Refactors the tests to use the builder for improved readability and less dependency on implementation details
  • Adds a way of creating a builder and initializing it to copy an existing histogram (needed for ES|QL tests coming in ES|QL block type for exponential histograms #133393

@elasticsearchmachine elasticsearchmachine added v9.2.0 external-contributor Pull request authored by a developer outside the Elasticsearch team labels Sep 3, 2025
@JonasKunz JonasKunz marked this pull request as ready for review September 3, 2025 11:22
@elasticsearchmachine elasticsearchmachine added the needs:triage Requires assignment of a team area label label Sep 3, 2025
@JonasKunz JonasKunz added the :StorageEngine/Mapping The storage related side of mappings label Sep 3, 2025
@elasticsearchmachine elasticsearchmachine added Team:StorageEngine and removed needs:triage Requires assignment of a team area label labels Sep 3, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-storage-engine (Team:StorageEngine)

@JonasKunz JonasKunz added needs:triage Requires assignment of a team area label >tech debt Team:StorageEngine and removed Team:StorageEngine labels Sep 3, 2025
@elasticsearchmachine elasticsearchmachine removed the needs:triage Requires assignment of a team area label label Sep 3, 2025
felixbarny
felixbarny approved these changes Sep 4, 2025
View reviewed changes
Copy link
Member

@felixbarny felixbarny left a comment

Choose a reason for hiding this comment

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

I like how much cleaner everything gets! Especially copyWithModification.

JonasKunz and others added 2 commits September 4, 2025 10:24
@JonasKunz @kkrik-es
Update libs/exponential-histogram/src/main/java/org/elasticsearch/exp…
1524f00 
…onentialhistogram/ExponentialHistogramBuilder.java

Co-authored-by: Kostas Krikellas <[email protected]>
@JonasKunz @kkrik-es
Update libs/exponential-histogram/src/main/java/org/elasticsearch/exp…
6bf4488 
…onentialhistogram/ExponentialHistogramBuilder.java

Co-authored-by: Kostas Krikellas <[email protected]>
@JonasKunz JonasKunz enabled auto-merge (squash) September 4, 2025 08:29
@JonasKunz JonasKunz merged commit ac0d061 into elastic:main Sep 4, 2025
33 checks passed
@JonasKunz JonasKunz deleted the histo-tests-refactor branch September 4, 2025 10:11
phananh1010 added a commit to phananh1010/elasticsearch that referenced this pull request Sep 11, 2025
@phananh1010
Mirror upstream elastic#134037 as single snapshot commit for AI review
d69e504 
BASE=7152e61bd1761b4fa64f60c7a174269243d6a826
HEAD=7e0efd0004ce38b906ebd10c9cf89ab713098fef
Branch=main
phananh1010 added a commit to phananh1010/elasticsearch that referenced this pull request Sep 11, 2025
@phananh1010
Mirror upstream elastic#134037 as single snapshot commit for AI review
a53abc8 
BASE=7152e61bd1761b4fa64f60c7a174269243d6a826
HEAD=7e0efd0004ce38b906ebd10c9cf89ab713098fef
Branch=main
phananh1010 added a commit to phananh1010/elasticsearch that referenced this pull request Sep 16, 2025
@phananh1010
Mirror upstream elastic#134037 as single snapshot commit for AI review
f78e9aa 
BASE=7152e61bd1761b4fa64f60c7a174269243d6a826
HEAD=7e0efd0004ce38b906ebd10c9cf89ab713098fef
Branch=main
phananh1010 added a commit to phananh1010/elasticsearch that referenced this pull request Sep 16, 2025
@phananh1010
Mirror upstream elastic#134037 as single snapshot commit for AI review
8314577 
BASE=7152e61bd1761b4fa64f60c7a174269243d6a826
HEAD=7e0efd0004ce38b906ebd10c9cf89ab713098fef
Branch=main
phananh1010 added a commit to phananh1010/elasticsearch that referenced this pull request Oct 9, 2025
@phananh1010
Mirror upstream elastic#134037 as single snapshot commit for AI review
c397a96 
BASE=7152e61bd1761b4fa64f60c7a174269243d6a826
HEAD=7e0efd0004ce38b906ebd10c9cf89ab713098fef
Branch=main
phananh1010 added a commit to phananh1010/elasticsearch that referenced this pull request Oct 16, 2025
@phananh1010
Mirror upstream elastic#134037 as single snapshot commit for AI review
927505a 
BASE=7152e61bd1761b4fa64f60c7a174269243d6a826
HEAD=7e0efd0004ce38b906ebd10c9cf89ab713098fef
Branch=main
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@felixbarny felixbarny felixbarny approved these changes

@kkrik-es kkrik-es kkrik-es approved these changes

Assignees

No one assigned

Labels

external-contributor Pull request authored by a developer outside the Elasticsearch team :StorageEngine/Mapping The storage related side of mappings Team:StorageEngine >tech debt v9.2.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@JonasKunz @elasticsearchmachine @felixbarny @kkrik-es