Skip to content

Fix propagation of dynamic mapping parameter when applying copy_to#121109

Merged
lkts merged 13 commits intoelastic:mainfrom
lkts:fix_113049
Jan 30, 2025
Merged

Fix propagation of dynamic mapping parameter when applying copy_to#121109
lkts merged 13 commits intoelastic:mainfrom
lkts:fix_113049

Conversation

@lkts
Copy link
Contributor

@lkts lkts commented Jan 28, 2025

Closes #113049.
@lkts lkts added >bug :Search Foundations/Mapping Index mappings, including merging and defining field types auto-backport Automatically create backport pull requests when merged :StorageEngine/Mapping The storage related side of mappings v8.18.0 labels Jan 28, 2025
@elasticsearchmachine elasticsearchmachine added Team:StorageEngine v9.0.0 Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch labels Jan 28, 2025
@elasticsearchmachine
Copy link
Collaborator

elasticsearchmachine commented Jan 28, 2025

Pinging @elastic/es-storage-engine (Team:StorageEngine)
@elasticsearchmachine
Copy link
Collaborator

elasticsearchmachine commented Jan 28, 2025

Hi @lkts, I've created a changelog YAML for you.
@elasticsearchmachine
Copy link
Collaborator

elasticsearchmachine commented Jan 28, 2025

Pinging @elastic/es-search-foundations (Team:Search Foundations)
lkts
lkts commented Jan 28, 2025
View reviewed changes
server/src/main/java/org/elasticsearch/index/mapper/DocumentParserContext.java

// Used to create a copy_to context.
// It is important to reset `dynamic` here since it is possible that we copy into a completely different object.
private Wrapper(RootObjectMapper root, DocumentParserContext in) {
Copy link
Contributor Author

@lkts lkts Jan 28, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Note that the call site didn't change here, this works because createCopyToContext is the only place that passes a root maper to Wrapper constructor. I think this is fragile but i don't have anything better.
Copy link
Contributor

@kkrik-es kkrik-es Jan 30, 2025

Choose a reason for hiding this comment

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

Shall we create a factory method like createWithRootDynamic and use it explicitly?
Copy link
Contributor Author

@lkts lkts Jan 30, 2025

Choose a reason for hiding this comment

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

It's an optioin but you still need this constructor to call super.
@jordan-powers jordan-powers mentioned this pull request Jan 29, 2025
Closed
@lkts
Copy link
Contributor Author

lkts commented Jan 29, 2025

Closes #120384.
@lkts lkts enabled auto-merge (squash) January 30, 2025 16:23
@lkts
Copy link
Contributor Author

lkts commented Jan 30, 2025

CI failure is #121335.
@lkts lkts merged commit 1225b07 into elastic:main Jan 30, 2025
17 checks passed
@elasticsearchmachine
Copy link
Collaborator

elasticsearchmachine commented Jan 30, 2025

💔 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 121109
saikatsarkar056 pushed a commit to saikatsarkar056/elasticsearch that referenced this pull request Jan 31, 2025
@lkts @saikatsarkar056
Fix propagation of dynamic mapping parameter when applying copy_to (e…
5205392 
…lastic#121109)
@lkts lkts mentioned this pull request Jan 31, 2025
Merged
@lkts
Copy link
Contributor Author

lkts commented Jan 31, 2025

💚 All backports created successfully

Status Branch Result
✅ 8.x

Questions ?

Please refer to the Backport tool documentation
@lkts lkts deleted the fix_113049 branch January 31, 2025 01:30
elasticsearchmachine pushed a commit that referenced this pull request Jan 31, 2025
@lkts
Fix propagation of dynamic mapping parameter when applying copy_to (#…
7ff4f3e 
…121109) (#121357)

(cherry picked from commit 1225b07)

# Conflicts:
#	rest-api-spec/build.gradle
bpintea pushed a commit to bpintea/elasticsearch that referenced this pull request Jan 31, 2025
@lkts @bpintea
Fix propagation of dynamic mapping parameter when applying copy_to (e…
2919025 
…lastic#121109)
@tlrx
Copy link
Member

tlrx commented Jan 31, 2025

I suspect this change needs to be backported to the 9.0 branch as well: some BWC tests that upgrade a cluster from 8.19.0 to 9.0 are now failing due to the missing required features [mapper.copy_to.dynamic_handling].

See #121362 or #121365 for example.
@kkrik-es kkrik-es mentioned this pull request Jan 31, 2025
Merged
@kkrik-es
Copy link
Contributor

kkrik-es commented Jan 31, 2025

💚 All backports created successfully

Status Branch Result
✅ 9.0

Questions ?

Please refer to the Backport tool documentation
kkrik-es pushed a commit to kkrik-es/elasticsearch that referenced this pull request Jan 31, 2025
@lkts
Fix propagation of dynamic mapping parameter when applying copy_to (e…
53548d9 
…lastic#121109)

(cherry picked from commit 1225b07)

# Conflicts:
#	rest-api-spec/build.gradle
@kkrik-es
Copy link
Contributor

kkrik-es commented Jan 31, 2025

I suspect this change needs to be backported to the 9.0 branch as well: some BWC tests that upgrade a cluster from 8.19.0 to 9.0 are now failing due to the missing required features [mapper.copy_to.dynamic_handling].

See #121362 or #121365 for example.

Created the backport with auto-merge.
@tlrx tlrx mentioned this pull request Jan 31, 2025
Closed
kkrik-es added a commit that referenced this pull request Jan 31, 2025
@kkrik-es @lkts
Fix propagation of dynamic mapping parameter when applying copy_to (#…
21710e7 
…121109) (#121386)

(cherry picked from commit 1225b07)

# Conflicts:
#	rest-api-spec/build.gradle

Co-authored-by: Oleksandr Kolomiiets <oleksandr.kolomiiets@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Automatically create backport pull requests when merged backport pending >bug :Search Foundations/Mapping Index mappings, including merging and defining field types :StorageEngine/Mapping The storage related side of mappings Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch Team:StorageEngine v8.19.0 v9.1.0

4 participants

@lkts @elasticsearchmachine @tlrx @kkrik-es