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

Remove getValue and getValues from Field #79516

Merged
merged 3 commits into from
Oct 19, 2021
Merged

Conversation

jdconrad
Copy link
Contributor

@jdconrad jdconrad commented Oct 19, 2021

With duck-typing in Painless and potentially non-dynamic languages requiring a cast under the current design, this change removes unnecessary methods from the Field base class. The removal of getValue and getValues allows sub classes to return primitives from these methods and opens up potential design space for performance improvements.

Note this removes getLong from the unsigned long field; however, this is currently undocumented and unused behavior.

@jdconrad jdconrad added :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache v8.0.0 labels Oct 19, 2021
@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label Oct 19, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

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

LGTM

@jdconrad jdconrad added the auto-backport Automatically create backport pull requests when merged label Oct 19, 2021
@jdconrad
Copy link
Contributor Author

@rjernst Thanks for the review.

@jdconrad jdconrad merged commit 541a4fd into elastic:master Oct 19, 2021
@jdconrad jdconrad added v7.16.0 and removed auto-backport Automatically create backport pull requests when merged labels Oct 19, 2021
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

Status Branch Result
7.x Commit could not be cherrypicked due to conflicts

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 79516

jdconrad added a commit to jdconrad/elasticsearch that referenced this pull request Oct 19, 2021
With duck-typing in Painless and potentially non-dynamic languages requiring a cast under the
current design, this change removes unnecessary methods from the Field base class. The removal of
getValue and getValues allows sub classes to return primitives from these methods and opens up
potential design space for performance improvements.

Note this removes getLong from the unsigned long field; however, this is currently undocumented and
unused behavior.
jdconrad added a commit that referenced this pull request Oct 19, 2021
With duck-typing in Painless and potentially non-dynamic languages requiring a cast under the 
current design, this change removes unnecessary methods from the Field base class. The removal 
of getValue and getValues allows sub classes to return primitives from these methods and opens 
up potential design space for performance improvements.

Note this removes getLong from the unsigned long field; however, this is currently undocumented 
and unused behavior.
weizijun added a commit to weizijun/elasticsearch that referenced this pull request Oct 20, 2021
* upstream/master: (24 commits)
  Implement framework for migrating system indices (elastic#78951)
  Improve transient settings deprecation message (elastic#79504)
  Remove getValue and getValues from Field (elastic#79516)
  Store Template's mappings as bytes for disk serialization (elastic#78746)
  [ML] Add queue_capacity setting to start deployment API (elastic#79433)
  [ML] muting rest compat test issue elastic#79518 (elastic#79519)
  Avoid redundant available indices check (elastic#76540)
  Re-enable BWC tests
  TEST Ensure password 14 chars length on Kerberos FIPS tests (elastic#79496)
  [DOCS] Temporarily remove APM links (elastic#79411)
  Fix CCSDuelIT for skipped shards (elastic#79490)
  Add other time accounting in HotThreads (elastic#79392)
  Add deprecation info API entries for deprecated monitoring settings (elastic#78799)
  Add note in breaking changes for nameid_format (elastic#77785)
  Use 'migration' instead of 'upgrade' in GET system feature migration status responses (elastic#79302)
  Upgrade lucene version 8b68bf60c98 (elastic#79461)
  Use Strings#EMPTY_ARRAY (elastic#79452)
  Quicker shared cache file preallocation (elastic#79447)
  [ML] Removing some code that's obsolete for 8.0 (elastic#79444)
  Ensure indexing_data CCR requests are compressed (elastic#79413)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Scripting Scripting abstractions, Painless, and Mustache >refactoring Team:Core/Infra Meta label for core/infra team v7.16.0 v8.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants