Change default locale of date mappers to ENGLISH by thecoop · Pull Request #112799 · elastic/elasticsearch

thecoop · 2024-09-12T10:12:46Z

This resolves #112744

Date processors are covered by #112796

elasticsearchmachine · 2024-09-12T10:13:30Z

Pinging @elastic/es-search-foundations (Team:Search Foundations)

elasticsearchmachine · 2024-09-12T10:13:56Z

Hi @thecoop, I've created a changelog YAML for you. Note that since this PR is labelled >breaking, you need to update the changelog YAML to fill out the extended information sections.

elasticsearchmachine · 2024-09-23T13:15:53Z

Pinging @elastic/es-data-management (Team:Data Management)

thecoop · 2024-09-23T15:33:44Z

...est/java/org/elasticsearch/aggregations/bucket/histogram/InternalAutoDateHistogramTests.java

-            deserialized.getMetadata(),
-            instance.getBucketInnerInterval()
-        );
-        assertEqualInstances(instance, modified);


This test can no longer pass, as the locale of the formatter changes from root to english, and that can't be easily fixed up afterwards

I am not familiar with this test but I would wonder if we need a replacement of some sort for it. This is a question for the @elastic/es-analytical-engine I think . cc @nik9000

thecoop · 2024-09-24T14:57:47Z

@elasticmachine update branch

javanna

I pinged a couple of folks to check some specific aspects of this change. I think my only question is whether we have proper rolling upgrade tests that cover for this change, to make sure that the change in default value is applied and works properly. Not sure if you have given this some thought already.

javanna · 2024-09-27T08:36:20Z

...est/java/org/elasticsearch/aggregations/bucket/histogram/InternalAutoDateHistogramTests.java

-            deserialized.getMetadata(),
-            instance.getBucketInnerInterval()
-        );
-        assertEqualInstances(instance, modified);


I am not familiar with this test but I would wonder if we need a replacement of some sort for it. This is a question for the @elastic/es-analytical-engine I think . cc @nik9000

javanna · 2024-09-27T08:38:00Z

server/src/main/java/org/elasticsearch/index/mapper/DataStreamTimestampFieldMapper.java

            configuredSettings.remove("type");
            configuredSettings.remove("meta");
            configuredSettings.remove("format");
+            configuredSettings.remove("locale");


@martijnvg do you know the answer to this question? Are you ok with this change?

modules/aggregations/build.gradle

thecoop · 2024-09-27T10:24:22Z

I think my only question is whether we have proper rolling upgrade tests that cover for this change, to make sure that the change in default value is applied and works properly.

I don't think we need to do anything specific here, as the default locale is not persisted in a 'canonical' definition, so it'll just use whatever the specified parameter default is

martijnvg

The org.elasticsearch.index.mapper package side LGTM.

nik9000 · 2024-10-04T11:54:49Z

server/src/main/java/org/elasticsearch/search/DocValueFormat.java

            String zoneId = in.readString();
            this.timeZone = ZoneId.of(zoneId);
-            this.formatter = DateFormatter.forPattern(formatterPattern).withZone(this.timeZone);
+            this.formatter = DateFormatter.forPattern(formatterPattern).withZone(this.timeZone).withLocale(locale);


Should we set this to DEFAULT_LOCALE rather than serialize it? It looks like you can't change it.

It is read from the specified DateTimeFormatter, which can be changed by eg the new locale option on RangeFieldMapper

elasticsearchmachine · 2024-10-07T09:23:46Z

💔 Backport failed

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

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

English is not changing between COMPAT and CLDR locale databases, whereas ROOT is

…14210) Backport #112799 to 8.16, for CLDR locale compatibility

English is not changing between COMPAT and CLDR locale databases, whereas ROOT is

thecoop added >non-issue blocker :Search Foundations/Mapping Index mappings, including merging and defining field types v8.16.0 v9.0.0 labels Sep 12, 2024

elasticsearchmachine added the Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch label Sep 12, 2024

thecoop added >bug >breaking and removed >non-issue Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch labels Sep 12, 2024

elasticsearchmachine added the Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch label Sep 12, 2024

thecoop mentioned this pull request Sep 12, 2024

Use ISO week date numbering for english too #112801

Closed

thecoop added the WIP label Sep 13, 2024

Change default locale of date mappers to ENGLISH

b2ba412

thecoop force-pushed the english-field-mappers branch from 37df868 to b2ba412 Compare September 23, 2024 10:06

thecoop added >non-issue test-update-serverless and removed >bug >breaking labels Sep 23, 2024

thecoop added 3 commits September 23, 2024 11:14

Update docs for locale change

9717ce6

Add locale to serialization format

94b0666

Update uses of root locale in other field mappers

e71bc00

thecoop removed the WIP label Sep 23, 2024

thecoop mentioned this pull request Sep 23, 2024

Change default locale of date processors to ENGLISH #112796

Merged

Some test updates

8bf7cf3

thecoop added the :StorageEngine/Data streams Data streams and their lifecycles label Sep 23, 2024

elasticsearchmachine added the Team:Data Management (obsolete) DO NOT USE. This team no longer exists. label Sep 23, 2024

thecoop added 3 commits September 23, 2024 14:45

Merge remote-tracking branch 'upstream/main' into english-field-mappers

6f03648

Specify default locale on default mappers

d70ed92

Merge remote-tracking branch 'upstream/main' into english-field-mappers

cf88f62

thecoop commented Sep 23, 2024

View reviewed changes

Propagate defaults more

05b4255

thecoop requested a review from a team as a code owner September 24, 2024 09:51

Merge branch 'main' into english-field-mappers

bf47823

thecoop removed the blocker label Sep 24, 2024

javanna reviewed Sep 27, 2024

View reviewed changes

thecoop mentioned this pull request Oct 2, 2024

[CI] RuntimeFieldsClientYamlTestSuiteIT test {yaml=runtime_fields/10_keyword/fetch fields} failing #112782

Closed

thecoop requested review from martijnvg and nik9000 October 2, 2024 10:19

thecoop added the auto-backport Automatically create backport pull requests when merged label Oct 2, 2024

mark-vieira approved these changes Oct 3, 2024

View reviewed changes

martijnvg approved these changes Oct 3, 2024

View reviewed changes

thecoop added 2 commits October 4, 2024 10:27

Reference the same default locale

b8ee548

Merge branch 'main' into english-field-mappers

460576b

thecoop force-pushed the english-field-mappers branch from e0a2896 to 460576b Compare October 4, 2024 09:29

nik9000 reviewed Oct 4, 2024

View reviewed changes

thecoop merged commit 4ef5ea6 into elastic:main Oct 7, 2024

thecoop deleted the english-field-mappers branch October 7, 2024 09:22

elasticsearchmachine added the backport pending label Oct 7, 2024

thecoop added a commit to thecoop/elasticsearch that referenced this pull request Oct 7, 2024

Change default locale of date mappers to ENGLISH (elastic#112799)

66c2cd6

English is not changing between COMPAT and CLDR locale databases, whereas ROOT is

thecoop mentioned this pull request Oct 7, 2024

[8.16] Change default locale of date mappers to ENGLISH (#112799) #114210

Merged

thecoop removed the backport pending label Oct 7, 2024

thecoop added a commit that referenced this pull request Oct 7, 2024

[8.16] Change default locale of date mappers to ENGLISH (#112799) (#1…

1b01548

…14210) Backport #112799 to 8.16, for CLDR locale compatibility

matthewabbott pushed a commit to matthewabbott/elasticsearch that referenced this pull request Oct 10, 2024

Change default locale of date mappers to ENGLISH (elastic#112799)

2c9b6b2

English is not changing between COMPAT and CLDR locale databases, whereas ROOT is

Conversation

thecoop commented Sep 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticsearchmachine commented Sep 12, 2024

Uh oh!

elasticsearchmachine commented Sep 12, 2024

Uh oh!

elasticsearchmachine commented Sep 23, 2024

Uh oh!

thecoop Sep 23, 2024

Choose a reason for hiding this comment

Uh oh!

javanna Sep 27, 2024

Choose a reason for hiding this comment

Uh oh!

thecoop commented Sep 24, 2024

Uh oh!

javanna left a comment

Choose a reason for hiding this comment

Uh oh!

javanna Sep 27, 2024

Choose a reason for hiding this comment

Uh oh!

javanna Sep 27, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

thecoop commented Sep 27, 2024

Uh oh!

martijnvg left a comment

Choose a reason for hiding this comment

Uh oh!

nik9000 Oct 4, 2024

Choose a reason for hiding this comment

Uh oh!

thecoop Oct 4, 2024

Choose a reason for hiding this comment

Uh oh!

elasticsearchmachine commented Oct 7, 2024

💔 Backport failed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

thecoop commented Sep 12, 2024 •

edited

Loading