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

Improve support for joda datetime to java datetime in Painless #83099

Merged
merged 7 commits into from
Jan 27, 2022

Conversation

jdconrad
Copy link
Contributor

Within Painless, this change augments TemporalAccessor with the method getMillis to make milliseconds easier to access directly from a datetime within a document.

This change also adds several methods from Joda datetimes augmented onto ZonedDateTime that throw an UnsupportedOperationException with a suggestion of how to implement that Joda method using ZonedDateTime instead. These methods include the following:

  • getCenturyOfEra
  • getEra
  • getHourOfDay
  • getMillisOfDay
  • getMillisOfSecond
  • getMinuteOfDay
  • getMinuteOfHour
  • getMonthOfYear
  • getSecondOfDay
  • getSecondOfMinute
  • getWeekOfWeekyear
  • getWeekyear
  • getYearOfCentury
  • getYearOfEra

@jdconrad jdconrad added >enhancement :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache v8.1.0 labels Jan 25, 2022
@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label Jan 25, 2022
@elasticmachine
Copy link
Collaborator

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

@elasticsearchmachine
Copy link
Collaborator

Hi @jdconrad, I've created a changelog YAML for you.

@jdconrad
Copy link
Contributor Author

@elasticmachine run elasticsearch-ci/packaging-tests-unix-sample

}

public static DayOfWeek getDayOfWeekEnum(ZonedDateTime receiver) {
return receiver.getDayOfWeek();
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should have a separate file or section these Augmentations that's simply unsupported.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems like a good idea. I would want to do that in as a follow up, though.

source: "/* avoid stash */ $('date', null).getCenturyOfEra()"
- match: { error.failed_shards.0.reason.caused_by.type: "unsupported_operation_exception" }
- match: { error.failed_shards.0.reason.caused_by.reason:
"joda method [getCenturyOfEra] is no longer available; use [<ZonedDateTime instance>.get(ChronoField.YEAR_OF_ERA) / 100] instead" }
Copy link
Contributor

Choose a reason for hiding this comment

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

For each of these suggestions, can you add a test that uses the suggestion right the test that suggests the alternative?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call. Caught a pair of bugs for the week based suggestions!

@stu-elastic
Copy link
Contributor

Can you reference the change that deprecated these files in the commit?

Copy link
Contributor

@stu-elastic stu-elastic left a comment

Choose a reason for hiding this comment

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

These augments return the methods removed in #78417

@jdconrad jdconrad merged commit ab7f002 into elastic:master Jan 27, 2022
weizijun added a commit to weizijun/elasticsearch that referenced this pull request Jan 31, 2022
* upstream/master: (100 commits)
  Avoid duplicate _type fields in v7 compat layer (elastic#83239)
  Bump bundled JDK to 17.0.2+8 (elastic#83243)
  [DOCS] Correct header syntax (elastic#83275)
  Add unit tests for indices.recovery.max_bytes_per_sec default values (elastic#83261)
  [DOCS] Add note that write indices are not replicated (elastic#82997)
  Add notes on indexing to kNN search guide (elastic#83188)
  Fix get-snapshot-api :docs:integTest (elastic#83273)
  FilterPathBasedFilter support match fieldname with dot (elastic#83178)
  Fix compilation issues in example-plugins (elastic#83258)
  fix ClusterStateListener javadoc (elastic#83246)
  Speed up Building Indices Lookup in Metadata (elastic#83241)
  Mute whole suite for elastic#82502 (elastic#83252)
  Make PeerFinder log messages happier (elastic#83222)
  [Docs] Add supported _terms_enum field types (elastic#83244)
  Add an aggregator for IPv4 and IPv6 subnets (elastic#82410)
  [CI] Fix 70_time_series/default sort yaml test failures (elastic#83217)
  Update test-failure Issue Template to include "needs:triage" label elastic#83226
  Add an index->step cache to the PolicyStepsRegistry (elastic#82316)
  Improve support for joda datetime to java datetime transition in Painless (elastic#83099)
  Fix joda migration for week based methods in Painless (elastic#83232)
  ...

# Conflicts:
#	x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/v2/TransportRollupAction.java
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 >enhancement Team:Core/Infra Meta label for core/infra team v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants